Skip to content

[finding] check-shard-attestation's invokesScript() reads an echo QUOTING the invocation as an invocation — adjacency without quote-awareness #10889

Description

@os-zhuang

Noticed while implementing #10877 (PR #10884), which needed a narrow "does this step actually
INVOKE the script?" recognizer and evaluated invokesScript() as the candidate to reuse.

What it is

scripts/check-shard-attestation.mjs's invokesScript(runText, flag) added adjacency to
fix #6589's false red (a bare --verify substring anywhere in a run: block). Adjacency is
the right idea, but the tokenizer under it splits on whitespace and then strips the quotes off
each piece:

consttokens=command.split(/\s+/).map((token)=>token.replace(/^['"]|['"]$/g,''));

Shell does not word-split inside quotes. So a quoted string being handed to echo is torn
into separate "tokens", and the script name plus its flag inside that string read exactly like
a program being run with an argument.

Measured, on this tree

Against the real exported invokesScript (worktree at c20069e17f):

true | genuine invocation node scripts/check-shard-attestation.mjs --verify --dir x
true | an ECHO merely quoting it echo "node scripts/check-shard-attestation.mjs --verify is how you would check"
false | a ::error:: annotation echo "::error::check-shard-attestation.mjs --verify exited 1"

The second line is the finding. The third is green only incidentally — the ::error:: prefix
happens to fuse onto the basename token so it matches neither === SCRIPT_BASENAME nor
endsWith('/' + SCRIPT_BASENAME). Write the same annotation with a space, or as
echo "run node scripts/check-shard-attestation.mjs --verify to reproduce", and it counts.

Why it matters, and its direction

invokesScript feeds presence questions — whether an attesting job ends with the
--emit pair, whether an aggregate gate runs --verify. Presence assertions fail toward a
silent green, so the symptom is the gate reporting that a leg attests when it does not.

⚠️ The trigger is the ordinary act of writing a diagnostic about the script, which is the
same #4890 shape the neighbouring ci.yml block already walked into for the sibling flag —
and the reason #10818/#10877 exist at all. This is the same family, one file over.

Not urgent and not currently mis-firing on the checked-in tree: no workflow presently
echoes the invocation with a spelling that matches. It is latent, exactly as #10818 was.

Prior art for the fix, already in the tree

#10884 solved the same problem for check-required-contexts.mjs without touching this file, by
splitting commands with a word-splitter that records which words came out of a quoted
region
and requiring the program word to be unquoted (arguments may be quoted freely —
refusing "--verify" would narrow a presence assertion into a false red). See commandWords()
and invokes() there.

Whether this file should adopt that, or the two should converge on one shared helper, is a
judgment call rather than a mechanical swap — and ⚠️#10628 already had to undo one
mirrored helper in a neighbouring file, so "extract a shared module" is not automatically the
right answer here.

Filed unassigned, per scope discipline — #10884 deliberately did not touch this file.


Generated by Claude Code

Metadata

Metadata

Assignees

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions