Skip to content

fix: secure commands and access paths without fleet migration - #273

Draft
vmelikyan wants to merge 10 commits into
mainfrom
security/main-safe-fixes
Draft

vmelikyan wants to merge 10 commits into
mainfrom
security/main-safe-fixes

Conversation

@vmelikyan

@vmelikyan vmelikyan commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Deployment data could reach shell command strings, v1 handlers exposed unauthenticated operations, and log/editor/agent paths lacked complete access checks. This change uses literal arguments, retires v1, requires scoped log and editor grants, and discovers command-based MCP tools within each workspace.

The base is main 976e713a; final head is 37207a27, which adds only checklist clarifications after verified product commit 2f6e9fec. Separate commits preserve the implementation and follow-up history. No environment namespaces, service accounts, RBAC, AWS identities, networks, databases or queues migrate relative to that base. Existing permissions are not revoked by the next Job. API dependencies, lockfile and Helm charts are unchanged.

Changes

  • Preserve native and Codefresh resource targets while replacing selected shell sinks with argument arrays or quoted fixed scripts.
  • Remove /api/v1/**. Protect fresh setup with expiring state, live Secret checks and serialized mutations. Keep configured installations locked. Replace the anonymous banner terminal with an authenticated environment link.
  • Issue log tickets after authentication, repository and pod/container checks. Restrict job-only HTTP listeners to GET /api/health and /api/jobs. Reject worker upgrades and fail closed when production authentication is unavailable.
  • Bind opaque editor grants to the session and owner. Strip Lifecycle credentials upstream and suppress upstream cookies. Preserve valid main-format preview grants with the same key. Deny Kubernetes tools without an authoritative environment namespace.
  • Discover command-based MCP tools for every run. Persist neither workspace catalogs nor discovery errors. Classify workspace tools as MCP writes regardless of annotations. Develop uses existing command and MCP-read permissions for workspace tools, without gaining HTTP/SSE write permission. Develop's workspace admission path applies the strictest applicable command/read/write approval rule, preserves exact-tool precedence, and blocks denied discovery before a process starts. Command-based servers can call remote services. Existing explicit MCP-write definitions retain their write-only approval policy. Read-annotated command tools now follow the workspace approval rules; owner availability and configured default approval modes can affect custom agents.
  • Skip directory symlink aliases during workspace command capture. These aliases previously caused an EISDIR error before approved commands could start. Preserve file aliases within the workspace and existing path checks.

Security scope

This touches 13 of the 22 critical findings. Source-complete candidates: 2, 8, 15, 17, 21. Partial fixes: 4, 9, 11, 12, 13, 16, 18, 19. Deferred: 1, 3, 5, 6, 7, 10, 14, 20, 22. The log-ticket issue is a separate High finding. These classifications do not establish production closure. Aurora helper command execution remains open.

Validation

Final-head CI passed on 37207a27, including lint, coverage tests and production build.

CI on the preceding documentation commit passed on 5ef2210b: frozen dependency install, lint, coverage tests and production build.

  • At API 2f6e9fec: 516 Jest suites and 10,091 tests passed, with 17 tests and 2 live suites skipped. Two snapshots passed. The eight focused policy suites passed 310 tests; runtime TypeScript and ESLint passed. Committed product/test hashes match the full-suite inputs. Jest reported a worker teardown warning. The unchanged gateway passed all 56 tests at 7856eadb; production compilation and CI passed at that revision and the following documentation commit. Fresh runtime compilation and CI at 2f6e9fec passed. The standard TypeScript project is not green: it reports 687 diagnostics, including three stale generated Next types. A scoped baseline overlay under the same dependencies/generated files also reports 687, with none added or removed; this is not a full historical checkout build.
  • At 2f6e9fec, two fresh managed runs in one session tested default and saved MCP selections. Both requested write approval, executed the expected fixture tool, and completed without external_mcp_write in the run plan. Shared and user catalogs stayed unchanged. Deployed source and compiled modules matched the commit. Archive restored the original service; the environment, workspace storage and temporary MCP configuration were removed. Original settings and property absence were restored, caches verified, and test credentials removed. This replay does not independently establish cross-session isolation or fresh discovery on each run.
  • The fresh local production-mode stack onboarded all four test repositories. Real PR events, private/native/Helm deployments, dependencies, Jobs, scheduled Cron executions and completion/error webhooks passed. An intentional job failure produced the expected error path.
  • UI fb030bb1 and CLI d42f7ae streamed live logs against API 7ac03a7e. Ticket denial/expiry, repository restriction, worker/v1 boundaries and actual editor HTTP/WebSocket credential filtering passed at that revision. These checks were not rerun on the later gateway and MCP commits; the affected log/editor source remained unchanged.
  • At 7856eadb, managed MCP default/explicit selections required write approval and returned distinct workspace markers. Fresh discovery, failure rejection and recovery passed without shared/user catalog writes. Fresh configured sandbox and static sessions completed approved edits and shell commands on the verified new gateway image. Chrome verified static approvals. Archive restored PR/static identities and configuration, preserved the fork base, and removed owned storage.
  • PR/static health probes passed all 296 direct-ingress samples during the follow-up control-plane rollout. This excludes DNS and does not prove a production-base upgrade. That earlier round removed eight owned environments and six sessions. Including the fresh focused replay, all nine owned environments were torn down and all seven sessions archived. Temporary settings were restored.

A valid shell-like Git ref failed before clone because existing Octokit URL interpolation changed its braces. Row 19 remains partial. The inherited scheduled-expiry SQL error remains separate: fixing it can activate overdue deletions and requires a backlog review. The local PostgreSQL bootstrap repair is outside this PR.

Rollout gates

Follow the operator checklist and v1/setup guidance. Map the production image to source and compare its migration ledger first.

This remains a draft pending installation-specific approval: inventory v1 and worker consumers, qualify actual AWS/Codefresh/ExternalSecret behavior, rehearse fresh multi-replica setup, and verify mixed-version rollout and the actual workspace backend. Agent/editor E2E used lifecycle_kubernetes; OpenSandbox remains unqualified. The live approval configuration cannot distinguish strictest-of-three from write-only rules; the focused policy tests cover that contract. Deploy the ticket-capable UI before API enforcement. Activate CLI 0.8.0 after compatible issuer/redeemer replicas are ready. Drain every old anonymous listener and socket. Reopen existing editor tabs. Existing application workloads need no permission migration from the pinned base, but control-plane connections can require reconnection.

Opus 5.5 Max reviewed the candidate, the corrective MCP commit, and the fresh replay/cleanup evidence. It confirmed agreement on the narrow candidate for installation-specific release qualification, with no concrete blocker found. The checklist records its policy, gateway-image and production-base caveats. The fresh source/evidence review used signed-in Claude CLI. Claude Desktop then reviewed the verdict and checklist, requested the effective-backend gate, and confirmed that commit 37207a27 resolves that objection. Its closing review did not repeat the source audit. Both agree on installation-specific qualification; neither grants production approval.

Commit and PR text review: passed for scope, behavior and evidence limits. Authored setup/schema/API UI text changed. Source wording and the observed banner/editor/approval flows were reviewed. Full fresh-setup and changed MCP-state browser coverage remains pending.

Matched clients: UI #97, CLI #19. No client package or release tag has been published.

Develop can use selected workspace MCP tools under the configured write policy.
Keep unavailable capabilities and explicit tool rules enforced, and show
unavailable stdio connections accurately. This also permits selected HTTP and
SSE write tools when the same policy allows them.
Keep path checks and file change capture while allowing commands in workspaces with directory links.
Admit workspace command tools through existing command and MCP-read
permissions without granting Develop access to HTTP/SSE write tools.
Keep write classification, enforce the applicable approval rules, and
exclude denied workspace connections before command discovery.

Verify the policy composition, selection, custom-agent restrictions and
existing exact-tool exceptions. Update the operator checklist.
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