Skip to content

fix(acp): harden runtime and workspace boundaries - #360

Merged
Sertaç Özercan (sozercan) merged 10 commits into
acpfrom
acp-f
Aug 13, 2026
Merged

fix(acp): harden runtime and workspace boundaries#360
Sertaç Özercan (sozercan) merged 10 commits into
acpfrom
acp-f

Conversation

@sozercan

Copy link
Copy Markdown
Collaborator

Summary

  • centralize security-critical provider proxy filtering and harden provider/SCM streaming behavior
  • tighten workspace, repository workflow, internal caller, symlink, and tool argument validation
  • add fail-closed workspace admission policies and refresh staged deployment/chart artifacts

Testing

  • git diff --cached --check (passed before commit)
  • focused Go package tests: all targeted packages passed except internal/api
    • TestInternalCallerAuthorizerVerifyNamespace/non_service_account_without_namespace_remains_allowed expects HTTP 204 but now receives HTTP 403

Stack

Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
- grant the chart controller role the admit/use verbs the restored
  workspace admission policies check, and regenerate staging
- remove the 39 branch-added files under deploy/ and charts/orka so the
  promoted snapshots exactly match the released state
- bound symlink chain validation at the kernel resolution depth so
  graph validation stays linear in the number of links
- replace the recursive ** matcher with greedy last-star backtracking
  so allowedPaths matching stays O(pattern x path)
- fail closed for namespace-less internal callers in the remaining
  authorization test and normalize security store timestamps to UTC at
  the store boundary

Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
…nt mode

Repository-monitor read-only presets now translate per runtime into the
exact surface each built-in runtime can enforce: claude projects the
{Glob, Grep, Read} allowlist (the path-scoped preset previously failed
session configuration), opencode keeps its Read/Glob permissions, and
codex maps the same surface onto its upstream read-only agent mode,
whose kernel-enforced sandbox confines every command to reads with no
network access while the controller rejects all elevation requests.

Codex reviewers are admitted again by the monitor API and reconciler,
and read-only agent task validation accepts codex; any other restricted
codex tool policy still fails closed, as does the read-only surface on
write-intent sessions.

Also extracts create_agent_task argument parsing helpers to satisfy
gocyclo and wraps the egress proxy copyBuffered signature for lll.

Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Read tasks without Bash now carry the restricted {Read,Glob,Grep} tool
policy for every provider, so the nightly and dispatched live ACP e2e
executes codex's native read-only agent mode instead of skipping the
restricted path. The blocking timeout and cancellation tasks keep the
codex carve-out because codex cannot express restricted policies that
include Bash.

Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
…less successes

Live ACP validation caught two undeletable-Task bugs in prompt attempt
reclamation:

- terminal validation compared the per-prompt Session mutation-lease
  generation against the RuntimeSession incarnation generation, which
  only coincide for a session's first prompt, so every continuation
  Task wedged forever on its cleanup finalizer
- completeSuccessWithDelivery emitted only the sparse terminal
  execution classification, so every successful sessionless Task failed
  the projection identity checks at reclamation

The finalized SessionTurn now binds to the Task by session UID with
lease-generation fencing against the PromptAttempt, and success
projections carry the complete frozen execution identity with volatile
fields cleared so retried settlements stay byte-identical.

The live kind e2e also now passes end to end for the first time: the
validator adopts the harness-v2 watch namespace in shared mode (the
isolated controller only serves Tasks there), stamps its namespace with
the controller-mode claim, probes chat completions with
max_completion_tokens, supports publishing a local Vekil build through
the run registry ahead of the Vekil deploy, and pins Vekil v0.14.1,
whose direct-bearer fallback accepts fine-grained Copilot PATs
noninteractively.

Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Codex's own sandbox is bubblewrap, which needs unprivileged user
namespaces the runtime Pod forbids, so selecting the upstream read-only
agent mode made every command fail and only the release-gate result
content check caught it: reads never executed. Read-only sessions now
keep the orka-external agent mode and rely on the boundary that
actually holds in-Pod: safe read commands execute, every elevation
request is rejected by the controller, file writes are mediated by the
supervisor, and the read-intent workspace delta classification fails
any modifying turn. Comments and docs no longer claim kernel
enforcement.

Also fixes two shared watch-namespace gates the release gate exposed:
write-task settlement for remote cleanup and failure diagnostics both
required a validator-created namespace and skipped the adopted
orka-system namespace.

Live release-gate evidence: codex read results now carry independently
verified repository content, and the clean-room publication chain
(branch push to a distinct fork, PR creation, exact-head verification)
completed with delivery VerifiedExact.

Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
…evidence

Tasks without a repository workspace freeze the protocol-only "empty"
revision in their projected delivery evidence, while the outbox projector
strips that value before the schema-validated Task status. Reclamation
compared the two shapes byte-for-byte, so every no-workspace Task with
delivery evidence became undeletable once its finalizer ran. Compare
through the projector's normalization and pin the protocol constant in
taskterminal so both sides stay coupled.
Recovery scenarios run apply-acp-production.sh twice into one shared
fake-kubectl log, so whole-log ordering asserts compared the recovery
run's dependency rollouts against the aborted run's endpoint waves and
failed for every post-dependency crash scenario. Scope the phase-order
asserts to the final converged invocation, which the aborted run's
state-file guards already fence.
Repository-monitor reviewers must omit spec.secretRef because provider
credentials come from the controller-managed runtime proxy; the CLI
security/monitor suite still created its Claude reviewer with a local
credential Secret and failed monitor creation.
@sozercan
Sertaç Özercan (sozercan) marked this pull request as ready for review August 13, 2026 19:36

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1d0cdff579

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

Comment thread internal/controller/acp_agent_configuration.go
Comment thread charts/orka/values.yaml
Comment thread scripts/live-acp-runtime-kind-e2e.sh
The kind wrapper now adopts the shared orka-system namespace by default,
but the smoke-mode EXIT handler only invoked delete_test_namespace_now
for namespaces the run created, so shared runs leaked their run-scoped
Tasks, Agents, and RuntimePools on retained clusters. Route shared
namespaces through the existing shared-mode cleanup branch, which
removes run-labeled resources and leaves the namespace in place.
@sozercan
Sertaç Özercan (sozercan) merged commit 2e41031 into acp Aug 13, 2026
27 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