Skip to content

Commit aa6553a

Browse files
committed
fix(review): remove a live log marker from the prompt document
1 parent 251a000 commit aa6553a

3 files changed

Lines changed: 36 additions & 6 deletions

File tree

docs/claude-pr-review-prompt.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ Three edits in that content were made by the workflow, not by anyone: `[block ta
1515
a block delimiter, `[log marker neutralised]` prefixes a line that would otherwise have been read as
1616
a GitHub Actions command, and `##[` is respaced to `## [` for the same reason. All three are
1717
sanitiser output. Read past them, and never quote one as if it were in the file — a `## [error]` in a
18-
CI excerpt was `##[error]` in the real log. If you need the exact line, `Read` the file.
18+
CI excerpt was `##` + `[error]`, unspaced, in the real log. If you need the exact line, `Read` the
19+
file.
1920

2021
## Tools
2122

tests/context-step-test.sh

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -319,11 +319,17 @@ expect_context '^\+incremental change$' "since-last-review diff carries its body
319319
# this is the assertion that catches it -- the body here is a command substitution and a
320320
# ${{ }} expression, and both must survive as characters.
321321
expect_context '\$\(touch /tmp/pwned\)' "PR body interpolates as literal text, not shell"
322-
# The whole default, not a prefix of it. This is the assertion that would have caught the
323-
# `}}` truncation in run_step's default: the Actions expression sits after the point where the
324-
# parameter expansion used to end, so its arrival proves the body reached the step intact.
325-
expect_context 'are literal text here\.$' "the whole PR body reaches the context, not a prefix"
326-
expect_context 'github\.token' "an Actions expression in the PR body survives as text"
322+
# The whole default, including the `}}` that the old inline form ate. `${PR_BODY-...}` ended at
323+
# the first `}` of `${{ github.token }}`, and the tail after it stayed inside the outer quotes
324+
# and was concatenated literally -- so a bare `github.token` match survived the bug, and only
325+
# the doubled brace distinguishes the fragment from the whole. Pinning it is what makes this
326+
# assertion about body integrity rather than about one substring surviving.
327+
#
328+
# The old form did also end `here.}` rather than `here.`, because the default's final `}` was
329+
# literal once the expansion had closed early, so an end-anchored match caught it too. This
330+
# spelling does not depend on that second-order effect.
331+
expect_context 'github\.token \}\} are literal text here\.$' \
332+
"the whole PR body reaches the context, not a prefix"
327333
expect "$([ -e /tmp/pwned ] && echo leaked || echo safe)" "safe" \
328334
"command substitution in the PR body did not execute"
329335

tests/workflow-lint-test.sh

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,29 @@ else
163163
echo "skip actionlint not installed; only the expression scan ran"
164164
fi
165165

166+
# The prompt document reaches the model through steps.prompt.outputs.content, which is the one
167+
# path into the prompt that `neutralise_untrusted` never touches -- it runs over the two
168+
# context step outputs only. So a workflow command written *here* is not conditional on what
169+
# an author does: it annotates the review's own check run on every run in the org, for as long
170+
# as the line is on main. This file documents the sanitiser, so it necessarily talks about the
171+
# markers, and one draft of that paragraph shipped a live `##[error]` for exactly that reason.
172+
#
173+
# Only the parsable spellings count. `##[` with no closing bracket on the line is inert (there
174+
# is nothing to close the command), and `## [error]` is the already-spaced form -- both appear
175+
# in the paragraph on purpose, and the contrast is the point of it.
176+
PROMPT_DOC=docs/claude-pr-review-prompt.md
177+
if [ ! -f "$PROMPT_DOC" ]; then
178+
echo "FAIL $PROMPT_DOC is missing, so the marker scan proves nothing"
179+
failures=$((failures + 1))
180+
elif found=$(grep -nE '##\[[A-Za-z][^]]*\]|^[[:space:]]*::' "$PROMPT_DOC"); then
181+
echo "FAIL $PROMPT_DOC contains a parsable Actions workflow command. It is injected into"
182+
echo " the prompt unsanitised, so this annotates every review run in the org:"
183+
printf '%s\n' "$found" | sed 's/^/ /'
184+
failures=$((failures + 1))
185+
else
186+
echo "ok the prompt document carries no parsable workflow command"
187+
fi
188+
166189
if [ "$failures" -ne 0 ]; then
167190
echo "$failures test(s) failed"
168191
exit 1

0 commit comments

Comments
 (0)