Harden prompt template substitution (B10, B15, B16, F7) - #367
Merged
Conversation
Replace the two ad-hoc per-key substitution loops in resolvePrompt() and
resolveSkills() with a single shared single-pass substitute() helper:
- B10: use a replacer function so untrusted issue values containing regex
replacement specials ($&, $`, $', $$) are inserted literally instead of
being interpreted.
- B15: a single pass means one variable's value can no longer inject another
variable's {{token}} on a later pass.
- B16: {{description}} is now cleaned with the same stripBranchLine/stripRepoLine
helpers used by builtin planning/review, so custom prompts get repo:/branch:
directive lines stripped. The unmodified text is exposed as {{descriptionRaw}}.
- F7: residual unknown {{token}}s are detected after substitution and logged
via logWarn (named), without throwing.
Adds src/__tests__/audit-prompt-safety.test.ts covering literal insertion of
replacement specials, no cross-variable token injection, description cleaning
for custom types, and the unknown-token warning.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
warnResidualTokens re-scanned the substituted output, so a resolved
variable whose value legitimately contains literal "{{description}}"
(the B15 scenario) triggered a spurious unresolved-token warning. Collect
unknown keys during the single substitute() pass instead, so a token that
came from an inserted value never warns. Also reword the description-cleaning
comment to note it affects all prompt paths (builtin skills, custom prompts,
cli-prompt-render).
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
resolvePhaseMcpConfig expands ~ via node:os homedir(); the test compared it to process.env.HOME. Bun caches homedir() on first call and ignores later process.env.HOME mutations, so when a sibling test changes HOME they diverge on Linux CI and this test fails (works on macOS by execution-order luck). Assert against the same homedir() the implementation uses. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This was referenced Jul 18, 2026
Merged
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
The custom/skill prompt substitution interpolated untrusted issue text unsafely.
String.replacereplacement →$&,$`,$',$$reinterpreted, corrupting promptssubstitute(content, vars)(shared by both former loops){{token}}{{description}}was raw for custom types while builtins stripped repo/branch directive lines{{description}}via the same strip helpers{{tokens}}passed through silently{{…}})Tests
audit-prompt-safety.test.ts—$-sequences inserted literally; cross-token injection prevented; description cleaned; genuinely-unknown token warns while a literal{{…}}inside a value does not.Verification
typecheck clean · lint exit 0 ·
bun test849 pass / 0 fail.🤖 Generated with Claude Code