Skip to content

fix(security): gate GET /approvals/pending behind AdminAuth (#180) - #185

Merged
HongmingWang-Rabbit merged 1 commit into
mainfrom
fix/issue-180-approvals-auth
Apr 15, 2026
Merged

fix(security): gate GET /approvals/pending behind AdminAuth (#180)#185
HongmingWang-Rabbit merged 1 commit into
mainfrom
fix/issue-180-approvals-auth

Conversation

@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor

Summary

Closes #180.

GET /approvals/pending was registered on the open router with no authentication middleware, allowing any unauthenticated caller to enumerate all pending approvals across every workspace on the platform.

  • Fix: added inline middleware.AdminAuth(db.DB) to the route registration in router.go — same one-line pattern used in PR fix(security): #164 #165 #166 — gate 6 unauth routes behind AdminAuth #167 for bundles, events, and PUT /canvas/viewport
  • Scope check: the three workspace-scoped approvals routes (POST/GET /workspaces/:id/approvals, POST /workspaces/:id/approvals/:approvalId/decide) were already correctly behind WorkspaceAuth inside the wsAuth group — no change needed there
  • Auth contract: same lazy-bootstrap fail-open as every other AdminAuth gate — fresh installs with no live tokens pass through; once any workspace has a token, Authorization: Bearer <token> is required

Changed files

File Change
platform/internal/router/router.go Add middleware.AdminAuth(db.DB) inline to r.GET("/approvals/pending", ...)
platform/internal/middleware/wsauth_middleware_test.go Two new regression tests documenting the #180 attack vector

Tests added

TestAdminAuth_Issue180_ApprovalsListing_NoBearer_Returns401  — tokens enrolled, no bearer → 401
TestAdminAuth_Issue180_ApprovalsListing_FailOpen_NoTokens    — fresh install, no tokens → 200 (fail-open preserved)

Full suite: all packages pass (go test ./... -timeout 120s).

Test plan

  • curl http://host:8080/approvals/pending (no auth header) → 401
  • curl -H "Authorization: Bearer <valid-token>" http://host:8080/approvals/pending → 200
  • All existing approvals tests still pass

🤖 Generated with Claude Code

GET /approvals/pending was registered on the open router with no
middleware, allowing any unauthenticated caller to enumerate all pending
approvals across every workspace on the platform.

Fix: add inline middleware.AdminAuth(db.DB) to the route registration,
matching the pattern used in PR #167 for bundles, events, and viewport.

The three workspace-scoped approvals routes (POST/GET /approvals,
POST /approvals/:id/decide) were already correctly behind WorkspaceAuth
inside the wsAuth group — no change needed there.

Tests: two new regression tests in wsauth_middleware_test.go —
  TestAdminAuth_Issue180_ApprovalsListing_NoBearer_Returns401
  TestAdminAuth_Issue180_ApprovalsListing_FailOpen_NoTokens

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@HongmingWang-Rabbit
HongmingWang-Rabbit merged commit 2804513 into main Apr 15, 2026
1 of 7 checks passed
HongmingWang-Rabbit pushed a commit that referenced this pull request Apr 15, 2026
…ht sweep

Captures ~27 PRs merged across both repos this session: security
hardening cluster (#94/#99/#106/#110/#119/#162/#155/#167/#185/#200/#203/
#209/#233), data-integrity fixes (#212/#224/#236), CI runner migration
(#186), platform/scheduler reliability (#95/#149/#207/#206), workspace
runtime features (#205/#208/#198/#216/#225/#235/#231), code-review
follow-ups (#228/#232).

Updated counts: 816 Go (+70), 1180 Python (+40), 453 vitest (unchanged
— UI/a11y patches), 97 jest (unchanged).

CLAUDE.md additions:
- Idle Loop section (#205) under Architectural Patterns
- Admin auth middleware variants section linking docs/runbooks/admin-auth.md
- Migration runner section explaining the .down.sql filter (#212)
- Per-route auth notes in the API table (PATCH field-whitelist, CanvasOrBearer
  on PUT /canvas/viewport, AdminAuth on bundles/events/templates-import/
  approvals-pending/admin-liveness)
- Database section updated with workspace_auth_tokens auto-revoke (#110),
  scheduler.error_detail surfacing (#206), workspace_schedules.last_status
  'skipped' state (#207)

PLAN.md additions:
- New Recently launched (overnight sweep) section with full PR/issue index
- Phase status updated (B–G now complete, H partial)
- Live infrastructure deltas (migration fix, token rotation, legal pages)
- Outstanding items consolidated

Edit-history file expanded from the tick-9 stub to a full session record
covering malware cleanup, CI runner migration, security cluster, data
integrity, infra/feature/code-review batches, and outstanding user
actions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@HongmingWang-Rabbit
HongmingWang-Rabbit deleted the fix/issue-180-approvals-auth branch April 16, 2026 12:31
molecule-ai Bot pushed a commit that referenced this pull request Apr 21, 2026
fix(security): gate GET /approvals/pending behind AdminAuth (#180)
molecule-ai Bot pushed a commit that referenced this pull request Apr 21, 2026
…ht sweep

Captures ~27 PRs merged across both repos this session: security
hardening cluster (#94/#99/#106/#110/#119/#162/#155/#167/#185/#200/#203/
#209/#233), data-integrity fixes (#212/#224/#236), CI runner migration
(#186), platform/scheduler reliability (#95/#149/#207/#206), workspace
runtime features (#205/#208/#198/#216/#225/#235/#231), code-review
follow-ups (#228/#232).

Updated counts: 816 Go (+70), 1180 Python (+40), 453 vitest (unchanged
— UI/a11y patches), 97 jest (unchanged).

CLAUDE.md additions:
- Idle Loop section (#205) under Architectural Patterns
- Admin auth middleware variants section linking docs/runbooks/admin-auth.md
- Migration runner section explaining the .down.sql filter (#212)
- Per-route auth notes in the API table (PATCH field-whitelist, CanvasOrBearer
  on PUT /canvas/viewport, AdminAuth on bundles/events/templates-import/
  approvals-pending/admin-liveness)
- Database section updated with workspace_auth_tokens auto-revoke (#110),
  scheduler.error_detail surfacing (#206), workspace_schedules.last_status
  'skipped' state (#207)

PLAN.md additions:
- New Recently launched (overnight sweep) section with full PR/issue index
- Phase status updated (B–G now complete, H partial)
- Live infrastructure deltas (migration fix, token rotation, legal pages)
- Outstanding items consolidated

Edit-history file expanded from the tick-9 stub to a full session record
covering malware cleanup, CI runner migration, security cluster, data
integrity, infra/feature/code-review batches, and outstanding user
actions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
molecule-ai Bot pushed a commit that referenced this pull request Apr 23, 2026
…bypass + workspace-server Dockerfile GID entrypoint

Three small, non-overlapping fixes extracted from closed PR #1664:

1. canvas/src/components/ContextMenu.tsx — Replace the useMemo-over-nodes
   pattern with a hashed-boolean selector (s.nodes.some(...)) so Zustand's
   useSyncExternalStore snapshot comparison is stable. Resolves React
   error #185 (infinite render loop). Moves the child-node list derivation
   into the delete handler via getState() so the render path no longer
   allocates a fresh array.

2. workspace-server/internal/handlers/a2a_proxy.go — Allow the
   Docker-bridge hostname path (ws-<id>:8000) to skip the SSRF guard in
   local-docker mode. Gated on !saasMode() so SaaS deployments keep the
   full private-IP blocklist (a remote workspace registration can't claim
   a ws-* hostname and reach a sensitive VPC IP).

3. workspace-server/Dockerfile — Add entrypoint.sh that discovers the
   docker.sock GID at boot and adds the platform user to that group, then
   exec's su-exec to drop privileges. Lets the platform container reach
   the host docker socket without running as root.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
molecule-ai Bot pushed a commit that referenced this pull request Apr 23, 2026
…bypass + workspace-server Dockerfile GID entrypoint

Three small, non-overlapping fixes extracted from closed PR #1664:

1. canvas/src/components/ContextMenu.tsx — Replace the useMemo-over-nodes
   pattern with a hashed-boolean selector (s.nodes.some(...)) so Zustand's
   useSyncExternalStore snapshot comparison is stable. Resolves React
   error #185 (infinite render loop). Moves the child-node list derivation
   into the delete handler via getState() so the render path no longer
   allocates a fresh array.

2. workspace-server/internal/handlers/a2a_proxy.go — Allow the
   Docker-bridge hostname path (ws-<id>:8000) to skip the SSRF guard in
   local-docker mode. Gated on !saasMode() so SaaS deployments keep the
   full private-IP blocklist (a remote workspace registration can't claim
   a ws-* hostname and reach a sensitive VPC IP).

3. workspace-server/Dockerfile — Add entrypoint.sh that discovers the
   docker.sock GID at boot and adds the platform user to that group, then
   exec's su-exec to drop privileges. Lets the platform container reach
   the host docker socket without running as root.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
molecule-ai Bot pushed a commit that referenced this pull request Apr 23, 2026
…bypass + workspace-server Dockerfile GID entrypoint

Three small, non-overlapping fixes extracted from closed PR #1664:

1. canvas/src/components/ContextMenu.tsx — Replace the useMemo-over-nodes
   pattern with a hashed-boolean selector (s.nodes.some(...)) so Zustand's
   useSyncExternalStore snapshot comparison is stable. Resolves React
   error #185 (infinite render loop). Moves the child-node list derivation
   into the delete handler via getState() so the render path no longer
   allocates a fresh array.

2. workspace-server/internal/handlers/a2a_proxy.go — Allow the
   Docker-bridge hostname path (ws-<id>:8000) to skip the SSRF guard in
   local-docker mode. Gated on !saasMode() so SaaS deployments keep the
   full private-IP blocklist (a remote workspace registration can't claim
   a ws-* hostname and reach a sensitive VPC IP).

3. workspace-server/Dockerfile — Add entrypoint.sh that discovers the
   docker.sock GID at boot and adds the platform user to that group, then
   exec's su-exec to drop privileges. Lets the platform container reach
   the host docker socket without running as root.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
molecule-ai Bot pushed a commit that referenced this pull request Apr 23, 2026
…bypass + workspace-server Dockerfile GID entrypoint

Three small, non-overlapping fixes extracted from closed PR #1664:

1. canvas/src/components/ContextMenu.tsx — Replace the useMemo-over-nodes
   pattern with a hashed-boolean selector (s.nodes.some(...)) so Zustand's
   useSyncExternalStore snapshot comparison is stable. Resolves React
   error #185 (infinite render loop). Moves the child-node list derivation
   into the delete handler via getState() so the render path no longer
   allocates a fresh array.

2. workspace-server/internal/handlers/a2a_proxy.go — Allow the
   Docker-bridge hostname path (ws-<id>:8000) to skip the SSRF guard in
   local-docker mode. Gated on !saasMode() so SaaS deployments keep the
   full private-IP blocklist (a remote workspace registration can't claim
   a ws-* hostname and reach a sensitive VPC IP).

3. workspace-server/Dockerfile — Add entrypoint.sh that discovers the
   docker.sock GID at boot and adds the platform user to that group, then
   exec's su-exec to drop privileges. Lets the platform container reach
   the host docker socket without running as root.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

MEDIUM: GET /approvals/pending has no authentication — any caller can list all pending approvals

1 participant