fix(security-audit): pass the advisory report through a file - #18
Open
arcuru-bot wants to merge 1 commit into
Open
arcuru-bot wants to merge 1 commit into
arcuru-bot wants to merge 1 commit into
Conversation
When cargo-deny reports an advisory the issue-reporting step died before it started, with: An error occurred trying to start process '/usr/bin/bash' ... Argument list too long The report was captured into a step output and re-exported as the AUDIT_OUTPUT environment variable of the reporting step. Because the audit step redirected stderr as well, that capture also swallowed the whole Nix dev-shell build log — 242 KB on a cold cache, against roughly 8 KB of actual cargo-deny report. A single environment string over 128 KiB exceeds the per-string execve limit, so the runner could not exec bash at all, and the step that exists to file the advisory failed in exactly the case it exists for. A clean audit never reached it. Two changes: - Build the dev shell in its own step, so the Nix build log is no longer part of what gets captured. - Write the report to a file under RUNNER_TEMP and hand it to `gh issue edit`/`gh issue create` with --body-file, so no copy of it passes through argv or the environment. A GitHub issue body is still capped at 65536 characters, so an unexpectedly large report keeps its tail — where cargo-deny prints the advisories and its verdict — and links the run log for the rest, rather than failing the API call.
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
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.
The bug
Deps: Security Auditon eideticamainwent red on 2026-08-19 (run 32222183430). Two things are in that run: a genuine advisory finding, and this defect. The step that is supposed to open or update the tracking issue never started:The reporting step received the whole report as the
AUDIT_OUTPUTenvironment variable. Because the audit step redirected stderr as well (2>&1), that capture also swallowed the entire Nix dev-shell build log. Measured from the failing run: 242,259 bytes captured, of which the cargo-deny report is about 8 KB — the rest iscopying path ...,building ..., and a run of 502 retries against a substituter.A single environment string over
MAX_ARG_STRLEN(128 KiB) makesexecvefail withE2BIG, so the runner could not start bash at all. Reproduced directly:So the reporting path breaks in exactly the case it exists to handle. A clean audit never exercises it, which is why this survived until a real advisory landed.
The fix
nix develop --command true), so the Nix build log is no longer part of what the audit step captures. On a warm store the secondnix developis quiet, and the captured report is just cargo-deny's output.RUNNER_TEMPand pass it withgh issue edit --body-file/gh issue create --body-file. No copy of the report goes through argv or the environment, so the size of the report can no longer decide whether the step starts. This also removes theGITHUB_OUTPUTheredoc, whoseAUDIT_EOFdelimiter was injectable from report content.advisories FAILEDverdict — with a note linking the run log. This is a safety valve, not the normal path: with the dev-shell log gone the real report is ~8 KB and is posted whole.Verification
Extracted the reporting step and ran it against the actual 242 KB capture from the failing run, with a stubbed
gh:The generated body ends with the full
smallstr/yrsadvisory block andadvisories FAILED, i.e. the payload survives even in the pathological case that used to kill the step.actionlint(with shellcheck) over all of.github/workflows/*.yml: exit 0.Blast radius
.github/workflows/security-audit.yml. It is a reusable workflow, so the change goes live for callers only when they bump their pinned SHA. Current consumers includearcuru/eidetica,arcuru/chaz,arcuru/pokemandarcuru/headjack; each needs a separate pin bump to pick this up.Not in scope
smallstr/yrsadvisory itself.flake-update.ymlandcargo-update.ymlcapturenix ... 2>&1into a step output the same way and interpolate it into a PR body. They have not hit the limit, but they are the same shape; tracked separately.