Skip to content

Fix project permission bypass#3915

Closed
BunsDev wants to merge 3 commits into
mainfrom
codex/fix-unauthenticated-project-grant-vulnerability
Closed

Fix project permission bypass#3915
BunsDev wants to merge 3 commits into
mainfrom
codex/fix-unauthenticated-project-grant-vulnerability

Conversation

@BunsDev

@BunsDev BunsDev commented Jul 26, 2026

Copy link
Copy Markdown
Member

Motivation

  • Close an authorization gap where HTTP routes and the permission core trusted client-supplied familiarId/proposedBy/claimedHumanApproval values, allowing unauthenticated callers to create/revoke grants or impersonate Supreme and bypass project access checks.

Description

  • Make direct grant mutation routes fail closed by refusing unauthenticated POST/DELETE to /api/project-grants and returning a 403 with a clear error message instead of trusting request bodies (file: src/app/api/project-grants/route.ts).
  • Make grant-proposal creation and resolution HTTP routes fail closed by refusing unauthenticated POST to /api/grant-proposals and PATCH to /api/grant-proposals/[id] until an authenticated human/Supreme approval flow is added (files: src/app/api/grant-proposals/route.ts, src/app/api/grant-proposals/[id]/route.ts).
  • Remove implicit, caller-trusted Supreme identity from permission checks so canAccessProject and filterProjectsForFamiliar require explicit persisted grants (file: src/lib/project-permissions.ts).
  • Update API contract and unit tests to assert the fail-closed behavior and to ensure unauthenticated mutation primitives are no longer exposed (files: src/app/api/api-contracts.test.ts, src/app/api/project-grants/route.test.ts, src/lib/project-permissions.test.ts).

Testing

  • Ran targeted unit/script checks: node --experimental-strip-types src/lib/project-permissions.test.ts, node --experimental-strip-types src/app/api/project-grants/route.test.ts, and node --experimental-strip-types src/app/api/api-contracts.test.ts and they succeeded.
  • Ran type check: pnpm typecheck and it succeeded.
  • Ran the full API test suite: pnpm test:api and all API tests passed.

Codex Task

Copilot AI review requested due to automatic review settings July 26, 2026 17:09
@BunsDev BunsDev self-assigned this Jul 26, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the project-permissions surface by removing implicit “Supreme” trust and making the HTTP routes that could mutate grants/proposals fail closed until a real authenticated approval flow exists.

Changes:

  • Removed implicit Supreme access from canAccessProject/filterProjectsForFamiliar, requiring persisted grants instead.
  • Made /api/project-grants POST/DELETE and /api/grant-proposals POST + /api/grant-proposals/[id] PATCH return 403 fail-closed responses.
  • Updated API contract tests and permission tests to assert the new fail-closed behavior and bootstrap-grant requirement.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/lib/project-permissions.ts Removes implicit Supreme access checks; keeps access purely grant-based and updates audit reasoning.
src/lib/project-permissions.test.ts Updates expectations to require explicit bootstrap grants for Supreme and verifies filtering behavior.
src/app/api/project-grants/route.ts Disables unauthenticated grant mutation endpoints by always returning 403 on POST/DELETE.
src/app/api/project-grants/route.test.ts Updates route-contract assertions to ensure mutation primitives aren’t exposed by the HTTP layer.
src/app/api/grant-proposals/route.ts Disables unauthenticated proposal creation by always returning 403 on POST.
src/app/api/grant-proposals/[id]/route.ts Disables unauthenticated proposal resolution by always returning 403 on PATCH.
src/app/api/api-contracts.test.ts Updates route contracts to reflect that these endpoints no longer parse JSON bodies.
Comments suppressed due to low confidence (1)

src/lib/project-permissions.ts:205

  • assertProjectAccess still loads loadHumanPermissionConfig() solely to pass config.supremeFamiliarId into canAccessProject, but that value is no longer used. This adds unnecessary file/env reads to every permission check; after making the parameter optional, this can just load permissions and call canAccessProject directly.
  const [permissions, config] = await Promise.all([
    loadProjectPermissions(),
    loadHumanPermissionConfig(),
  ]);
  const allowed = canAccessProject(permissions, ctx, projectId, config.supremeFamiliarId);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/lib/project-permissions.ts Outdated
Comment thread src/app/api/project-grants/route.ts
Comment thread src/app/api/grant-proposals/route.ts
@BunsDev

BunsDev commented Jul 26, 2026

Copy link
Copy Markdown
Member Author

@copilot resolve the merge conflicts on this branch.

Copilot AI commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts on this branch.

Conflicts are resolved on this branch. I merged origin/main and finalized the permission-route conflict resolution in 7a30923e (with merge commit a279fb60).

@BunsDev BunsDev closed this Jul 26, 2026
@BunsDev
BunsDev deleted the codex/fix-unauthenticated-project-grant-vulnerability branch July 26, 2026 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants