Security hardening: injection-study findings — P0 fixes, persistence/unread-exec gates, read_only default, sandbox default-on - #145
Merged
Conversation
…P0-2) P0-1: parseRunFlags/parseContinueArgs/parseReplFlags silently folded unrecognised flags into the task text. A typo'd or version-drifted flag corrupted the prompt with no signal, and anything that controls argv (wrapper scripts, CI job definitions, Makefile targets) gained a prompt-injection vector into the CLI itself. - unknown flag-shaped args now exit non-zero naming the offender - explicit "--" separator passes everything after it verbatim - dangling --id/--external-ref values error instead of becoming task text - trailing standalone flags after the task still work P0-2: `odek --version` (and -v) now alias the version subcommand — the form packaging scripts and CI preflights reach for first.
…how + events (P0-3) Parallel tool calls rendered as CALL,CALL,…,RESULT,RESULT,… with nothing tying a result to its call. A transcript parser pairing them sequentially attaches another call's output to a call — the injection study's harness did exactly that and scored three real compromises as clean. - session show emits a #label on both TOOL CALL and TOOL RESULT headers; provider tool-call IDs preferred, deterministic synthetic labels (m<msg>-c<n>) when omitted, '#unmatched' for orphaned results - tool_call_started/completed/failed JSONL events carry a matching call_id (provider ID verbatim, or it<iter>-call<idx> fallback)
…args opt-in (P0-4) --events-jsonl redacts arguments to args_bytes + args_sha256, so the stream alone cannot answer the first question an incident review asks; reconstruction requires session show, and the session may already be deleted. - tool_call_started now always carries args_summary: argv[0] (env assignments skipped), target path(s) / URL host, and the danger class — structure without argument content, secrets still redacted - --events-include-args (Config.EventsIncludeArgs / Engine.SetEventsIncludeArgs) opts the stream into raw redacted args
…ts (H-5) Scenario 20 asked the agent to append one documented line to a shell profile: not destructive, not egress, not an install, executes nothing in-session — so nothing escalated and the hook was planted in 9/9 draws while Claude Code refused the same write in both postures. New 'persistence' class (rank above system_write, default prompt, never eligible for trust shortcuts — the payload fires outside the session that granted trust), keyed on write targets rather than command shape: - shell profiles, fish config, direnv (.envrc), .git/hooks/*, .github/workflows/*, .gitlab-ci.yml, cron (crontab + /etc/cron.*), systemd system+user units, macOS LaunchAgents/Daemons, profile.d - crontab (non-listing), npm pkg-set/set-script lifecycle hooks, jq .scripts rewrites of package.json - content sniffing for package.json install lifecycle scripts and conftest.py autouse fixtures (write_file / patch / batch_patch) Reads keep the existing classifier — reading a CI workflow must stay frictionless. ClassifyPathWrite is the write-aware entry point; the shell classifier escalates redirect/operand targets before SystemWrite.
…(H-6) Scenarios 21/22 were the only ones that executed on the capable tier, and the payload sat inside the correct, documented fix for a real failure, firing on the verification run. 12 of 14 executing runs flagged the tamper in the same reply — model detection is sound, timing is not, so the control must be mechanical: - session read ledger (danger.RecordRead/WasRead): read_file/batch_read and successful writes mark paths; shell cat/head/tail viewers record operands on success. Failed reads never license execution — the observed 'cat errored, ran the file instead' failure stays gated. - UnreadScriptTargets: verb-aware scan (interpreters, source, direct invocation) of script-file operands that exist and were not read - new unread_exec class: SystemWrite tier, default prompt, never trust-shortcuttable; intercepts checkApproval/parallel_shell even when code_execution is allowed or trusted; explicit 'unread_exec': 'allow' override still honored - loop batch gate shows unread_exec in the approval card
The deny posture refused every gated call including reads: 51 study runs never reached the payload because the agent couldn't ls, tree, or cat. Real containment, but not resistance — and containment via inability gets flipped to 'allow' to meet a deadline, taking every protection with it. - new Action read_only (non_interactive: "read_only"): without a TTY, Safe-classified shell commands and native read tools over ordinary paths proceed; writes, exec, egress, and sensitive-location reads fail closed - unset non_interactive now defaults to read_only; explicitly set INVALID values still fail closed to deny (a typo must never loosen the gate); explicit deny/allow unchanged - config loader accepts and validates the new value
odek shipped sandbox support off by default with an honest warning — meaning the one control that actually contains the 'ran attacker- controlled code' class (where model quality does not help) had to be discovered. Now it must be deliberately given up: - run/continue/repl: sandbox defaults ON when no layer sets it - Docker unavailable (or unapproved project Dockerfile): implicit runs degrade to unsandboxed with a loud notice — explicit --sandbox keeps the hard-fail; ODEK_REQUIRE_SANDBOX=1 makes any fallback fatal - --no-sandbox flag + ODEK_NO_SANDBOX=1 opt out; config loader tracks SandboxExplicit through the layer merge - continue pins the session's original sandbox posture instead of inheriting the new default (no mid-conversation containment flips) - serve/telegram/subagent/mcp keep explicit-only semantics - TestMain sets ODEK_NO_SANDBOX=1 so spawned binaries stay hermetic
Observed draw: odek wrote the persistence hook, then read the payload, correctly identified the injection, and told the user 'The setup is blocked.' The setup was not blocked — the hook was already on disk. Detection that lands after the side effect is reporting; a reply that misreports the side effect is worse than silence because it actively stops the user from looking. Before a final answer goes out, its claims are diffed against the run-scoped ledger of completed mutating tool calls (write_file/patch/ batch_patch successes; shell/parallel_shell commands classified local_write or higher; failures excluded). On conflict, an explicitly-attributed odek consistency notice naming the actions is appended to the reply and a reply_ledger_mismatch signal fires. Conservative claim patterns — accurate replies and read-only runs are never annotated.
…ervability - SECURITY.md: persistence + unread_exec classes documented with rationale; non_interactive read_only default; sandbox default-on semantics; reply/ledger reconciliation; CLI argument discipline; repo-immutable policy promoted to a named invariant; attack-vector matrix rows for every new mitigation; taint-tracking noted as the next step beyond the labelling boundary - EXTENSIONS.md: call_id correlation, args_summary structure, --events-include-args opt-in - CONFIG.md: sandbox default flip + opt-outs, schedule safety floor extended with persistence/unread_exec - CLI.md: --version aliases, strict flag errors + '--' separator, --no-sandbox, --events-include-args - schedule runner: persistence/unread_exec join the non-overrideable deny floor
…ity, sandbox enforcement gaps From the pre-merge review of this branch: - CRIT-001: recordViewerReads recorded redirect targets and piped/partial operands — 'cat payload.sh > run.sh' licensed executing the never-seen copy, silently re-enabling the exact scenario the unread_exec gate exists to stop. Any pipe/redirect in a viewer command now disables recording entirely. - HIGH-001: read_file/batch_read record only full-file reads; a partial offset/limit window over a longer file no longer licenses execution. - HIGH-002: consistency-notice header carries an unpredictable [ref <nonce>] so model output cannot pre-forge the runtime's attribution; event-stream signal remains the authoritative record. - HIGH-003: mutation ledger evaluates parallel_shell per entry (one failed sibling no longer erases successful neighbors); shell failure detection is prefix-only so stdout mentioning 'error' keeps real mutations; file tools keep the JSON-error shape. - MED-003: ODEK_REQUIRE_SANDBOX=1 now outranks every opt-out, including explicit --no-sandbox — contradictory operator instructions fail loudly instead of guessing. - MED-004: batch approval card names the gating unread scripts inline. - LOW: args_summary classes now match what the gates actually enforce (script gate for shell, write-aware for write tools); rune-boundary truncation; unread_exec config participates as deny-wins/both-allow rather than doc-only override; docs updated for ledger process scope and the deliberate Dockerfile-degradation policy call.
…fault, risk classes, auditable events
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses the indirect-prompt-injection benchmark findings end to end: all four P0 bugs, all five hardening gaps, plus the observability the study needed but didn't have. Every change carries regression tests tied to the observed failure shape; the branch survived a hostile pre-merge review that caught one real licensing hole (fixed, tested).
P0 — bugs
run/continue/repl;--separator for dash-prefixed tasks). Closes the silent-prompt-corruption + argv injection vector.odek --version/-valias the version subcommand.call_idon TOOL CALL/RESULT headers insession showand ontool_call_started/completed/failedJSONL events — batched parallel calls can be paired programmatically (the study's parser mis-paired them and scored three real compromises as clean).args_summary(argv0 / target path / URL host / danger class) always present;--events-include-argsopts into raw secret-redacted args for incident review.Hardening
persistencerisk class — deferred-execution write targets (shell profiles,.envrc, git hooks, CI workflows, cron/systemd/launchd,npm pkg setlifecycle hooks) plus content sniffing forpackage.jsoninstall scripts andconftest.pyautouse fixtures. Ranked abovesystem_write, never session-trust-shortcuttable. Reads stay frictionless (ClassifyPathWriteis write-only).unread_execgate — executing a repo-supplied script whose contents were not fully read this session prompts per-script. Session read ledger; failed/partial/redirected reads license nothing ("cat errored → ran the file" stays gated). Intercepts approval even whencode_executionis allowed/trusted.non_interactive: "read_only"is the new headless default: inspection proceeds, mutations fail closed. Explicit invalid values still fail closed todeny. Containment that survives contact with a deadline.--no-sandbox/ODEK_NO_SANDBOX=1opt out;ODEK_REQUIRE_SANDBOX=1outranks everything;continuepins the session's original posture.reply_ledger_mismatchsignal.Review pass
CRIT-001 (
cat payload.sh > run.shlicensing execution of the never-seen copy), HIGH-001/002/003 (partial reads, spoofable notice header, per-entry parallel_shell ledger), MED-003/004 and the actionable LOWs are all fixed with tests. Two deliberate policy calls documented in SECURITY.md (Dockerfile-degradation fallback; process-scoped read ledger on long-lived surfaces).Test plan
go test ./... -count=1— full suite greengo test -raceon danger/loop/events/config — cleangolangci-lint run ./internal/... ./cmd/...— 0 issues🤖 Generated with Kai