fix(hub): restore SECURITY-GATE CheckAccess in getProject handler - #1391
Open
ptone wants to merge 4 commits into
Open
fix(hub): restore SECURITY-GATE CheckAccess in getProject handler#1391ptone wants to merge 4 commits into
ptone wants to merge 4 commits into
Conversation
…rship-based Policy access control (#1414) * refactor: eradicate dead project visibility field Remove the project `Visibility` field and all references to it. This field was accidentally re-introduced by a stale-branch merge (PR #330, commit 2b71d8c) after being deliberately removed in PR #332 (commit 056e8f6). It round-trips through the DB and API but zero authorization or filtering logic reads it. Changes: - Remove visibility field from ent schema (project.go), triggering codegen removal of all generated accessors. The DB column is left orphaned per the repo's established WithDropColumn(false) pattern. - Remove Visibility from store.Project struct, ProjectFilter, ProjectToProjectInfo conversion, and all entadapter read/write/ backfill/filter paths. - Remove visibility from CreateProjectRequest, UpdateProjectRequest (both hub handlers and hubclient), register path, clone path. - Remove --visibility CLI flag from hub project create commands. - Remove visibility selector UI from project-create.ts and hardcoded 'private' from onboarding.ts project creation bodies. - Update visibility constants doc comment to reflect they are for skills/templates/harness configs only (not projects). - Update tests: remove visibility assertions from clone tests, remove visibility filter test case from store domain test, remove visibility from test helpers. The VisibilityPrivate/Team/Public constants in api/types.go and store/models.go are retained — they are actively used by the skills and template systems. Project access control will be expressed through the existing role and role-binding model (per-project members group + hub-members group) in a follow-up commit. * feat(hub): implement membership-based project access control Replace the wildcard hub-member-read-all policy with per-type read policies, so that project and agent reads are gated by per-project membership. This is the net-new replacement for the dead visibility field removed in the previous commit. Changes: - seed.go: narrow hub-member-read-all wildcard into explicit per-type read policies for non-project resources (user, group, template, harness_config, broker, runtime_broker, gcp_service_account, policy, skill, quota, role, role_binding). Project/agent reads now require membership in the project's members group. - handlers_projects_core.go: add ensureProjectMemberReadPolicy() which creates read+list policies for project/agent resources scoped to each project and bound to the project's members group. Add CheckAccess gate on getProject handler (SECURITY-GATE marker). - project-settings.ts: update Members card description to hint about adding hub-members group for everyone-visibility. - project_visibility_test.go: 12 new tests covering member read, non-member denial, unauthenticated denial, list filtering, hub-members group transitive visibility, policy narrowing, and end-to-end project creation access control. * test: remove dead Visibility field from test fixtures and update policy assertions Remove Visibility from store.Project{} struct literals across 42 test files after eradicating the field from the schema. Update policy-related test assertions to reflect narrowed hub-member-read-all (per-type policies instead of wildcard) and membership-based project access control. * fix(hub): address review findings R1-R3, N1-N3, and CI test failures R1: Add backfillProjectMemberReadPolicies startup function that ensures per-project member-read policies exist for all existing projects, preventing lockout of regular members after narrowing hub-member-read-all. Extract standalone ensureProjectMemberReadPolicies in seed.go so both the startup backfill and inline handler path share one impl. R2: Fix staticcheck QF1008 lint violations — use embedded field selector (resp.ID instead of resp.Project.ID) in project_visibility_test.go. R3: Add slug migration for member-read-project and member-read-agent policies in migrateProjectSlug, matching the existing pattern for member-create-agents. CI: Add 'hub' to per-type policy list — the wildcard previously covered resource type "hub" used by admin route guards (server-config, diagnostics, etc). Update seed_tombstone_test.go to use per-type policy names instead of deleted hub-member-read-all. Update scoped_admin_test.go comments. Fix policy count assertion (14 seeded). N1: Fix seed.go comments about broker (stays hub-wide, not per-project). N2: Update design note to match implementation (only project+agent gated). N3: Remove dead visibility field from TypeScript Project interface. * fix: remove dead visibility column from fixture generator project seed The projects table no longer has a visibility column after eradication. The fixture generator's project row still referenced it, causing TestFixtureCoverage, TestFixtureLoadable, and TestFixtureDeterministic to fail with "column visibility does not exist". * Update pkg/hub/handlers_projects_core.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --------- Co-authored-by: Scion Agent (proj-visibility-dev) <agent@scion.dev> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Scion Agent (du-0829) <agent@scion.dev>
Co-authored-by: Scion Agent (changelog-daily) <agent@scion.dev>
Move the createProjectGroup backfill call from before the SECURITY-GATE CheckAccess block to after it, alongside createProjectMembersGroupAndPolicy. CheckAccess doesn't depend on anything createProjectGroup sets up, so the write should not run before authorization is verified.
ptone
force-pushed
the
scion/proj-visibility-gate-fix
branch
from
August 30, 2026 12:03
8eea9a5 to
116193b
Compare
Owner
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SECURITY-GATE: CheckAccessblock ingetProjectthat was dropped during the merge of PR feat: complete group management admin UI/UX #1414createProjectGroupandcreateProjectMembersGroupAndPolicyrun after authorization, not before (write-before-auth anti-pattern)Without this gate, any authenticated user can read any project regardless of membership — the per-project membership access control added in #1414 is bypassed.
Test plan
go build ./...clean13ac6cfonscion/proj-visibility-dev, whichproj-visibility-revvalidated