Fence finding text structurally in the swarm merge/verify prompts#28
Merged
Conversation
Close the second-order prompt-injection path: finding free-text from external backends (codex/grok) flowed raw into the merge- and verify-stage prompts with only an instruction-level "treat as DATA" guard. Mirror the diff-nonce fence for finding fields. - Add a per-run FINDING_NONCE in the skill's Bash prep (secrets.token_hex), deliberately NOT written into the external prompt so backends can't forge the delimiter; pass it to the workflow via args.findingNonce. - Workflow fenceFindings(): wrap the numbered merge list (FINDINGS-<nonce>) and per-solo verifier fields (FINDING-<nonce>) in nonce-carrying delimiters. Sandbox has no RNG, so the workflow only collision-checks the nonce against the returned findings and extends it deterministically on collision. - Fences layer WITH the existing DATA instruction and the output-gate scrub, not instead of them; scrub_secrets stage boundaries unchanged. - Document both fence hops in pipeline-blueprint.md § Security; bump swarm 0.3.0 -> 0.3.1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019FzeHRNUA2eud1nRSyCGbB
Address findings from a swarm review of the fence patch itself: - Critical: the verify stage interpolated `c.file`/`c.line` OUTSIDE the finding fence — but `file` is backend-controlled (schema caps length, not charset), so a newline-laden `file` value could pose as trusted scaffolding and flip the verifier's verdict: the exact second-order hole the patch closes. Move file/line inside the fenced body and label the path a claim, not a trusted coordinate. - Warning: a missing/malformed findingNonce dropped the structural fence silently, contradicting the "never silently insecure" contract. Log a visible degradation warning at both the absent and malformed cases. - Minor: blueprint § Security implied a merge/verify secret-scrub that does not exist (scrub runs only at the output gate). Reword to say fences — not a scrub — protect the merge/verify agents; item 2's scrub runs on the way out. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019FzeHRNUA2eud1nRSyCGbB
Second swarm-review round on the fence patch surfaced follow-ups: - Security (new vector): the verifier reads the backend-supplied `c.file` path, so a crafted `../../.ssh/id_rsa` / `~/.aws/credentials` / absolute path could exfiltrate a file. Fencing stopped instruction-injection in the text; add repoSafePath() to confine the READ target — out-of-repo paths are flagged and the verifier is told not to open them. - Fail closed at the source: SKILL.md aborts (SWARM_NONCE_UNAVAILABLE) if python3/secrets can't mint the finding nonce, instead of silently degrading the workflow fence to the instruction-only guard downstream. - Nonce validator now enforces a length floor (>=16, matching token_hex(8)) not just shape, so a short/low-entropy caller token is rejected. - Docs: reconcile blueprint items 2 and 3 — scrubbing runs at the adapter and output boundaries, NOT at merge/verify; fencing (not a scrub) is what protects those in-session agents. Removes the self-contradiction. Consensus-cluster fencing (raised, verdict: disagree) left as-is: that text is already fenced at merge and re-enters no later agent prompt; "don't fully trust consensus" is the documented residual (§ Security item 5). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019FzeHRNUA2eud1nRSyCGbB
Follow-ups from a third swarm-review pass:
- Fix (regression I introduced): under `set -euo pipefail` a failed python3
command substitution aborts before the `[ -z ]` guard, so the nonce
fail-closed check was dead code and $TMPD leaked. Catch failure in an `|| {…}`
list at both nonce sites (diff + finding) so the marker + cleanup fire.
- --fix path-safety gate: the `--fix` re-read opens the backend-supplied
`file:line` in the unsandboxed main session. Add a step-0 repoSafePath gate
(skipped-unsafe-path) and carry a `pathSafe` flag on every returned finding so
consensus clusters + --fix reads are covered, not only the solo verifier.
- Visible degradation: propagate `fenceDegraded` into the return payload +
balance (not just the /workflows log); presenter renders a prominent warning.
- Harden repoSafePath: also reject Windows drive-absolute paths and any
control/newline chars; tighten the nonce validator to hex (token_hex shape).
- Sanitize newlines when building the merge numbered list so each finding is one
line (mis-clustering hardening; coverage guard already prevents loss).
- Docs: blueprint records the finding-fence threat-model assumptions (nonce
secrecy needs a non-steerable orchestrator; symlink-realpath is the residual).
Left as residual (declined, beyond this minimal feature): symlink realpath
containment (no fs in the workflow sandbox) and re-architecting nonce transport;
over-rejection of `..` paths kept as the conservative security default.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019FzeHRNUA2eud1nRSyCGbB
b3bc627 to
38a5824
Compare
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
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
summary/failure_scenario/mechanismthat the merge/verify agents then read unfenced.Changes
skills/review/SKILL.md) — mint a per-runFINDING_NONCE(secrets.token_hex) in the Bash prep, deliberately not written into the external prompt so backends never see it and cannot forge the delimiter; pass it viaargs.findingNonce.workflows/swarm-review.js) —fenceFindings()wraps the numbered merge list (FINDINGS-<nonce>) and per-solo verifier fields (FINDING-<nonce>) in nonce-carrying delimiters. The sandbox has no RNG, so the workflow only collision-checks the nonce against the returned findings and extends it deterministically (nonce-1,-2…) on collision.pipeline-blueprint.md§ Security documents both fence hops (and corrects a stale note claiming the diff was unfenced);README.mdsecurity paragraph updated; knowledge entryswarm-review-pipeline.mdcaptures the RNG-in-sandbox constraint.0.3.0→0.3.1(plugin.json+marketplace.json).Readiness
swarm-review-pipeline.mdupdatedTest plan
scrub_secrets/ output-gate still runs at every stage boundary (unchanged in this PR)/swarm:reviewon a diff and confirm merge/verify prompts carryFINDINGS-<nonce>/FINDING-<nonce>delimiters🤖 Generated with Claude Code