Skip to content

Apply the configured log user and group lists to daemon log requests - #8973

Merged
rzo1 merged 1 commit into
masterfrom
fix/logviewer-daemon-log-file-access
Aug 22, 2026
Merged

Apply the configured log user and group lists to daemon log requests#8973
rzo1 merged 1 commit into
masterfrom
fix/logviewer-daemon-log-file-access

Conversation

@rzo1

@rzo1 rzo1 commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

The daemon log paths combined the daemon flag with the authorizer result as isDaemon || allowed, so the configured logs.users/logs.groups result was discarded for daemon files.

The three daemon log paths (page, download, search) now evaluate the same lists the worker log paths already use. Extends ResourceAuthorizerTest and the three handler tests.

@rzo1 rzo1 added this to the 3.1.0 milestone Aug 19, 2026
@rzo1 rzo1 self-assigned this Aug 19, 2026
@reiabreu

Copy link
Copy Markdown
Contributor

Disclaimer: this comment was drafted with the help of an LLM.

Thanks for tightening up the daemon-log authorization here — the per-endpoint deny tests are a nice addition.

One small suggestion on coverage: both ResourceAuthorizer tests stub getUserGroups(...) to return an empty set, so the group-based path (logs.groups / nimbus.admins.groups) isn't actually exercised. A case where a user is authorized purely via group membership would close that gap. Separately, the three handler tests assert the 403 deny path but not the authorized 200 path — a positive case could guard against accidentally over-blocking.

Neither is a blocker — just an additional safeguard for a security-sensitive change.

@rzo1
rzo1 force-pushed the fix/logviewer-daemon-log-file-access branch from 3748f59 to 471f3e2 Compare August 22, 2026 16:54
@rzo1
rzo1 force-pushed the fix/logviewer-daemon-log-file-access branch from 471f3e2 to 63e31ce Compare August 22, 2026 17:26
@rzo1

rzo1 commented Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

Disclaimer: this comment was drafted with the help of an LLM.

Thanks for tightening up the daemon-log authorization here — the per-endpoint deny tests are a nice addition.

One small suggestion on coverage: both ResourceAuthorizer tests stub getUserGroups(...) to return an empty set, so the group-based path (logs.groups / nimbus.admins.groups) isn't actually exercised. A case where a user is authorized purely via group membership would close that gap. Separately, the three handler tests assert the 403 deny path but not the authorized 200 path — a positive case could guard against accidentally over-blocking.

Neither is a blocker — just an additional safeguard for a security-sensitive change.

Done.

@rzo1
rzo1 merged commit b7865e4 into master Aug 22, 2026
3 checks passed
@rzo1
rzo1 deleted the fix/logviewer-daemon-log-file-access branch August 22, 2026 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants