Skip to content

fix(cloud): align managed dashboard visibility with project grants - #1609

Merged
dnlrsls merged 1 commit into
Gentleman-Programming:mainfrom
dnlrsls:fix/managed-dashboard-projects-1599
Oct 2, 2026
Merged

dnlrsls merged 1 commit into
Gentleman-Programming:mainfrom
dnlrsls:fix/managed-dashboard-projects-1599

Conversation

@dnlrsls

@dnlrsls dnlrsls commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

Post-merge validation update: #1609 merged at 2026-10-02T00:48:42Z with head b79dfd33. The supplemental PostgreSQL tests in 31f3774f were published to the fork afterward and are NOT part of this merged PR. Test results below include that supplemental validation; integration of the new test file requires a separate PR.

🔗 Linked Issue

Closes #1599

🏷️ PR Type

  • type:bug — Bug fix
  • type:feature — New feature
  • type:question — Question requiring tracked work
  • type:docs — Documentation only
  • type:refactor — Code refactoring
  • type:chore — Maintenance
  • type:breaking-change — Breaking change

📝 Summary

  • Keep the shared dashboard read-model cache permission-neutral; apply managed grants and legacy deployment scopes independently.
  • Include registered projects without synced content as zero-count inventory rows, preserving existing synced projects.
  • Preserve deny-by-default managed views and legacy allowlist behavior; document the credential contract.

📂 Changes

File Change
internal/cloud/cloudstore/dashboard_queries.go Neutral SQL/cache inputs, independently scoped views, registered empty projects and registry error propagation
internal/cloud/cloudstore/dashboard_principal_test.go Outside-env grants, empty registrations, sequential isolation, legacy exclusions/wildcard and controls regressions
DOCS.md Managed/legacy authorization and project inventory contract

🧪 Test Plan

  • RED observed before production edits: sequential-principal regression failed because project-b was absent.
  • go test ./internal/cloud/cloudstore -run 'TestDashboardPrincipalScope|TestDashboardReadModelScopedFiltersAllowlistAcrossSurfaces|TestDashboardQuerySurfacesReuseCachedReadModelUntilInvalidated|TestSetDashboardAllowedProjectsInvalidatesCachedReadModel' — passed.
  • go test ./internal/cloud/cloudserver -run 'TestDashboardPrincipalHTTPRequestsUseScopedStoresAndFailClosed' — passed.
  • go test ./internal/cloud/cloudstore ./internal/cloud/cloudserver — passed; PostgreSQL-backed tests unavailable without test DSN.
  • Independent spot check: go test ./internal/cloud/cloudstore -run 'TestDashboardPrincipalScope' -count=1 -v — 5 tests passed, no failures/skips.
  • Independent HTTP spot check: go test ./internal/cloud/cloudserver -run 'TestDashboardPrincipalHTTPRequestsUseScopedStoresAndFailClosed' -count=1 -v — 1 test passed, no failures/skips.
  • git diff --check — no whitespace errors; LF-to-CRLF warnings only. Documentation and scope boundaries read back.
  • PostgreSQL 18.6 disposable localhost cluster: CLOUDSTORE_TEST_DSN=<isolated test DSN> go test ./internal/cloud/cloudstore -run '^TestDashboardRegistryPostgres' -count=1 -v — four tests passed, zero skips; independently repeated with the same outcome (1.104s).
  • Same isolated DSN with focused -count=2 and go test ./internal/cloud/cloudstore ./internal/cloud/cloudserver -count=1 — passed. Aggregate package skip counts were not captured.
  • Without DSN, the four new tests skip intentionally. CI does not configure PostgreSQL, so CI green is not evidence these gated tests ran.
  • Owned PostgreSQL stopped after tests: pg_ctl confirmed no server; port free; logs retained. Existing schema cleanup ignores DROP errors; residual schema absence was not independently queried.
  • Real browser/manual deployment reproduction — not run; fixture/store, HTTP and PostgreSQL tests are the local evidence.

🤖 Automated Checks

GitHub CI is pending. Local results do not establish CI outcomes.

Check Status
Check Issue Reference Pending
Check Issue Has status:approved Pending
Check PR Has type:* Label Pending
Check PR Has No Transient Artifacts Pending
Unit Tests Pending
E2E Tests Pending
Plugin Tests Pending
Lint Pending
Windows Setup Test Pending
Cloud Sync Wrapper Tests (Windows) Pending

✅ Contributor Checklist

  • Linked approved issue fix(cloud): managed principals see an empty project list in the dashboard #1599.
  • Exactly one type:* label: type:bug.
  • Recorded focused regression and affected package commands/outcomes.
  • Recorded additional local checks and missing CI/database evidence.
  • Updated docs with behavior change.
  • Conventional commit; no Co-Authored-By trailers.
  • All three merged paths and the separately published test file comply with the Transient Artifact Policy; local task state is not included.

💬 Notes for Reviewers

Native review review-2607c757fe12086b approved this 162-line slice and its exact acknowledgement completed. The registration-refresh evidence gap was subsequently covered by the PostgreSQL test-only slice in 31f3774f, independently verified and approved by native review review-08b9b8b15363a9e0 with exact acknowledgement completed. The test-only slice did not introduce a new production RED/GREEN cycle. Managed sync and grant-management permissions were not changed. Existing project-control cache invalidation is preserved.

Rollback boundary: revert b79dfd33 to restore prior dashboard behavior and documentation; 31f3774f is a separate test-only work unit.

Summary by CodeRabbit

  • Features
    • Dashboard project inventory, details, statistics, browser views, and sync controls now reflect the principal’s project grants. No grants show no projects; wildcard access must be explicitly granted.
    • Registered projects with no synced content appear with zero counts, and changing project sync controls refreshes dashboard inventory.
    • Explicit grants can show projects outside the legacy deployment scope, while legacy access remains limited to that scope.

Keep the shared dashboard cache permission-neutral, preserve legacy deployment scopes, and include registered empty projects.
@dnlrsls dnlrsls added the type:bug Bug fix label Oct 2, 2026
@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (1)
skills/architecture-guardrails/SKILL.md — Agent Skill

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 27974dd3-ba01-4b06-b416-a170f6f37f48

📥 Commits

Reviewing files that changed from the base of the PR and between 5b8e718 and b79dfd3.

📒 Files selected for processing (3)
  • DOCS.md
  • internal/cloud/cloudstore/dashboard_principal_test.go
  • internal/cloud/cloudstore/dashboard_queries.go

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


📝 Walkthrough

Walkthrough

Managed dashboard views now use principal grants without intersecting them with deployment scopes. The dashboard read model also includes registered projects that have no synced content. Ordinary readers remain scoped by deployment settings.

Changes

Managed dashboard visibility

Layer / File(s) Summary
Neutral read model and registered projects
internal/cloud/cloudstore/dashboard_queries.go
The shared read model loads without deployment scoping. It adds registered projects with no content as empty project details and statistics rows.
Grant-scoped dashboard reads
internal/cloud/cloudstore/dashboard_queries.go, internal/cloud/cloudstore/dashboard_principal_test.go, DOCS.md
Managed dashboard views use principal grants, including grants outside deployment scopes. Chunk and mutation queries no longer apply deployment-scope filters to unscoped reads. Tests cover managed and legacy access, empty registered projects, and scoped dashboard data. The documentation describes grant-based managed access and project registration.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: gentleman-programming, alan-thegentleman

Merge Risk: ⚪ Minimal · up to b79df

Managed dashboard visibility follows project grants, including registered empty projects, while legacy access remains deployment-scoped. No merge-blocking issue remains established; merge after normal checks pass.

Security Architecture Review

Security architecture risk: 🔵 Low · up to b79df

Managed credentials can now read granted projects outside deployment allowlists. Empty grants remain deny-by-default, and privileged mutations retain administrator checks. No authorization bypass was established, but grant-lifecycle and database-backed failure coverage remain incomplete.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — A compromised managed dashboard credential remains bounded by its resolved grants for project reads, but those grants are no longer capped by the deployment allowlist. An explicitly wildcard-granted credential can read every project represented in the connected CloudStore inventory. The maximum read exposure therefore spans that store, not merely its legacy deployment scope.

Security Findings and Attack Paths

  • observed — The hypothesized empty-scope widening is not a demonstrated managed authorization bypass: empty managed grants are explicitly denied, and unrestricted handling of an empty legacy scope already existed in the base row loaders and model filter.

Trust Boundaries and Controls

  • observed — Dashboard credentials cross into managed identity through signed-session validation and managed-principal revalidation. Scoped read routes establish that session before selecting a request store. The grant-authorizer adapter obtains project names by principal ID from the grant store; the backing grant-store implementation remains outside the completed inspection.
  • observed — Sync-control POSTs require a dashboard session and independently reject non-administrators before calling the configured base store. Project-grant membership alone is therefore not sufficient to reach this HTTP mutation sink.

Resilience and Maintainability Implications

  • observed — Project details contain nested slice aliases rather than fully detached snapshots. The same scoped filtering and detail-return behavior existed before this PR, and no attacker-reachable consumer mutation was established. This remains an in-process ownership limitation, not a demonstrated new cross-project attack.

Hardening Proposals

  • proposed — Before introducing mutable consumers of project-detail results, detach nested slices or expose read-only representations so callers cannot alter the shared cached snapshot.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning 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 6 functions across 2 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: aligning managed dashboard visibility with project grants.
Linked Issues check ✅ Passed The changes address the coding requirements in issue #1599. Managed dashboard views use the permission-neutral read model and filter it by explicit principal grants. The legacy deployment scope remain…
Out of Scope Changes check ✅ Passed The changed query code, regression tests, and DOCS.md updates support issue #1599. The tests verify the required scope isolation and cache behavior. No unrelated product behavior or unrelated files ar…
Full details: Docstring Coverage

Explanation

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 6 functions across 2 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@dnlrsls
dnlrsls added this pull request to the merge queue Oct 2, 2026
Merged via the queue into Gentleman-Programming:main with commit e48fac2 Oct 2, 2026
29 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(cloud): managed principals see an empty project list in the dashboard

1 participant