fix(scan): stop go-shell-exec-command reporting a fully literal argv - #158
Merged
Conversation
`exec.Command("cmd", "/c", "ver")` reads the Windows version. It does
re-introduce a shell, which is what the rule says, but there is nothing in
the argv for anyone to reach — the metacharacters the consequence warns
about are metacharacters nobody can supply. gosec's G204 draws the line in
the same place: constant arguments are not the finding.
This was the entire output of a whole-repository scan of SibtainOcn/Quiesce,
a local Windows CLI in Go. One finding, and it was this one. The maintainer
declined the scan workflow on the grounds that the classes it targets have
no surface in that codebase, and he was right down to the single line.
With the guard the repository scans clean, which is the honest result.
The guard has to end at the closing paren so that a literal followed by
anything else still reports: `"ls " + dir` leaves a `+`, `fmt.Sprintf(…)`
leaves an identifier, a bare variable leaves a name. Escaped quotes inside
a literal are consumed rather than treated as the end of one, so
`"echo \"hi\""` does not fall out of the guard on a technicality.
323 tests pass. Both halves are covered: three shapes that must go quiet
and three that must still fire.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ThreatCrush Security Scan63 finding(s) HIGH/CRITICAL: 4 | MEDIUM: 52 | LOW: 7
…and 13 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
ralyodio
added a commit
to profullstack/sh1pt
that referenced
this pull request
Aug 17, 2026
…ers (#971) 0.11.3 carries two false-positive fixes that reach nobody on a pinned pack until this moves — which is the standing property this pack documents about itself, not a surprise. - go-shell-exec-command no longer fires on a fully literal argv (profullstack/threatcrush#158). `exec.Command("cmd", "/c", "ver")` has nothing in it for anyone to influence; gosec's G204 draws the same line. - js-uninitialized-buffer no longer fires on a buffer filled before use (#156). Spec and integrity bumped in the same edit, as the input's own description requires: a hash from a different version fails closed, which is the right direction to fail and a confusing one to debug. The hash is npm's `dist.integrity` for 0.11.3, verified independently rather than copied — the published tarball was downloaded and hashed, and `openssl dgst -sha512 | openssl base64` reproduces it byte for byte: sha512-lxWvTtLDgckiWlRB3wMSoBNfMZ/3ao0CcmwETGyKclc+5NMU5Pl0jXSr0h+QrTtfxh7TNStk4ZgP5h8xbEvIWw== Verified against the published package rather than a local build: installed @profullstack/threatcrush@0.11.3 from the registry and re-scanned. Quiesce, the Go CLI whose single finding motivated #158, now reports 0. The malware-test-prs fixtures still report all 43 criticals, so detection has not moved. README's documented default moved with the manifest so the two cannot disagree. The 1.6.0/0.11.0 anecdote further down is left alone; it is accurate history and it is the reason this pack explains the lag at all. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
exec.Command("cmd", "/c", "ver")reads the Windows version. It does re-introduce a shell — which is what the rule says — but there is nothing in the argv for anyone to reach, so the metacharacters the consequence warns about are metacharacters nobody can supply. gosec's G204 draws the line in the same place: constant arguments are not the finding.Where it came from
This was the entire output of a whole-repository scan of SibtainOcn/Quiesce, a local Windows CLI in Go. One finding, and it was this one:
The maintainer had already declined the scan workflow on the grounds that the classes ThreatCrush targets have no surface in that codebase. He was right down to the single line. With this guard the repository scans clean, which is the honest result rather than a quieter wrong one.
The guard
A
lineGuard, so it only reads the matched line — the exemption must not become a window-wide amnesty.It ends at the closing paren, which is what keeps it narrow. A literal followed by anything else still reports:
exec.Command("sh", "-c", "ping -c 1 "+host)+before the)exec.Command("sh", "-c", fmt.Sprintf("ping %s", host)))exec.Command("sh", "-c", command))Escaped quotes inside a literal are consumed rather than read as the end of one, so
exec.Command("bash", "-c", "echo \"hi\"")does not fall out of the guard on a technicality.Verification
packages/scansuite.false-positives.test.tspairs three shapes that must go quiet with three that must still fire — a fix that only silences is indistinguishable from deleting the rule.ui.gofrom Quiesce:[].🤖 Generated with Claude Code