Skip to content

[SEC] MCP attempts, delivery SSRF, transcript read and log hygiene - #124

Merged
ustoppble merged 8 commits into
mainfrom
sec-mcp
Nov 4, 2026
Merged

ustoppble merged 8 commits into
mainfrom
sec-mcp

Conversation

@ustoppble

Copy link
Copy Markdown
Owner

Security fixes from the Codex audits (MCP, delivery, logs)

One change per finding, each with tests first.

  1. HIGH — member opens an attempt in the admin's mission. mission_attempt_start now requires canChangeMission; a member who cannot change the mission gets the same NOT_FOUND as a mission that is not there.
  2. HIGH — member forges usage / closes the admin's attempt. mission_report_usage requires canChangeMission and that the caller is the token that opened the attempt, or an admin. Same "not found" answer otherwise. Migration 0053_mission_attempt_opener adds mission_attempt.opened_by_token_id (nullable, no backfill: existing attempts are admin-only).
  3. LOW — /account/agent names an unlinked project. The name now comes from projectScope(principal); with no linked project the page falls back to the workspace name.
  4. MEDIUM — SSRF through repo_url. Delivery verification only runs git on an https: remote without credentials whose DNS answers are all public; git is pinned to the resolved address (http.curloptResolve), protocol.allow=never + https only, no redirects, no credential helper, scrubbed environment. file:/path remotes are local checkouts and are not checked; git:, ssh:, http:, ext:: and private/loopback/link-local destinations are refused. The repo_url schema (project_create / project_update) rejects ext::, git:, http:, ftp: and credentials; file:// stays valid because card routing matches local checkouts by it.
  5. MEDIUM — arbitrary read through the transcript path. The path must be a regular file (not a symlink, device or FIFO), under 128 MiB, whose realpath sits inside a known transcript folder (~/.claude, ~/.codex, ~/.grok, ~/.gemini, ~/.cursor, opencode, ~/.overclock-app, CLAUDE_CONFIG_DIR, plus the operator's OVERCLICK_TRANSCRIPT_DIRS). The model identity is read streaming, opened with O_NOFOLLOW.
  6. MEDIUM — raw errors in logs. A failed tool call, migrate and seed log only operation, error type, driver code and a short ref (also returned to the caller); no driver message, SQL, params or stack in production.

Verification

  • pnpm typecheck, pnpm build, pnpm test green (mcp-core 179, db 181, web 1620; 3 real-Postgres files skipped as before).
  • pnpm db:generate after the migration: "No schema changes".

Notes for the reviewer

  • Deployment: transcripts outside the default folders need OVERCLICK_TRANSCRIPT_DIRS.
  • The web actions that write repoUrl directly (actions/projects.ts, actions/onboarding.ts) are outside this change; verification enforces its own allowlist regardless of what is stored.
  • Not merged, not deployed.

🤖 Generated with Claude Code

ustoppble and others added 8 commits November 4, 2026 17:23
…and only the opening token or an admin reports on it

Migration 0053 records the token that opened an attempt; null is admin-only.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… not the first project of the organization

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…inned and stripped of credentials; repo_url refuses ext::, git:, http: and credentials

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…egular file under a size cap, streamed read; repo_url schema without the URL global

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… type, code and a ref; never the driver message, SQL, params or stack

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…r folder

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…OVERCLICK_DEBUG_RAW_ERRORS, never in production

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@ustoppble
ustoppble merged commit dd972b3 into main Nov 4, 2026
2 checks passed
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.

1 participant