Repository navigation
fix: the Hypatia gate could never fire — the defects that made it unconditionally vacuous #98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,14 +47,28 @@ jobs: | |
| if: steps.install.outputs.installed == 'true' | ||
| run: | | ||
| set +e | ||
| panic-attack assail --format json . > panic-attack-findings.json 2>&1 | ||
| panic-attack assail --format json . > panic-attack-findings.json | ||
| PA_EXIT=$? | ||
| set -e | ||
|
|
||
| # Same defect class as the Hypatia job below: `2>&1` folded the | ||
| # scanner's stderr into the JSON payload, so every jq parse failed, | ||
| # every count silently became 0 via `|| echo 0`, and "Fail on critical | ||
| # findings" could never fire on any input. Keep stderr on the log. | ||
| if [ ! -s panic-attack-findings.json ]; then | ||
| echo "[]" > panic-attack-findings.json | ||
| fi | ||
|
|
||
| # Deliberately a WARNING, not a failure. panic-attack is a downloaded | ||
| # release binary whose exit-code and output contract are not verified | ||
| # here, and it has no confirmed --exit-zero equivalent, so we surface a | ||
| # malformed payload in the log rather than block on an unverified tool. | ||
| # Promote to `exit 1` (as the Hypatia job does) once that contract is | ||
| # confirmed -- see the follow-up issue linked from this PR. | ||
| if ! jq -e 'type == "array"' panic-attack-findings.json >/dev/null 2>&1; then | ||
| echo "::warning::panic-attack output is not a JSON array (exit ${PA_EXIT}); counts below are unreliable" | ||
| fi | ||
|
|
||
| # Parse finding counts | ||
| TOTAL=$(jq '. | length' panic-attack-findings.json 2>/dev/null || echo 0) | ||
| CRITICAL=$(jq '[.[] | select(.severity == "critical")] | length' panic-attack-findings.json 2>/dev/null || echo 0) | ||
|
|
@@ -73,13 +87,19 @@ jobs: | |
| if: steps.install.outputs.installed == 'true' | ||
| run: | | ||
| # Convert JSON findings into GitHub Actions annotations | ||
| jq -r '.[] | select(.file != null) | | ||
| # Findings carry no `.message` (keys: action,file,line,reason,rule_module, | ||
| # severity,type), so every annotation read "null". `.file` is an absolute | ||
| # runner path, which GitHub cannot anchor to the diff, so it is made | ||
| # workspace-relative here. | ||
| jq -r --arg ws "$GITHUB_WORKSPACE" '.[] | select(.file != null) | | ||
| (.file | ltrimstr($ws + "/")) as $f | | ||
| (.reason // .message // .type // "finding") as $m | | ||
| if .severity == "critical" then | ||
| "::error file=\(.file),line=\(.line // 1)::[panic-attack] \(.message)" | ||
| "::error file=\($f),line=\(.line // 1)::[panic-attack] \($m)" | ||
| elif .severity == "high" then | ||
| "::error file=\(.file),line=\(.line // 1)::[panic-attack] \(.message)" | ||
| "::error file=\($f),line=\(.line // 1)::[panic-attack] \($m)" | ||
| else | ||
| "::warning file=\(.file),line=\(.line // 1)::[panic-attack] \(.message)" | ||
| "::warning file=\($f),line=\(.line // 1)::[panic-attack] \($m)" | ||
| end | ||
| ' panic-attack-findings.json || true | ||
|
|
||
|
|
@@ -164,12 +184,28 @@ jobs: | |
| if: steps.build.outputs.ready == 'true' | ||
| run: | | ||
| set +e | ||
| HYPATIA_FORMAT=json "$HOME/hypatia/hypatia-cli.sh" scan . > hypatia-findings.json 2>&1 | ||
| HYPATIA_FORMAT=json "$HOME/hypatia/hypatia-cli.sh" scan . --exit-zero > hypatia-findings.json | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
curl -fsSL https://raw.githubusercontent.com/hyperpolymath/hypatia/main/lib/hypatia/cli.ex |
grep -nE 'severity.*medium|severity.*low|exit_zero'Repository: hyperpolymath/nextgen-typing Length of output: 1200 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
cat -n .github/workflows/static-analysis-gate.yml | sed -n '170,245p'
printf '\nHypatia severity references in the workflow:\n'
rg -n -C 2 'HYPATIA_(FORMAT|SEVERITY)|--severity|hypatia-cli' .github/workflows/static-analysis-gate.ymlRepository: hyperpolymath/nextgen-typing Length of output: 4715 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
curl -fsSL https://raw.githubusercontent.com/hyperpolymath/hypatia/main/lib/hypatia/cli.ex |
sed -n '108,130p;330,355p;420,440p;585,610p'Repository: hyperpolymath/nextgen-typing Length of output: 3394 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
curl -fsSL https://raw.githubusercontent.com/hyperpolymath/hypatia/main/lib/hypatia/cli.ex |
grep -n -C 5 -E 'severity.*(filter|minimum|threshold)|severity.*in|severity_order|filter.*severity|config\.severity|opts\[:severity\]'Repository: hyperpolymath/nextgen-typing Length of output: 12569 Include low-severity findings in the Hypatia output. The Hypatia CLI defaults to a 🤖 Prompt for AI Agents |
||
| HYP_EXIT=$? | ||
| set -e | ||
|
|
||
| if [ ! -s hypatia-findings.json ] || ! jq empty hypatia-findings.json 2>/dev/null; then | ||
| echo "[]" > hypatia-findings.json | ||
| # --exit-zero is Hypatia's own documented CI recipe (lib/hypatia/cli.ex), | ||
| # for exactly this case: "use in CI when a downstream step gates on | ||
| # severity counts". Findings go to stdout, the one-line summary to | ||
| # stderr, and the process exits 0 unless the SCANNER itself failed. | ||
| # | ||
| # Do NOT redirect stderr into the payload with `2>&1`: that folds the | ||
| # summary line into the JSON, so every parse fails, the old `[]` | ||
| # fallback substituted a clean result, CRITICAL was always 0, and the | ||
| # gate below could never fire on any input. Keep stderr on the log. | ||
| if [ "$HYP_EXIT" -ne 0 ]; then | ||
| echo "::error::Hypatia scanner execution failed with exit ${HYP_EXIT}" | ||
| exit "$HYP_EXIT" | ||
| fi | ||
| # `jq empty` is NOT sufficient -- it succeeds on any valid JSON, | ||
| # including a bare string, object or null. Assert the array. | ||
| if [ ! -s hypatia-findings.json ] || ! jq -e 'type == "array"' hypatia-findings.json >/dev/null; then | ||
| echo "::error::Hypatia did not produce a valid JSON findings array" | ||
| exit 1 | ||
| fi | ||
|
|
||
| TOTAL=$(jq '. | length' hypatia-findings.json 2>/dev/null || echo 0) | ||
|
|
@@ -187,13 +223,19 @@ jobs: | |
| - name: Emit check annotations | ||
| if: steps.build.outputs.ready == 'true' | ||
| run: | | ||
| jq -r '.[] | select(.file != null) | | ||
| # Findings carry no `.message` (keys: action,file,line,reason,rule_module, | ||
| # severity,type), so every annotation read "null". `.file` is an absolute | ||
| # runner path, which GitHub cannot anchor to the diff, so it is made | ||
| # workspace-relative here. | ||
| jq -r --arg ws "$GITHUB_WORKSPACE" '.[] | select(.file != null) | | ||
| (.file | ltrimstr($ws + "/")) as $f | | ||
| (.reason // .message // .type // "finding") as $m | | ||
| if .severity == "critical" then | ||
| "::error file=\(.file),line=\(.line // 1)::[hypatia] \(.message)" | ||
| "::error file=\($f),line=\(.line // 1)::[hypatia] \($m)" | ||
| elif .severity == "high" then | ||
| "::error file=\(.file),line=\(.line // 1)::[hypatia] \(.message)" | ||
| "::error file=\($f),line=\(.line // 1)::[hypatia] \($m)" | ||
| else | ||
| "::warning file=\(.file),line=\(.line // 1)::[hypatia] \(.message)" | ||
| "::warning file=\($f),line=\(.line // 1)::[hypatia] \($m)" | ||
| end | ||
| ' hypatia-findings.json || true | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: hyperpolymath/nextgen-typing
Length of output: 231
🏁 Script executed:
Repository: hyperpolymath/nextgen-typing
Length of output: 4791
🌐 Web query:
GitHub Actions workflow commands escaping property values percent carriage return newline colon comma command message official documentation💡 Result:
To safely use workflow commands in GitHub Actions, you must escape specific characters in property values and message data to prevent command injection and parsing errors [1][2][3]. The official GitHub Actions runner interprets these commands via standard output (stdout), and failing to escape reserved characters can cause the runner to misinterpret strings as new commands or delimiters [1][2]. The escaping rules are implemented in the official GitHub Actions toolkit (
@actions/core) and consist of percent-encoding reserved characters [1][2]. Escaping Requirements: 1. Data (Message Content): When providing a message payload to a workflow command (e.g.,::notice::my message), the following characters must be escaped [1][2]: - Percent (%) becomes%25[1][2] - Carriage Return (\r) becomes%0D[1][2] - Line Feed/Newline (\n) becomes%0A[1][2] 2. Properties (Key-Value Pairs): When providing properties to a workflow command (e.g.,::notice file=main.js,line=1::my message), you must escape all characters required for data, plus the characters used as command delimiters [1][3]: - Percent (%) becomes%25[1][3] - Carriage Return (\r) becomes%0D[1][3] - Line Feed/Newline (\n) becomes%0A[1][3] - Colon (:) becomes%3A[1][3] - Comma (,) becomes%2C[1][3] Implementation Note: The GitHub Actions runner recognizes these percent-encoded values and decodes them correctly during execution [1]. If you are building custom actions or logging workflow commands from scripts, you should apply these transformations to your output strings before printing them to stdout to ensure security and stability [1][2]. For complex or multiline data, a common best practice to avoid escaping issues is to use environment files (such as GITHUB_OUTPUT, GITHUB_ENV, or GITHUB_STEP_SUMMARY) instead of printing commands directly to stdout [4][5][6].Citations:
Escape scanner fields before emitting workflow commands.
At both annotation blocks, encode scanner-controlled
fileand message values before writing them to stdout. A newline can expose a subsequent::...sequence as a new workflow command. Encode%, CR, and LF in messages, and also,and:in annotation properties.📍 Affects 1 file
.github/workflows/static-analysis-gate.yml#L94-L96(this comment).github/workflows/static-analysis-gate.yml#L230-L232🤖 Prompt for AI Agents