ROX-37613: recheck permissions for cached global metrics by parametalol · Pull Request #23442 · stackrox/stackrox

parametalol · 2026-10-15T13:25:14Z

Description

Global metric registries cache results across identities, so datastore authorization alone cannot protect later scrapes. Check the current caller's read permissions before gathering and filter cached global metric families per request. Certificate expiry metrics require Administration read access because their underlying service permits any authenticated user.

User-facing documentation

CHANGELOG.md is updated OR update is not needed
documentation PR is created and is linked above OR is not needed

Testing and quality

the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
CI results are inspected

Automated testing

How I validated my change

go test ./central/metrics/custom -run 'TestRunner_(ServeHTTP|GlobalMetricsCheckCurrentPermissions)$' -count=1 (passed).
Added coverage for cached metric visibility for admin, workflow-only, no-access, and anonymous requests. CI remains to be inspected.

Global registries retain results across identities; datasource authorization alone cannot protect a subsequent scrape. Filter shared metric families per request using the affected tracker's required read permission, including platform-wide certificate expiry. Request: 'missing global tracker permission check is a bug, and needs to be addressed separately!' Follow-up: 'the fix for the cached global-metric permission checks needs its own Jira bug record ... use the Jira ID in the according branch name.' AI-assisted: code partially generated by AI.

openshift-ci · 2026-10-15T13:25:18Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

coderabbitai · 2026-10-15T13:29:24Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Advanced
Run ID: 91be38ce-df54-41d2-b645-109277182dbe

📥 Commits

Reviewing files that changed from the base of the PR and between b917020 and a08becf.

📒 Files selected for processing: 6

central/metrics/custom/administrative_events/tracker.go
central/metrics/custom/expiry/tracker.go
central/metrics/custom/policies/tracker.go
central/metrics/custom/runner.go
central/metrics/custom/runner_test.go
central/metrics/custom/tracker/tracker_base.go

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Summary

Summary by CodeRabbit

Bug Fixes
- Metrics are now shown only when the requesting identity has the required view access. Platform-wide administrative and expiry metrics require administration access, while policy metrics require workflow administration access.
- Requests without the necessary access—including anonymous requests—no longer receive restricted global metrics. Authorized requests see only the metrics permitted for their access level.

Custom metrics trackers now declare view permissions. The metrics runner checks tracker access for each request and limits global metrics to prefixes associated with viewable trackers.

Changes: Custom metrics access control

Layer / File(s)	Summary
Tracker permissions and resource requirements `central/metrics/custom/tracker/tracker_base.go`, `central/metrics/custom/administrative_events/tracker.go`, `central/metrics/custom/expiry/tracker.go`, `central/metrics/custom/policies/tracker.go`	`Tracker` adds `CanView` and `IsGlobal`. `TrackerBase` implements these methods and adds `RequireView`. The administrative events and expiry trackers require `Administration` view access. The policies tracker requires `WorkflowAdministration` view access.
Request-scoped metric filtering and tests `central/metrics/custom/runner.go`, `central/metrics/custom/runner_test.go`	`ServeHTTP` starts only viewable trackers and filters global metric families by permitted prefixes. Tests check metric output for admin, workflow-only, no-access, and anonymous identities.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Request
  participant Runner
  participant Tracker
  participant GlobalRegistry
  Request->>Runner: request with identity context
  Runner->>Tracker: CanView(context)
  Tracker-->>Runner: permission result
  Runner->>GlobalRegistry: gather permitted metric families
  Runner-->>Request: filtered metrics response

Merge Risk: ⚪ Minimal · up to a08be

The permission filtering has no identified merge-blocking issue. The change is ready to merge after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 6 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly identifies the main change: rechecking permissions for cached global metrics.
Description check		The description explains the authorization issue, the implementation, and the validation performed. CI inspection remains unchecked, but the description is otherwise sufficiently complete.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

_{Comment @coderabbitai help to get the list of available commands.}

codecov · 2026-10-15T13:30:46Z

Codecov Report

❌ Patch coverage is 62.50000% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.97%. Comparing base (b917020) to head (a08becf).

Files with missing lines	Patch %	Lines
central/metrics/custom/tracker/tracker_base.go	0.00%	10 Missing ⚠️
central/metrics/custom/runner.go	90.90%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #23442      +/-   ##
==========================================
- Coverage   52.00%   51.97%   -0.04%     
==========================================
  Files        2912     2912              
  Lines      183385   183415      +30     
==========================================
- Hits        95377    95327      -50     
- Misses      79680    79739      +59     
- Partials     8328     8349      +21

Flag	Coverage Δ
go-unit-tests	`51.97% <62.50%> (-0.04%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

github-actions · 2026-10-15T13:33:03Z

🚀 Build Images Ready

Images are ready for commit a08becf. To use with deploy scripts:

export MAIN_IMAGE_TAG=5.1.x-278-ga08becf09e

parametalol added the ai-assisted label Oct 15, 2026

openshift-ci Bot added the do-not-merge/work-in-progress label Oct 15, 2026

github-actions Bot added the area/central label Oct 15, 2026

Navigation Menu

Repository navigation

ROX-37613: recheck permissions for cached global metrics - #23442

ROX-37613: recheck permissions for cached global metrics#23442
parametalol wants to merge 1 commit into
masterfrom
michael/ROX-37613-global-metrics-authorization

parametalol commented Oct 15, 2026

Uh oh!

openshift-ci Bot commented Oct 15, 2026

Uh oh!

coderabbitai Bot commented Oct 15, 2026

Summary by CodeRabbit

Sequence Diagram(s)

Uh oh!

codecov Bot commented Oct 15, 2026

Uh oh!

github-actions Bot commented Oct 15, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

parametalol commented Oct 15, 2026

Description

User-facing documentation

Testing and quality

Automated testing

How I validated my change

Uh oh!

openshift-ci Bot commented Oct 15, 2026

Uh oh!

coderabbitai Bot commented Oct 15, 2026

Summary by CodeRabbit

Sequence Diagram(s)

Uh oh!

codecov Bot commented Oct 15, 2026

Codecov Report

Uh oh!

github-actions Bot commented Oct 15, 2026

🚀 Build Images Ready

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant