fix(agent): close opencode env-guard shell wrapper bypass (PEN-1305)

[kkroo]

Thinking Path

PEN-1305 shipped in layers: server-side transcript redaction, the claude adapter pre-execution block, and opencode permission.bash deny globs. The glob layer cannot catch wrapped or chained dump forms, so the opencode adapter added a canary-gated tool.execute.before env-guard plugin. This PR updates Paperclip's production adapter pin to include that guard, then carries the follow-up review fix that parses the actual shell -c command string before positional arguments.

Linked Issues or Issue Description

Refs PEN-1305.
Related adapter work: kkroo/paperclip-adapter-opencode-k8s#52.

What Changed

• Dockerfile: moved OPENCODE_K8S_REF to 83197d46b0784c941801165464d48aca1b979909.
• The pinned adapter now blocks sh -c env ignored and bash -c "env" ignored by parsing the command-string argument separately from shell positional arguments.
• server/src/agent-shell-guard.ts: mirrored the same shell-wrapper parsing behavior in Paperclip's in-repo guard.
• Tests now cover quoted, unquoted, nested, and positional-argument shell wrappers, plus helper-plus-dump chains.

Verification

• Adapter repo: npm test -- src/server/env-guard-plugin.test.ts passed, 90/90.
• Adapter repo: npm run typecheck passed.
• Adapter repo: npm run build passed.
• Paperclip: pnpm exec vitest run server/src/__tests__/agent-shell-guard.test.ts server/src/__tests__/docker-opencode-runtime-pin.test.ts --reporter=dot passed, 31/31.
• Paperclip: pnpm --filter @paperclipai/server typecheck passed in the fix(agent): close opencode env-guard shell wrapper bypass (PEN-1305) #1027 worktree.

Risks

• Default-off flag: fleet behavior remains unchanged until an agent's adapter config enables envGuardPlugin.
• Canary risk is bounded by the adapter's fail-open install/classification path and by server-side transcript redaction as a backstop.
• The in-repo guard now blocks helper-plus-dump chains before applying the safe-helper allowlist.

Model Used

Claude Fable 5 (claude-fable-5) via Claude Code, plus Codex GPT-5 for the follow-up review fix.

Rollout plan

1. Image roll after master merge.
2. Enable envGuardPlugin: true on one opencode agent.
3. Probe wrapped dump forms and confirm the PEN-1305 block message appears without wedging the run.
4. Follow up to enable the guard fleet-wide after the canary is clean.

Checklist

• I have included a thinking path that traces from project context to this change
• I have specified the model used (with version and capability details)
• I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
• I have searched GitHub for duplicate or related PRs and linked them above
• I have either linked existing issues with Fixes: # / Closes # / Refs # or described the issue in-PR
• I have run tests locally and they pass
• I have added or updated tests where applicable
• I have considered and documented any risks above

[allyblockcast]

🔗 Paperclip issue: PEN-1305

[allyblockcast]

🔗 Paperclip issue: PEN-1305

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[allyblockcast]

Hey @kkroo! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• Missing section: ## Thinking Path
• Missing section: ## What Changed
• Missing section: ## Risks
• Missing section: ## Model Used

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

[allyblockcast]

Hey @kkroo! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• Missing section: ## Verification

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

[kkroo]

@ally please review exact head de84c7e3984a045d0f8b6da05e4feeb258abda4a, focusing on the opencode_k8s pin bump, default-off envGuardPlugin rollout safety, and whether the Dockerfile/test pin move preserves PEN-1305 canary gating.

[allyblockcast]

Approved exact head de84c7e after inspecting the two-file opencode_k8s pin/test update, confirming the envGuardPlugin rollout remains default-off/canary-gated, and finding no unresolved review threads. Current CI is still running; auto-merge should wait on it.

Superseded at de84c7e: the consolidated App review found unresolved Important findings on this exact head.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: de84c7e

Important Issues (3)

• [gstack/review] Dockerfile:347 — The canary cannot be reliably rolled back for durable workspace-isolation sessions. The pinned adapter writes paperclip-env-guard.js under XDG_CONFIG_HOME, which resolves below the persistent sessionRoot for workspace isolation, but the envGuardPlugin=false branch performs no cleanup. After enabling one canary, turning the flag off can therefore leave the auto-discovered plugin active on later runs.
  • Make the disabled branch remove the generated plugin/helper, or place them in per-Job ephemeral storage, and add an enabled-to-disabled lifecycle test against a pre-populated session root.
• [pr-review-toolkit] Dockerfile:347 — The pinned guard’s safe-helper exception bypasses dump detection for compound commands. classifyAgentShellCommand returns allow as soon as SAFE_ENV_INSPECTION_RE matches anywhere, before testing FULL_ENV_DUMP_RE, so commands such as paperclip-safe-env && printenv or env; ./scripts/safe-env-inspect.mjs are allowed.
  • Restrict the exception to a complete helper-only command or classify each shell segment, then add combined helper-plus-dump cases to the shared corpus and literal-plugin tests.
• [native-codex] Dockerfile:347 — The new plugin still misses valid unquoted wrapper forms such as sh -lc env and bash -c printenv. SHELL_WRAPPER_RE requires matching quotes around the payload, and the dump regex does not match while the wrapper prefix remains, despite wrapped dumps being the stated reason for this pin.
  • Recognize unquoted -c payloads and add them to both the classifier corpus and literal tool.execute.before artifact tests before canarying.

Strengths

• The new immutable SHA is a direct child of the prior pin, and the Docker build checks out that exact commit.
• Fresh configurations default envGuardPlugin off, with focused adapter tests for the clean-state off/on manifest branches and plugin installation ordering.
• The adapter tests execute the literal generated plugin and cover loader shape, malformed hook payloads, base64 round-tripping, and fail-open behavior.

Recommended Action

1. Fix the three Important rollout/security gaps before merge.
2. Extend the adapter corpus with rollback and adversarial compound/wrapper cases.
3. Re-pin Paperclip to the corrected adapter commit and move the lockstep assertion with it.

[allyblockcast]

Reviewed current head; no active unresolved review threads.

Superseded at 9f00a2e: the App review found an unresolved Important shell-wrapper bypass on this exact head.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 9f00a2e

Prior Findings Dispositioned (3)

• prior:de84c7e important 1 — fixed — src/server/job-manifest.ts:1159 — The exact pinned adapter now routes the default-off branch through buildEnvGuardPluginCleanupShell; src/server/env-guard-plugin.ts:173 removes both owned persistent artifacts, and the pre-populated cleanup test is at src/server/env-guard-plugin.test.ts:215.
• prior:de84c7e important 2 — fixed — src/server/env-guard-plugin.ts:70 — Classification now tests the dump regex before an anchored helper-only regex; the literal-plugin corpus covers both helper-plus-dump orderings at src/server/env-guard-plugin.test.ts:35.
• prior:de84c7e important 3 — fixed — src/server/env-guard-plugin.ts:42 — The wrapper regex now accepts unquoted payloads, and exact-artifact tests cover sh -lc env, bash -c printenv, and /bin/sh -lc env at src/server/env-guard-plugin.test.ts:32.

Important Issues (1)

• [gstack/review + native-codex] Dockerfile:352 — The newly pinned guard still permits full dumps through valid shell-wrapper positional arguments. sh -c env ignored executes bare env (ignored becomes shell $0), but the adapter unwraps the whole tail as env ignored, which the dump regex allows. Likewise, bash -c "env" ignored does not match the quoted-wrapper branch because tokens follow the closing quote. The added corpus only covers wrappers without positional arguments.
  • Parse the shell -c command-string argument separately from subsequent $0/positional arguments for both quoted and unquoted forms, then add classifier and literal-plugin cases such as sh -c env ignored, bash -c printenv ignored, and bash -c "env" ignored before canarying.

Strengths

• The immutable adapter pin and lockstep Paperclip assertion move together.
• The corrective adapter commit is a direct child of the previously reviewed pin and fixes all three prior findings with exact-artifact coverage.
• The cleanup path is narrowly scoped to the two generated guard artifacts, preserving unrelated OpenCode configuration.

Recommended Action

1. Close the remaining shell-wrapper positional-argument bypass before merge.
2. Re-pin to the corrected adapter commit and move the lockstep assertion with it.

[allyblockcast]

Hey @kkroo! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• PR is titled build: but includes source code changes (server/src/agent-shell-guard.ts). Please retitle as fix:, feat:, or refactor: so the right gates run, or remove the source code changes if this is genuinely a build: PR.

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

[allyblockcast]

Approved head d6beacc after addressing the remaining env-guard shell-wrapper positional-argument bypass. Adapter pin now targets 83197d46; Paperclip's local guard matches the behavior. Verified focused adapter guard suite, adapter typecheck/build, focused Paperclip guard/Dockerfile tests, and server typecheck.

[allyblockcast]

Hey @kkroo! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• Add the dedup-search checkbox to your PR description and check it once you have searched the GitHub PR list for similar PRs. See the PR template at .github/PULL_REQUEST_TEMPLATE.md and CONTRIBUTING.md → "Before You Start: Search First".

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip