Repository navigation
State the public-repository boundary, and fix what already crossed it - #188
Merged
Merged
Conversation
This repository is public — its history, comments, test output, commit messages and pull-request descriptions included — and the only rule saying so was one line at the end of a section about comment style, covering customer names, internal URLs and hostnames. That is a narrow slice of what can leak. `CLAUDE.md` now leads with the boundary, so it governs everything below it rather than trailing a subsection. `AGENTS.md` carries the same text, because the rule must not depend on which agent is reading. A pull-request template asks for the review explicitly: not because a checkbox prevents anything, but because the review it names is the only control that covers prose. Secret scanning finds credentials; nothing finds a paragraph of architecture. Writing the rule turned up three places already crossing it, two of them mine: - `README.md` pointed readers at the wider ecosystem by naming private repositories and linking to a path inside one. That link 404s for everyone outside the organisation, so it was already failing its audience as well as naming what it should not. - `tests/map/canary-case.ts` and `tests/protect/canary-engine-proof.test.ts` named a private repository as the owner of the reviewed recipe. The chain's SHAPE is public and worth documenting — a reviewed recipe, a generated rule, an engine that must block the exploit — but which internal system holds which half is not, and neither test needs it. They now say "Patchstack SaaS" and "stays private", which is all a reader of this repository can act on. A sweep for the other classes came back clean: no local filesystem paths, no ticket references outside the policy's own example, no developer usernames. Three apparent internal-hostname hits are `metadata.google.internal`, which is the public cloud metadata endpoint the SSRF guard exists to block. The comment edits are comment-only: the emitted canary map is byte-identical, so no vendored copy moves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Comprehensive public-boundary guidance with a robust, well-tested persona harness. 🎯 Quality: 86% Excellent · 📦 Size: Medium 📈 This month: Your 145th PR — above team average · Averaging Good |
Two changes, one obligation.
## The rule
The boundary only covered private PATCHSTACK information, so a change could satisfy every line of it and
still publish material that was not ours. Both instruction files and the pull-request template now name
third-party confidential and proprietary material as a separate obligation — copied system prompts,
private evaluation transcripts, unpublished documentation, customer-provided integration details — and
require public availability to be verified rather than assumed.
## What was already crossing it
Four field-test personas embedded the verbatim system prompts of third-party AI-builder platforms, and the
harness README carried the full refusal-mode research narrative with dated session reports. Neither holds
any Patchstack internals, which is exactly why the first version of the boundary did not catch them.
Excluding them from the npm tarball was never the relevant control. It stops an installing agent reading
them; it does nothing about confidentiality or third-party IP on a public repository page.
So the four personas are now synthetic: they reproduce the PRESSURES that produced the recorded refusals —
a non-technical user, heightened supply-chain caution, a runtime that stages commands instead of running
them — written from our own analysis rather than from upstream text. The harness contract is unchanged
(`{{FIXTURE_DIR}}` and `{{INSTALL_PROMPT}}`), and each file says at the top that it is synthetic and why.
The README keeps what the harness does, how to run it, the scoring contract and the void-round rules, and
loses the research narrative and the persona provenance. Every reference into the removed sections was
followed and rewritten rather than left dangling, including the claim in the agent notes that the personas
carried real platform prompts.
## The trade, stated rather than buried
A synthetic persona is weaker evidence than the policy a product actually runs. What this harness now
proves is that the install prompt survives the SHAPE of that pressure, and both READMEs say so. The
high-fidelity material belongs in a private evaluation repository, where it can be used and re-run; it has
been copied out rather than discarded.
Removing it from HEAD does not retract it — it remains in this repository's history, in forks, and in
anything that cached or notified on it. That is a separate decision, and not one a force-push settles.
Verified: all seven personas still drive the harness to a fully green scripted round, so the change is a
content change and not a harness change. Full suite unchanged at 1683 passed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The provenance I put at the top of each synthetic persona was reaching the agent. The runner read the file whole and sent it to stdin, comment included — so the agent was told, before it read a word of policy, that the policy was a synthetic reconstruction, that it was weaker than a real platform's, and that the harness was measuring whether the install prompt survived its scrutiny. That is not a cosmetic leak. It announces the evaluation and hands the agent a reason to discount the policy it is supposed to be applying, so a green round no longer distinguishes a prompt that survives platform pressure from one an agent waved through because it knew it was being measured. An HTML comment is not a hiding place: a model reads it like any other text. It is also the exact trap the install prompt is built to avoid. Meta-commentary about surviving scrutiny reads as manipulation — the reason that narration is kept out of the shipped docs — and the harness was committing it while testing for it. Provenance now lives in a `<!-- field-test:meta ... -->` block, and one composer strips those blocks before substitution. Marked blocks only, not all HTML comments: a persona may need an inline note the agent SHOULD see, and deleting it silently would change the pressure under test. An unterminated block drops the remainder rather than leaking it — the persona is then visibly truncated, which fails loudly instead of quietly. The guard is in the always-run suite, not in `field-test/`. The harness is what gates changes to the install prompt, so a harness that has stopped discriminating is worse than none: it reports the same green either way. It asserts over every persona that no provenance line survives composition, that the policy and both substitutions DO survive (a composer returning nothing would pass the first half), and — separately — that the runner actually calls the composer with no second substitution path. That last one is the shape of the original defect: the runner had its own `replaceAll` chain and consulted nothing. `stub-capture.mjs` proves it from the only vantage point that counts, the agent's own stdin. Reasoning about composition is not the same check. Verified end to end: 3548 bytes captured, zero provenance hits, policy present, fixture directory substituted. Both regressions confirmed to fail — bypassing the composer leaks two hits into real stdin, and a composer that strips nothing fails four assertions. Also replaced the two references to a numbered refusal mode whose taxonomy is no longer public, in the README and the runner, with the behaviour itself — a contradiction between the shipped docs and `dist/`. Public documentation has to be self-contained. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…block Both gaps were the same mistake in different places: a check that holds today but stops discriminating under a plausible future edit. ## A malformed block now throws Returning the truncated persona cannot leak, which is why it looked sufficient. It is not loud. A malformed block sitting AFTER both substitutions and enough policy text leaves a persona that still contains the fixture directory, the install prompt and a plausible amount of policy — so every length and substitution check passes, and the harness evaluates a silently incomplete policy. The round then measures a pressure nobody wrote, and reports the same green. `stripPersonaMeta` throws instead. The test pins the specific shape that made truncation look adequate: a block placed late enough that the shape checks would have been satisfied. ## The guard moved to `child.stdin.write` The previous wiring test read source strings — that the runner mentions the composer and has no second `replaceAll`. That stays true while a later edit hands `runAgent` a raw persona, or a new call site skips the composer, and neither would show up anywhere. Source text is not the seam. So `runAgent` moved into `agent.mjs` and refuses to write a prompt containing a provenance marker, or an empty one. The refusal is now where every present and future caller passes through, which makes the mistake impossible rather than merely detectable. It throws rather than stripping: stripping there would let the seam quietly tolerate a caller that skipped the composer, and the next such caller would pass a persona whose substitutions had been skipped too. The test drives that seam directly with the capture stub — the agent's own stdin, byte-for-byte against the composed prompt — with no fixture, no `npm install` and no network, which is what lets it live in the always-run suite instead of a README command. It also asserts the two refusals: a raw persona, and nothing at all. Mutation-checked, and the fourth one is why the byte-for-byte comparison earns its place: seam guard removed 1 failed seam strips instead of throwing 1 failed unterminated block truncates again 1 failed stdin write truncated in the pipe 7 failed All seven personas still run end to end. The `--confirm` second turn composes from the already-stripped prompt, so it inherits the same guarantee. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
/review |
mariojgt
approved these changes
Aug 27, 2026
daniloradovic
approved these changes
Aug 27, 2026
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.
Adds the boundary policy as
CLAUDE.md's leading section, the same text inAGENTS.md, and a PR-template checkbox. Then fixes the three places already crossing it.Why it leads the file
The only existing rule was one line at the end of a section about comment style:
That is a narrow slice. It says nothing about private repository names and paths, service topology, reachability recipes, rule-authoring rationale, or the "how we found this" narration that internal review naturally produces. The boundary now sits above the commands so it governs everything below it, rather than trailing a subsection about comments.
AGENTS.mdcarries the same text verbatim, becauseCLAUDE.mdonly governs Claude-based work and the rule must not depend on which agent is reading.The PR template asks for the review explicitly — not because a checkbox prevents anything, but because the review it names is the only control that covers prose. Secret scanning finds credentials; nothing finds a paragraph of architecture. Both files say that in as many words, so the checkbox is not mistaken for enforcement.
What the rule already indicted
Writing it turned up three violations, two of them mine:
README.mdtests/map/canary-case.tstests/protect/canary-engine-proof.test.tsThe README link 404s for everyone outside the organisation, so it was already failing its audience as well as naming what it shouldn't.
For the canary comments: the chain's shape is public and worth documenting — a reviewed recipe, a generated rule, an engine that must block the exploit. Which internal system holds which half is not, and neither test needs it. They now say "Patchstack SaaS" and "stays private", which is all a reader of this repository can act on.
Sweep
Clean on the other classes: no local filesystem paths, no ticket references outside the policy's own example, no developer usernames. Three apparent internal-hostname hits are
metadata.google.internal— the public cloud metadata endpoint the SSRF guard exists to block.The comment edits are comment-only: the emitted canary map is byte-identical, so no vendored copy moves. Full suite 1683 passed, 7 skipped;
AGENTS.mdand the template ship nowhere (npm pack --dry-runfinds neither).Two things for you to rule on separately
1. My own commit and PR style in this repository has been violating the new rule. The policy says historical investigation, mutation notes and "how we found this" belong in private records — and I have been writing exactly that into public commit messages and PR bodies here, including this one's siblings. I have not rewritten history and don't propose to; going forward I'll keep the reasoning in the review conversation and the commit to the invariant. Worth a decision on whether you want anything about the existing bodies.
2.
field-test/personas/embeds other companies' actual system prompts, andfield-test/README.mdnarrates the refusal-mode research in detail. That is a different class from this policy — not private Patchstack information — but it is third-party proprietary content and internal research in a public repository. That README even argues the research must stay out of the shipped tarball because agents read it and quote it back; the repository is public, so they can read it anyway. Out of scope for this PR, and I'd rather flag it than quietly rewrite it.🤖 Generated with Claude Code