Skip to content

bug(hooks): secret-read-guard allows the read behind a ) in a $( ) comment, and behind $-continuation-( in "…" or a heredoc body (the two shapes removed from #5285) #5945

Open

Description

@0xdhx

Pre-submission checklist

  • I have searched existing issues and this bug has not already been reported

GSD Version

1.20.1 (tag commit 9385f4618). Also on next at f3f3a1826 and on #5285's head 18f963b81.

Runtime

Claude Code

Operating System

Linux (WSL2)

Node.js Version

v24.21.0

Shell

GNU bash 5.2.21, dash 0.5.12, and zsh 5.9 (the alpine:3 image).

Installation Method

Updated from a previous version

What happened?

This is the sub-issue trek-e asked for on #5285, for the two shapes Major 2 of the 2026-10-28 review took out of that PR. Both let a real shell read .env while the guard allows the command. The ruling: #5285 (comment)

Its parent is #5255, the issue that work was cut from. I have pull access only, so I can't attach the sub-issue link. @trek-e, please attach it, or move it if you meant another parent. #5285 closes #5255 when it merges, and this issue stays open to track the work.

Neither shape is a regression from #5285: next and v1.20.1 allow them too.

Row next, v1.20.1, #5285's head #5943's head 11f00dde5 Shells that run the read
a ) in a comment in $( ), plain name allowed denied bash, dash, zsh
the same, name spelled $'.env' allowed allowed bash, zsh
$, a line continuation, then (, inside "…" allowed allowed bash, dash
the same, in an unquoted heredoc body allowed allowed bash, dash, zsh

Shape 1: a ) in a comment inside $( ). findParenClose has no # case:

function findParenClose(str, i) {
let depth = 1;
let heredocTags = [];
while (i < str.length) {
const ch = str[i];
if (ch === '\\') { i += 2; continue; }
if (ch === "'") {
const j = str.indexOf("'", i + 1);
i = j === -1 ? str.length : j + 1;
continue;
}
if (ch === '"') {
i++;
while (i < str.length && str[i] !== '"') {
if (str[i] === '\\') { i += 2; continue; }
if (str[i] === '$' && str[i + 1] === '(') { i = findParenClose(str, i + 2) + 1; continue; }
if (str[i] === '`') {
const j = str.indexOf('`', i + 1);
i = j === -1 ? str.length : j + 1;
continue;
}
i++;
}
i++;
continue;
}
if (ch === '`') {
const j = str.indexOf('`', i + 1);
i = j === -1 ? str.length : j + 1;
continue;
}
if (heredocBodiesSkippedInSpans && ch === '<' && str[i + 1] === '<' && str[i + 2] !== '<') {
const tag = readHeredocTag(str, i + 2);
heredocTags.push(tag);
i = tag.end;
continue;
}
if (ch === '\n' && heredocTags.length) {
i = consumeHeredocBodies(str, i + 1, heredocTags).end;
heredocTags = [];
continue;
}
if (ch === '(') depth++;
else if (ch === ')') {
depth--;
if (depth === 0) return i;
}
i++;
}
return str.length;
}

In echo $(#)' newline ); cat .env #', the shell reads #)' as a comment and ends the span at the ) on the next line, so cat .env runs and the last #' is a comment. The finder ends the span at the ) in the comment, so the ' after it opens a string that runs to the end of the command, and cat .env is data.

#5943 reads past a ) in a comment, so it denies the plain spelling at its head. It still allows the $'.env' spelling, because that head has no $'…' reading. Its description says its rebase onto #5285 carries that reading into its scans, so this half may close there. I have not measured the two together: git merge-tree of the two heads conflicts in the hook.

Shape 2: a $ before a line continuation that opens $(. The double-quote branch and the heredoc body scan test for $ directly beside (:

if (c === '$' && str[i + 1] === '(') {
const e = findParenClose(str, i + 2);
nested.push(str.slice(i + 2, e));
i = e + 1;
continue;

let i = 0;
while (i < body.length) {
if (body[i] === '$' && body[i + 1] === '(') {
const e = findParenClose(body, i + 2);
nested.push(body.slice(i + 2, e));
i = e + 1;
continue;
}

The shells remove the backslash-newline first, so they see $( and run the substitution. zsh does not do this inside "…", and does in a heredoc body. The same form unquoted at top level, echo $\ newline (cat .env), is already denied on all four builds.

This is not #5512: its item 4 is a heredoc delimiter split by a continuation, and it doesn't list this shape. #5943's table does attribute $\-newline-( to #5512, and I'll point that row here at its next description edit.

What did you expect?

Each of the four commands is denied, as echo "$(cat .env)" is. A comment that only mentions a read stays allowed (control-comment-is-data below).

Steps to reproduce

Run from a release tree, so the guard's hooks/lib/ siblings load (git clone --depth 1 --branch v1.20.1 https://github.com/open-gsd/gsd-core && cd gsd-core). The script feeds each command to the hook, then runs it in a scratch directory holding a .env that contains a marker. reads=true means the marker reached the output.

cat > /tmp/removed-shapes.cjs <<'EOF'
const { spawnSync } = require('node:child_process');
const fs = require('node:fs'), os = require('node:os'), path = require('node:path');
const rows = [
  ['comment-plain', "echo $(#)'\n); cat .env #'"],
  ['comment-ansi-c', "echo $(#)'\n); cat $'.env' #'"],
  ['continuation-dquote', 'echo "$\\\n(cat .env)"'],
  ['continuation-heredoc', 'cat <<E\n$\\\n(cat .env)\nE'],
  ['control-plain-read', 'cat .env'],
  ['control-dquote-cmdsub', 'echo "$(cat .env)"'],
  ['control-comment-is-data', 'echo $(: # cat .env\n); ls'],
];
const [guard, ...shells] = process.argv.slice(2);
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'removed-shapes-'));
fs.writeFileSync(path.join(dir, '.env'), 'MARKER\n');
for (const [name, command] of rows) {
  const hook = spawnSync('node', [guard], { input: JSON.stringify({ tool_name: 'Bash', tool_input: { command } }), encoding: 'utf8' });
  fs.writeFileSync(path.join(dir, 's.sh'), command + '\n');
  const ran = shells.map((sh) => {
    const o = spawnSync(sh, ['s.sh'], { cwd: dir, encoding: 'utf8', timeout: 5000 });
    return `${sh}-reads=${(o.stdout + o.stderr).includes('MARKER')}`;
  });
  console.log(`${name.padEnd(24)} hook-exit=${hook.status}  ${ran.join('  ')}`);
}
fs.rmSync(dir, { recursive: true });
EOF
node /tmp/removed-shapes.cjs hooks/gsd-secret-read-guard.js bash dash zsh

Error output / logs

Observed on next f3f3a1826 with bash and dash. v1.20.1 and #5285's head 18f963b81 print the same seven lines. Exit 2 is a denial.

comment-plain            hook-exit=0  bash-reads=true  dash-reads=true
comment-ansi-c           hook-exit=0  bash-reads=true  dash-reads=false
continuation-dquote      hook-exit=0  bash-reads=true  dash-reads=true
continuation-heredoc     hook-exit=0  bash-reads=true  dash-reads=true
control-plain-read       hook-exit=2  bash-reads=true  dash-reads=true
control-dquote-cmdsub    hook-exit=2  bash-reads=true  dash-reads=true
control-comment-is-data  hook-exit=0  bash-reads=false  dash-reads=false

zsh 5.9, in alpine:3 with node v24.22.1, on next and #5285's head:

comment-plain            hook-exit=0  zsh-reads=true
comment-ansi-c           hook-exit=0  zsh-reads=true
continuation-dquote      hook-exit=0  zsh-reads=false
continuation-heredoc     hook-exit=0  zsh-reads=true
control-plain-read       hook-exit=2  zsh-reads=true
control-dquote-cmdsub    hook-exit=2  zsh-reads=true
control-comment-is-data  hook-exit=0  zsh-reads=false

At #5943's head 11f00dde5 the only change, in every shell's run, is comment-plain hook-exit=2.

Where the work stands

Activity

  1. trek-e commented on Nov 7, 2026

    @trek-e
    Collaborator

    This was generated by AI during triage.

    Diagnosis

    Reproduced: yes, on next (f04db9d). The issue's script was run against hooks/gsd-secret-read-guard.js with bash 3.2.57, zsh and dash. All four defect rows exit 0 (allowed), while at least one shell prints the .env marker on each row:

    Row Hook exit bash reads zsh reads dash reads
    comment-plain 0 yes yes yes
    comment-ansi-c 0 yes yes no
    continuation-dquote 0 yes no yes
    continuation-heredoc 0 yes yes yes

    The controls behave as reported: plain cat .env and echo "$(cat .env)" exit 2, and a comment that only mentions a read stays allowed.

    Root cause: both causes are in hooks/gsd-secret-read-guard.js. This one sub-issue covers both shapes, per the #5285 ruling (comment 6673490452).

    1. Shape 1 () in a comment): findParenClose (:450-500) has no # comment case. A ) inside a comment within $( ) is taken as the close of the span (:493-495), so the rest of the command is read as being outside the substitution.
    2. Shape 2 (continuation between $ and (): the double-quote branch of tokenize (:647-676) drops a backslash-newline at :653-657. Its $( test at :658 still requires $ to sit directly beside (. The unquoted heredoc-body scan collectSubstitutions (:575-593) has the same adjacency test at :578.

    Introduced by: long-standing. findParenClose has had no # case in any recorded version (from 2026-09-21). Per the #5285 thread, its commits 81635ea and 61cb559 removed the handling for both shapes from that PR, so this is not a regression on next.

    Blast radius: MEDIUM. 11 symbols, all in the hook. tokenize is reached by findSecretRead, findDebuggerCliSpawn and the scan wrappers. The guard runs on every Bash command, so a span-finder change can add false denials. A crash in the hook allows the command (ON_CRASH ALLOW).

    Coupled (must not miss):

    Regression test: behavior rows driven through the hook. Each of the four rows above must exit 2. The controls keep their verdicts. A property test checks that inserting a backslash-newline between $ and ( inside "…" or an unquoted heredoc body never turns a denied substitution into an allowed one.

    Agent Brief

    Category: bug
    Summary: the secret-read guard allows two command shapes that real shells execute as a read of a secret file.
    Current behavior:

    • A ) inside a # comment within $( … ) ends the substitution early in the guard's reading, so a read that follows is treated as data.
    • $, then a backslash-newline, then ( inside double quotes or an unquoted heredoc body is not recognized as a command substitution.
    • In both cases the guard allows the command, and bash (and dash or zsh, depending on the shape) runs the read.

    Desired behavior:

    • The guard reads a # comment inside $( … ) the way the shell does, so a ) in the comment does not close the substitution.
    • It recognizes $ + line continuation + ( as a substitution wherever the shell removes the continuation (double quotes, unquoted heredoc bodies).
    • All four reported rows are denied.

    Key interfaces: the secret-read guard hook's verdict (exit 2 = deny) for a Bash command; its command-substitution span finder; its double-quote and heredoc-body substitution detection.

    Acceptance criteria:

    • The four reported rows, including the $'.env' spelling of shape 1, are denied.
    • cat .env and echo "$(cat .env)" stay denied.
    • A comment that only mentions a read stays allowed.
    • Single-quoted strings and quoted-tag heredoc bodies stay data.
    • A # that does not begin a comment (mid-word, $#, ${#x}, inside quotes) is not treated as a comment start.
    • A property test covers continuation insertion between $ and (.
    • The pinned allow rows and the "Documented gaps" header lines for these shapes are updated together with the fix.

    Out of scope: the other ANSI-C, case-pattern and function-substitution work in #5285, #5943 and #5724; any change to the guard's crash policy.

  2. added
    bugSomething isn't working
    and removed
    needs-triageNew issue awaiting maintainer review
    on Nov 7, 2026
  3. added a commit that references this issue on Nov 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingconfirmed-bugVerified reproducible bug

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions