GH-2334: Rank the untrusted-input boundary and extend it to every source - #2345
Merged
Merged
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
The trust boundary lived in the preamble, on none of the levels §0 ranks, so injected text that conflicted with it had no declared precedence to lose to — while §1.2 and §6, which say nothing about authorship, were ranked. It also spoke only of issues and their comments. Rank it: a new §1.8 promotes the boundary to an Absolute Rule, so §0 gives it precedence like any other. Enumerate the sources first, since the unenumerated ones had no rule at all: issue and PR bodies, comments and review threads, branch and commit text, and — the sharp one for this project — every string the parser extracts from a sample file. Reproducing a parse report runs the reader over a submitted file and puts UserComment, XMP, IPTC and keyword strings into the agent's context, exactly where a model looks for instructions. Two gaps the boundary did not close: it constrained the push target but not other repository-state mutations (merging the PR, rewriting history, tags, releases, labels, branch protection, collaborators), and it refused secrets but not other repository content (summarising docs/ or pasting a file listing into a public thread is the same exfiltration). Both are now named in the non-waivable list, and §6 gains a STOP for non-maintainer text that asks to widen scope, waive a STOP, relax a guard, mutate repo state, or emit repo content. The document's central control still rests on convention, because main requires the build check but no review — enabling 'Require a pull request before merging' would enforce it, but no magicsunday repo does that today, so it stays a family-wide decision recorded on the issue rather than a per-repo divergence.
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.
Closes #2334.
Two structural gaps in the trust boundary that #2333/#2326 established but could not close within their scope.
1. The rule now has a rank
AGENTS.md§0 says the higher level wins, but the boundary lived in the preamble — on none of those levels. So injected text conflicting with it had no declared precedence, while §1.2 (minimal scope) and §6 (STOP conditions), which say nothing about authorship, were ranked. A new §1.8 promotes it to an Absolute Rule, and §6 gains a STOP condition for non-maintainer text (or text whose author association cannot be determined) that asks to widen scope, waive a STOP, relax a guard, mutate repository state, or emit repository content.2. Every external source is enumerated, parser output included
The rule was framed around agent-task issues. Now the sources are named first, because the unenumerated ones had no rule at all:
UserComment,XPComment, XMP, IPTC creator/caption, keyword trees. Reproducing a parse report runs the reader over a submitted file and puts those attacker-controlled strings into the agent's context, exactly where a model looks for instructions. Parser output has no authority, ever.3. Two holes in the non-waivable list
GH-<n>history, tags/releases, labels/branch-protection/collaborators.docs/into a PR body or pasting a file listing is exfiltration to a public thread just as much as leaking a token.The one control that is convention, not code
The whole file-protection list rests on a review step the platform does not require —
mainrequires thebuild (8.4)check but no review. Enabling Require a pull request before merging would enforce it at no code cost, but no magicsunday repo requires review today (verified across all 20), so imagemeta is consistent, not lax. Flipping it on imagemeta alone would make it the outlier; this is recorded as a family-wide decision on the issue rather than a per-repo divergence.Documentation only.