mirror of
https://github.com/trailofbits/skills.git
synced 2026-09-14 14:28:48 +08:00
Merge main into agentic-actions-cli-detection
#292 shipped agentic-actions-auditor 1.3.0, so this takes 1.4.0. Keeps this branch's broadened description, which names both invocation surfaces. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
kz-tob
@@ -177,7 +177,7 @@
|
||||
},
|
||||
{
|
||||
"name": "static-analysis",
|
||||
"version": "1.4.2",
|
||||
"version": "1.4.3",
|
||||
"description": "Static analysis toolkit with CodeQL, Semgrep, and SARIF parsing for security vulnerability detection",
|
||||
"author": {
|
||||
"name": "Axel Mierczuk & Paweł Płatek"
|
||||
@@ -303,7 +303,7 @@
|
||||
},
|
||||
{
|
||||
"name": "supply-chain-risk-auditor",
|
||||
"version": "2.0.1",
|
||||
"version": "2.0.2",
|
||||
"description": "Audit a project's npm, PyPI, and Go dependencies for supply-chain risk: version-matched advisories for direct dependencies and the full lockfile tree, abandoned upstreams, npm publisher concentration, and install scripts",
|
||||
"author": {
|
||||
"name": "Eric Quintero"
|
||||
@@ -333,7 +333,7 @@
|
||||
},
|
||||
{
|
||||
"name": "agentic-actions-auditor",
|
||||
"version": "1.3.0",
|
||||
"version": "1.4.0",
|
||||
"description": "Audits GitHub Actions workflows for security vulnerabilities in AI agent integrations, whether invoked as published actions (Claude Code Action, Gemini CLI, OpenAI Codex, GitHub AI Inference) or as CLI commands in run: steps",
|
||||
"author": {
|
||||
"name": "Emilio López & Will Vandevanter"
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
{
|
||||
"name": "agentic-actions-auditor",
|
||||
"description": "Audits GitHub Actions workflows for security vulnerabilities in AI agent integrations, whether invoked as published actions (Claude Code Action, Gemini CLI, OpenAI Codex, GitHub AI Inference) or as CLI commands in run: steps",
|
||||
"version": "1.3.0",
|
||||
"version": "1.4.0",
|
||||
"author": {
|
||||
"name": "Emilio López & Will Vandevanter"
|
||||
}
|
||||
|
||||
@@ -252,7 +252,9 @@ Vector I assumes -- it means the step is gated only by its `if:` condition, or n
|
||||
|
||||
**Claude Code Action:**
|
||||
- `prompt` -- the instruction sent to the AI agent
|
||||
- `direct_prompt`, `override_prompt` -- the same sink on pre-v1 workflows, which are still common
|
||||
- `claude_args` -- CLI arguments passed to Claude (may contain `--allowedTools`, `--disallowedTools`)
|
||||
- `allowed_tools`, `disallowed_tools`, `custom_instructions` -- the pre-v1 spellings of what `claude_args` now carries
|
||||
- `allowed_non_write_users` -- which users can trigger the action (wildcard `"*"` is a red flag)
|
||||
- `allowed_bots` -- which bots can trigger the action
|
||||
- `settings` -- path to Claude settings file (may configure tool permissions)
|
||||
|
||||
+3
-2
@@ -6,7 +6,7 @@ Security-relevant configuration fields, default behaviors, dangerous configurati
|
||||
|
||||
### Default Security Posture
|
||||
|
||||
- Bash tool disabled by default; commands must be explicitly allowed via `--allowedTools` in `claude_args`
|
||||
- Bash tool disabled by default; commands must be explicitly allowed via `--allowedTools` in `claude_args`, or via the `allowed_tools` input on pre-v1 workflows (see foundations.md)
|
||||
- Only users with repository write access can trigger (default when `allowed_non_write_users` is omitted)
|
||||
- GitHub Apps and bots blocked by default (when `allowed_bots` is omitted)
|
||||
- Commits to new branch, does NOT auto-create PRs (requires human review)
|
||||
@@ -18,10 +18,11 @@ Security-relevant configuration fields, default behaviors, dangerous configurati
|
||||
| Configuration | Risk |
|
||||
|--------------|------|
|
||||
| `claude_args: "--allowedTools Bash(*)"` | Unrestricted shell access; any prompt injection achieves full RCE |
|
||||
| `allowed_tools: "Bash(*)"` | The same risk on pre-v1 workflows, under the older input name |
|
||||
| `allowed_non_write_users: "*"` | Any GitHub user can trigger the action, including external contributors and attackers |
|
||||
| `allowed_bots: "*"` | Any bot can trigger, enables automated attack chains via bot-to-bot escalation |
|
||||
| `show_full_output: true` (in public repos) | Exposes full conversation including potential secrets in workflow logs |
|
||||
| `prompt` containing `${{ github.event.* }}` | Direct expression injection of attacker-controlled content into AI prompt |
|
||||
| `prompt` or `direct_prompt` containing `${{ github.event.* }}` | Direct expression injection of attacker-controlled content into AI prompt |
|
||||
|
||||
### Remediation Patterns
|
||||
|
||||
|
||||
+20
-1
@@ -153,11 +153,13 @@ Composite action (actions/issue-triage/action.yml):
|
||||
|
||||
```
|
||||
Caller workflow (.github/workflows/ci.yml):
|
||||
on: pull_request_target
|
||||
jobs:
|
||||
ai-review:
|
||||
uses: org/shared/.github/workflows/ai-review.yml@main
|
||||
with:
|
||||
pr_body: ${{ github.event.pull_request.body }}
|
||||
secrets: inherit
|
||||
|
||||
Called workflow (org/shared/.github/workflows/ai-review.yml):
|
||||
on:
|
||||
@@ -183,11 +185,28 @@ Called workflow (org/shared/.github/workflows/ai-review.yml):
|
||||
-> org/shared/.github/workflows/ai-review.yml, on.workflow_call.inputs
|
||||
4. AI action: prompt contains ${{ inputs.pr_body }}
|
||||
-> org/shared/.github/workflows/ai-review.yml, jobs.review.steps[0]
|
||||
5. Claude executes with tainted prompt via pull_request_target (has secrets access)
|
||||
5. Claude executes with tainted prompt via pull_request_target
|
||||
-> secrets reach the callee through `secrets: inherit` on the calling job
|
||||
-> .github/workflows/ci.yml, jobs.ai-review.secrets
|
||||
```
|
||||
|
||||
The trace format follows the same stacked multi-line style as other data flow traces in this skill. Each hop shows the relevant YAML location. Cross-file findings have a longer trace because they span multiple files, but are otherwise identical to direct findings.
|
||||
|
||||
## Secrets Across the Boundary
|
||||
|
||||
SKILL.md Step 5b scores severity partly on secrets availability, and the two kinds of resolved file carry secrets in opposite ways. Neither can be read off the resolved file alone.
|
||||
|
||||
**Composite actions have no `secrets` context.** GitHub's [contexts reference](https://docs.github.com/en/actions/reference/workflows-and-actions/contexts#secrets-context) states it is unavailable there and that a secret must be passed explicitly as an input. Two consequences, in opposite directions:
|
||||
|
||||
- A `${{ secrets.NAME }}` written inside `runs.steps[]` resolves to empty. An `anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}` in a composite action is not a secret exposure; it is a broken workflow. Do not report it as one.
|
||||
- A secret that did reach the action arrived through the caller's `with:` and now lives in `inputs.*`. Searching the composite file for `secrets.` finds nothing while the secret is present. Read the caller's `with:` block to decide whether one is.
|
||||
|
||||
**Reusable workflows receive secrets explicitly or by inheritance.** The caller names them under `jobs.<id>.secrets`, or passes all of them with `secrets: inherit`. Under `inherit` the called workflow need not declare them in `on.workflow_call.secrets` to reference them, so an empty `secrets:` block there is not evidence of a callee without secrets. `secrets: inherit` is the maximum, not a default: it raises severity rather than lowering it.
|
||||
|
||||
**Inheritance is not transitive.** In a chain A calls B calls C, C receives a secret only if A passed it to B and B passed it to C. The depth limit stops resolution at B, so nothing about C's secrets can be asserted from A.
|
||||
|
||||
Record what the caller passes alongside the input trace. A finding whose severity rests on secrets access should name the line that grants it.
|
||||
|
||||
## Depth Limit and Unresolved References
|
||||
|
||||
**Depth limit:** Fixed at 1 level. The top-level workflow is depth 0. Resolved files (composite actions and reusable workflows) are depth 1. Any cross-file references found at depth 1 are logged as unresolved with reason "Depth limit exceeded (max 1 level)" -- do NOT follow them.
|
||||
|
||||
+17
-1
@@ -85,7 +85,7 @@ Where each supported action receives prompt content that could carry attacker in
|
||||
|
||||
| Action | Prompt Fields | Notes |
|
||||
|--------|--------------|-------|
|
||||
| `anthropics/claude-code-action` | `with.prompt` | Also check `with.claude_args` for embedded instructions |
|
||||
| `anthropics/claude-code-action` | `with.prompt`, `with.direct_prompt`, `with.override_prompt` | The last two are the pre-v1 names; also check `with.claude_args` for embedded instructions |
|
||||
| `google-github-actions/run-gemini-cli` | `with.prompt` | Shell-style env var interpolation in prompt text |
|
||||
| `google-gemini/gemini-cli-action` | `with.prompt` | Legacy/archived Gemini action reference |
|
||||
| `openai/codex-action` | `with.prompt`, `with.prompt-file` | `prompt-file` may point to attacker-controlled file |
|
||||
@@ -94,3 +94,19 @@ Where each supported action receives prompt content that could carry attacker in
|
||||
| CLI-invoked agent (`run:` block) | positional argument, `-p`/`--print`, `-m`/`--message`/`--message-file`, a heredoc, a file the command reads, or stdin from a pipe | No `with:` block exists; the prompt is the command line and what feeds it |
|
||||
|
||||
When checking for attacker-controlled content in prompts, examine ALL fields listed for the relevant action, not just the primary `prompt` field.
|
||||
|
||||
### Claude Code Action Renamed Its Inputs at v1
|
||||
|
||||
`claude-code-action` replaced its input surface at v1. The v0.x names are absent from the v1 `action.yml` and the v1 names are absent from v0.x, so a signature written against one set silently matches nothing in the other. The action's [migration guide](https://github.com/anthropics/claude-code-action/blob/main/docs/migration-guide.md) gives the mapping; these are the security-relevant rows:
|
||||
|
||||
| v0.x, including the `@beta` tag | v1 and later | Why it matters |
|
||||
|---------------------------------|--------------|----------------|
|
||||
| `direct_prompt` | `prompt` | Prompt sink |
|
||||
| `override_prompt` | `prompt` | Prompt sink |
|
||||
| `custom_instructions` | `claude_args: --append-system-prompt` | System prompt sink |
|
||||
| `allowed_tools` | `claude_args: --allowedTools` | Tool allowlist, Vectors F and H |
|
||||
| `disallowed_tools` | `claude_args: --disallowedTools` | Tool denylist |
|
||||
|
||||
Both sets appear in the wild, so match on either. The ref after `@` does not identify the set: `@beta` carries the v0.x names, and files pinned at `@v1` or a commit SHA still carry `direct_prompt`. Read the field names present in the file, not the ref.
|
||||
|
||||
The tool lists also separate differently. `claude_args: '--allowedTools "Bash(a:*) Bash(b:*)"'` separates on spaces; the v0.x `allowed_tools: "Bash(a:*),Bash(b:*)"` separates on commas. A matcher that splits one way reads the other as a single entry.
|
||||
|
||||
+1
@@ -33,6 +33,7 @@ The expression is resolved before any step code executes. The AI action receives
|
||||
`${{ github.event.* }}` expressions inside any text-accepting field of an AI action step:
|
||||
|
||||
- `with.prompt` -- the primary prompt field (all actions)
|
||||
- `with.direct_prompt`, `with.override_prompt` -- the same field on pre-v1 Claude Code Action workflows
|
||||
- `with.system-prompt` -- system prompt (GitHub AI Inference)
|
||||
- `with.prompt-file` -- if it resolves to an attacker-controlled path (Codex, AI Inference)
|
||||
- `with.claude_args` -- may embed expressions as inline instructions (Claude Code Action)
|
||||
|
||||
+2
-2
@@ -35,14 +35,14 @@ The critical insight: the restriction is on the **command name**, not on shell i
|
||||
## What to Look For
|
||||
|
||||
1. **Gemini CLI:** `with.settings` JSON containing a `coreTools` array that includes `run_shell_command(echo)` or other shell commands supporting expansion
|
||||
2. **Claude Code Action:** `with.claude_args` containing `--allowedTools` with `Bash(echo:*)`, `Bash(cat:*)`, `Bash(printf:*)`, or similar restricted-but-expandable command patterns
|
||||
2. **Claude Code Action:** `with.claude_args` containing `--allowedTools`, or the pre-v1 `with.allowed_tools` input, with `Bash(echo:*)`, `Bash(cat:*)`, `Bash(printf:*)`, or similar restricted-but-expandable command patterns
|
||||
3. **General:** Any tool restriction pattern that allows a shell command supporting `$()`, backtick substitution, or process substitution (`<()`)
|
||||
4. **Dangerous expandable commands:** `echo`, `cat`, `printf`, `tee`, `head`, `tail`, `wc`, `sort`, and most standard Unix utilities -- these all pass arguments through a shell that evaluates subshell expressions
|
||||
|
||||
## Where to Look
|
||||
|
||||
1. `with.settings` (Gemini CLI) -- parse the JSON string for `coreTools` arrays containing shell command names
|
||||
2. `with.claude_args` (Claude Code Action) -- look for `--allowedTools` flags with `Bash(command:*)` patterns
|
||||
2. `with.claude_args` (Claude Code Action) -- look for `--allowedTools` flags with `Bash(command:*)` patterns. Pre-v1 workflows carry the same patterns in a `with.allowed_tools` string instead
|
||||
3. `with.codex-args` (OpenAI Codex) -- check for tool restriction flags
|
||||
4. Look specifically for patterns suggesting **restricted** tool access rather than fully open access -- fully open tool access is Vector H, not Vector F
|
||||
|
||||
|
||||
+2
-1
@@ -32,6 +32,7 @@ Without dangerous configs, a successful prompt injection may still be contained
|
||||
**Claude Code Action (`anthropics/claude-code-action`):**
|
||||
|
||||
- `with.claude_args` containing `--allowedTools Bash(*)` or `--allowedTools "Bash(*)"` -- unrestricted shell access, the AI can execute any command
|
||||
- `with.allowed_tools` containing `Bash(*)` -- the pre-v1 spelling of the same thing, on a plain input rather than inside `claude_args`
|
||||
- `with.claude_args` with broad tool patterns combining multiple unrestricted categories (e.g., `Bash(npm:*) Bash(git:*) Bash(curl:*)`)
|
||||
- `with.settings` pointing to a settings file -- flag for manual review, the file may override tool permissions in ways not visible in the workflow YAML
|
||||
|
||||
@@ -51,7 +52,7 @@ Without dangerous configs, a successful prompt injection may still be contained
|
||||
|
||||
The `with:` block of AI action steps:
|
||||
|
||||
- **Claude:** Parse `with.claude_args` string for `--allowedTools` patterns. Also check `with.settings` for external config file path
|
||||
- **Claude:** Parse `with.claude_args` string for `--allowedTools` patterns, and `with.allowed_tools` on pre-v1 workflows. Also check `with.settings` for external config file path
|
||||
- **Codex:** Check `with.sandbox` and `with.safety-strategy` field values directly
|
||||
- **Gemini:** Parse `with.settings` JSON string for `"sandbox": false` and approval mode settings. Check any args-style fields for `--yolo` or `--approval-mode=yolo`
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "static-analysis",
|
||||
"version": "1.4.2",
|
||||
"version": "1.4.3",
|
||||
"description": "Static analysis toolkit with CodeQL, Semgrep, and SARIF parsing for security vulnerability detection",
|
||||
"author": {
|
||||
"name": "Axel Mierczuk & Paweł Płatek"
|
||||
|
||||
@@ -259,6 +259,7 @@ read from the processes and the JSON they wrote.
|
||||
- [ ] User explicitly approved the scan plan (Step 3 gate passed)
|
||||
- [ ] `run-scans.sh` exited 0 and wrote `$OUTPUT_DIR/scans.json`
|
||||
- [ ] `failed` and `skipped` from `scans.json` are empty, or listed in the report
|
||||
- [ ] Scans marked `partial` in `scans.json` are none, or listed in the report — they ran with some of their rules failing to compile
|
||||
- [ ] Every `semgrep` command used `--metrics=off`
|
||||
- [ ] Approved plan written to `$OUTPUT_DIR/rulesets.json` at the Step 3 gate, and passed to
|
||||
the scanner unchanged
|
||||
|
||||
@@ -12,6 +12,16 @@ readonly METRICS_OFF="--metrics=off"
|
||||
SEVERITY_FLAGS=(--severity WARNING --severity ERROR)
|
||||
readonly DEFAULT_JOBS=4
|
||||
|
||||
# A semgrep join rule carries `mode: join` and the `join:` block that mode needs, and requiring
|
||||
# both is what keeps the prune below off a rule that merely mentions the words. Each is anchored
|
||||
# as a YAML key on its own line — optionally the first key of a list item, optionally quoted,
|
||||
# with a trailing comment allowed — because one file holds many rules and deleting it on a
|
||||
# `message:` that quotes the docs would take every sibling rule with it. A rule that names the
|
||||
# mode without the block is not a valid join rule; semgrep rejects it per-rule, which the exit-2
|
||||
# handling below now survives, so leaving it in place costs nothing.
|
||||
readonly JOIN_MODE_RE="^[[:space:]]*(-[[:space:]]+)?mode:[[:space:]]*(join|\"join\"|'join')[[:space:]]*(#.*)?\$"
|
||||
readonly JOIN_BLOCK_RE="^[[:space:]]*join:[[:space:]]*(#.*)?\$"
|
||||
|
||||
usage() {
|
||||
cat <<'USAGE'
|
||||
Usage: run-scans.sh --target DIR --output-dir DIR --mode MODE --rulesets FILE [options]
|
||||
@@ -25,8 +35,9 @@ Usage: run-scans.sh --target DIR --output-dir DIR --mode MODE --rulesets FILE [o
|
||||
--dry-run print the commands that would run, then exit; clones nothing
|
||||
|
||||
Writes OUTPUT_DIR/scans.json:
|
||||
{scans:[{lang,ruleset,json,sarif,findings}], failed:[...], skipped:[...],
|
||||
unscoped:[lang], alsoShared:["lang/ruleset"]}
|
||||
{scans:[{lang,ruleset,json,sarif,findings,filesScanned,partial,exitCode}],
|
||||
failed:[...], skipped:[...], unscoped:[lang], alsoShared:["lang/ruleset"]}
|
||||
partial is a scan that wrote complete output while some of its rules failed to compile.
|
||||
USAGE
|
||||
}
|
||||
|
||||
@@ -113,6 +124,25 @@ repo_dir_name() {
|
||||
printf '%s' "${owner:-unknown}-${repo:-rules}" | sed 's/[^A-Za-z0-9._-]\{1,\}/-/g'
|
||||
}
|
||||
|
||||
# Deletes every .yaml/.yml under $1 that the remaining arguments select, printing what it took.
|
||||
# The status is returned rather than swallowed: the prunes that use this keep files semgrep
|
||||
# cannot handle out of a --config directory, and one that quietly stops matching puts the bug it
|
||||
# exists to prevent straight back, with nothing on stderr to say so.
|
||||
prune_yaml() { # prune_yaml <dir> <find-predicate...>
|
||||
local dir=$1
|
||||
shift
|
||||
find "$dir" \( -name '*.yaml' -o -name '*.yml' \) -type f "$@" -print -delete
|
||||
}
|
||||
|
||||
# `wc -l` reads one line in the empty string, so the zero case comes from the string itself.
|
||||
count_lines() {
|
||||
if [ -z "$1" ]; then
|
||||
echo 0
|
||||
else
|
||||
printf '%s\n' "$1" | wc -l | tr -d ' '
|
||||
fi
|
||||
}
|
||||
|
||||
TARGET="" OUTPUT_DIR="" MODE="" RULESETS_FILE="" PRO="" DRY_RUN=""
|
||||
JOBS=$DEFAULT_JOBS
|
||||
while [ $# -gt 0 ]; do
|
||||
@@ -353,6 +383,41 @@ while [ $i -lt ${#CLONE_URLS[@]} ]; do
|
||||
printf '%s\t%s\n' "$url" "$(printf '%s' "$err" | tail -n 3 | tr '\n' ' ')" >>"$SKIPPED"
|
||||
continue
|
||||
fi
|
||||
# semgrep parses EVERY .yaml/.yml under a --config directory as a rule file, and a single
|
||||
# unparseable one aborts the whole scan with exit 7 — no findings from any of the rules that
|
||||
# were fine. Rule repos ship their own CI config alongside their rules, and a workflow's
|
||||
# `on: pull_request:` is a null value, which semgrep rejects outright. Observed killing both
|
||||
# trailofbits/semgrep-rules (.github/workflows/semgrep-rules-format.yml) and
|
||||
# elttam/semgrep-rules (perf-templates/benchmark-tests.yml) — two required rulesets silently
|
||||
# contributing nothing.
|
||||
#
|
||||
# A semgrep rule file always has a top-level `rules:` key; nothing else here does. Pruning on
|
||||
# that also drops the `*.test.yaml` fixtures, which are rule test inputs rather than rules.
|
||||
# Measured on this prune alone: keeps 118/145 files for trailofbits and 80/94 for elttam,
|
||||
# losing no real rule. The join prune below then takes elttam's one join rule, leaving 79.
|
||||
#
|
||||
# `mode: join` rules are the second thing semgrep 1.173 cannot take: they crash it outright
|
||||
# (AttributeError in join_rule.py), which kills the whole batch and writes no output at all —
|
||||
# a hard process failure rather than the rule-level error a bad rule produces. elttam's
|
||||
# rules/generic/jsp-likely-xss.yaml is one. Only .yaml/.yml are considered, so a README
|
||||
# quoting the docs is not a candidate, and the pair of keys above is what makes it a rule.
|
||||
#
|
||||
# A prune that deletes nothing is a legitimate outcome — a repo may ship rules and nothing
|
||||
# else — but a prune that cannot run is not, so its status is checked and the ruleset is
|
||||
# skipped with a reason rather than scanned from a half-pruned tree.
|
||||
if ! non_rules=$(prune_yaml "$dest" ! -exec grep -q '^rules:' {} \;) ||
|
||||
! join_rules=$(prune_yaml "$dest" -exec grep -qE "$JOIN_MODE_RE" {} \; \
|
||||
-exec grep -qE "$JOIN_BLOCK_RE" {} \;); then
|
||||
printf '%s\t%s\n' "$url" "could not prune unscannable YAML from the clone" >>"$SKIPPED"
|
||||
rm -rf "$dest"
|
||||
continue
|
||||
fi
|
||||
# Reported every time, including as zeroes. These two prunes are the difference between a
|
||||
# ruleset that runs and one that contributes nothing, and a run that stopped pruning would
|
||||
# otherwise look exactly like a run with nothing to prune.
|
||||
printf '%s: pruned %s non-rule YAML file(s) and %s join-mode rule file(s)\n' \
|
||||
"$name" "$(count_lines "$non_rules")" "$(count_lines "$join_rules")" >&2
|
||||
|
||||
# A repository that cloned but carries no rules scans nothing, and reporting it as fine would
|
||||
# show a completed scan against a ruleset that never ran.
|
||||
#
|
||||
@@ -534,9 +599,37 @@ while IFS=$'\t' read -r stem lang ruleset config includes; do
|
||||
scanned=-1
|
||||
ok=""
|
||||
# Exit 0 covers both "found nothing" and "found plenty", so it says nothing about findings.
|
||||
# Exit 1 is a successful scan on older versions. 7 means the config would not load and 2 a
|
||||
# bad argument; in both cases no scan happened.
|
||||
if { [ "$rc" -eq 0 ] || [ "$rc" -eq 1 ]; } && [ -s "$json" ] && [ -s "$sarif" ]; then
|
||||
# Exit 1 is a successful scan on older versions.
|
||||
#
|
||||
# Exit 2 is not only "no scan happened". semgrep also returns 2 when individual rules fail
|
||||
# to compile while the rest of the run completes and writes complete output — e.g. 12 Java
|
||||
# rules in elttam/semgrep-rules that current semgrep cannot parse, alongside 107 that ran
|
||||
# fine, and apiiro/malicious-code-ruleset whose own log read "Scan completed successfully
|
||||
# • Findings: 51". Both were thrown away. Exit 2 also still covers a bad argument, where
|
||||
# nothing is written at all; the artifact checks below tell the two apart, so 2 is allowed
|
||||
# through and flagged partial rather than trusted outright.
|
||||
#
|
||||
# Anything outside 0/1/2 stays fatal however plausible the artifacts look, exit 7 (config
|
||||
# would not load) included. Verified on semgrep 1.173: the exit code for an unloadable
|
||||
# config depends on the OUTPUT FLAGS, which is worth knowing before trusting either number.
|
||||
# Same rules directory, same target, back to back:
|
||||
#
|
||||
# semgrep --config rules target -> 7, nothing written
|
||||
# semgrep --config rules -o out.json --sarif-output=out.sarif -> 2, nothing written
|
||||
#
|
||||
# This script uses the second form, so a config that will not load reaches here as 2 with
|
||||
# no artifacts, and the -s checks below reject it on their own. The fatal branch is
|
||||
# therefore belt-and-braces rather than the thing doing the work — but it costs nothing,
|
||||
# it is what the suite pins, and it means a future semgrep that writes an empty result set
|
||||
# alongside a hard failure cannot be read as a clean scan.
|
||||
partial=""
|
||||
fatal=""
|
||||
case "$rc" in
|
||||
0 | 1) ;;
|
||||
2) partial=1 ;;
|
||||
*) fatal=1 ;;
|
||||
esac
|
||||
if [ -z "$fatal" ] && [ -s "$json" ] && [ -s "$sarif" ]; then
|
||||
if findings=$(jq -e '.results | length' "$json" 2>/dev/null); then
|
||||
ok=1
|
||||
# null and empty are different answers: a semgrep that does not report .paths gives -1,
|
||||
@@ -550,9 +643,14 @@ while IFS=$'\t' read -r stem lang ruleset config includes; do
|
||||
fi
|
||||
if [ -n "$ok" ]; then
|
||||
[ "$scanned" -ne 0 ] || printf '%s/%s\n' "$lang" "$ruleset" >>"$COVERED_NOTHING"
|
||||
# `partial` marks a run whose results are real but incomplete — some rules failed to
|
||||
# compile. Reporting it as an unqualified success would overstate coverage; dropping it
|
||||
# entirely (the old behaviour) understated it far worse.
|
||||
jq -nc --arg lang "$lang" --arg ruleset "$ruleset" --arg json "$json" \
|
||||
--arg sarif "$sarif" --argjson findings "$findings" --argjson scanned "$scanned" \
|
||||
'{lang:$lang, ruleset:$ruleset, json:$json, sarif:$sarif, findings:$findings, filesScanned:$scanned}' \
|
||||
--argjson partial "$([ -n "$partial" ] && echo true || echo false)" --arg rc "$rc" \
|
||||
'{lang:$lang, ruleset:$ruleset, json:$json, sarif:$sarif, findings:$findings,
|
||||
filesScanned:$scanned, partial:$partial, exitCode:($rc|tonumber)}' \
|
||||
>>"$WORK/scans.jsonl"
|
||||
else
|
||||
# Carries the same paths a success does: a scan that crashed part-way may still have
|
||||
|
||||
@@ -315,7 +315,8 @@ and the severity flags are all its job, not yours. It writes `$OUTPUT_DIR/scans.
|
||||
|
||||
| Field | Meaning |
|
||||
|-------|---------|
|
||||
| `scans` | Rulesets that ran, with `json`, `sarif`, `findings` and `filesScanned` for each. `findings` is counted from the JSON the scan wrote; `filesScanned` is how many files semgrep opened, or `-1` when it did not say |
|
||||
| `scans` | Rulesets that ran, with `json`, `sarif`, `findings`, `filesScanned`, `partial` and `exitCode` for each. `findings` is counted from the JSON the scan wrote; `filesScanned` is how many files semgrep opened, or `-1` when it did not say; `exitCode` is what semgrep exited with |
|
||||
| `scans[].partial` | `true` when the scan wrote complete output while some of its rules failed to compile — semgrep exits 2 and reports the rest of the run normally. The findings are real and in the merge; the rules that never compiled found nothing and cannot say so, so this reads as an unqualified success unless it is called out. **Must be shown.** |
|
||||
| `coveredNothing` | Rulesets that ran against zero files, because their `--include` globs matched nothing in the target. They report 0 findings exactly like a ruleset that ran and found nothing, so a plan naming a language the target does not contain reads as a clean audit. **Must be shown.** |
|
||||
| `failed` | Rulesets that ran and did not produce usable output, with the `json` and `sarif` paths they may have partly written, and the stderr excerpt. **Must be shown to the user.** |
|
||||
| `skipped` | Rulesets dropped before scanning, mostly repos that would not clone. **Must be shown.** |
|
||||
@@ -331,6 +332,9 @@ arguments, because the approved plan is what produced them.
|
||||
|
||||
**If `failed` or `skipped` is non-empty**, carry both into the Step 5 report. A run that covered
|
||||
four of nine rulesets reads exactly like one that covered four of four unless you say otherwise.
|
||||
The same line is why any scan with `partial: true` is carried across as well: it is in `scans` as
|
||||
a success, so the rules of it that never ran are invisible in every count the report otherwise
|
||||
prints.
|
||||
|
||||
---
|
||||
|
||||
@@ -413,6 +417,12 @@ one finding flagged by two rulesets is one row in the merge and two in that sum]
|
||||
- Skipped: <ruleset> — <reason from the workflow>
|
||||
- Failed: <ruleset> — <error from the workflow>
|
||||
|
||||
### Ran Partially:
|
||||
[omit when no scan has partial: true]
|
||||
- <ruleset> — ran and wrote full output, but some of its rules failed to compile (semgrep exit
|
||||
<exitCode>). Its findings are in the total below; the rules that did not compile scanned
|
||||
nothing, so this ruleset's coverage is narrower than its entry in the scan count suggests
|
||||
|
||||
### Also Covered Unscoped:
|
||||
[omit when alsoShared is empty]
|
||||
- <ruleset> — already running over the whole target from the baseline, so it was not scanned
|
||||
|
||||
@@ -16,7 +16,7 @@ command -v uv >/dev/null 2>&1 || {
|
||||
|
||||
PLUGIN_ROOT="$(cd "$(dirname "$0")/.." && pwd)"
|
||||
SCRIPT="$PLUGIN_ROOT/skills/semgrep/scripts/run-scans.sh"
|
||||
readonly EXPECTED_ASSERTIONS=78
|
||||
readonly EXPECTED_ASSERTIONS=93
|
||||
|
||||
command -v jq >/dev/null 2>&1 || {
|
||||
echo "run_scan_tests.sh: jq not found — required" >&2
|
||||
@@ -311,6 +311,29 @@ eq "$(jq '.failed | length' "$WORK/o3/scans.json")" "1" "a scan that exited 7 mu
|
||||
contains "$(jq -r '.failed[0].json' "$WORK/o3/scans.json")" "raw/python-python.json" \
|
||||
"a failed entry must carry the paths it may have partly written"
|
||||
|
||||
# Exit 2 is the case this differs on. semgrep returns 2 both for a bad argument, where it
|
||||
# writes nothing, and for a run where SOME rules failed to compile while the rest completed
|
||||
# and wrote full output. The first must fail; the second must be kept and marked partial,
|
||||
# because discarding it throws away a complete result set over rules that were never going
|
||||
# to run.
|
||||
export STUB_RC=2 STUB_RESULTS='{"a":1},{"b":2},{"c":3}'
|
||||
rc=$(run_real "$ONE" "$WORK/o2b")
|
||||
eq "$(jq '.scans | length' "$WORK/o2b/scans.json")" "1" \
|
||||
"exit 2 with complete output must be kept, not discarded"
|
||||
eq "$(jq -r '.scans[0].findings' "$WORK/o2b/scans.json")" "3" \
|
||||
"the findings of a partial scan must still be counted"
|
||||
eq "$(jq -r '.scans[0].partial' "$WORK/o2b/scans.json")" "true" \
|
||||
"a scan kept despite exit 2 must be flagged partial, not reported as clean"
|
||||
eq "$(jq '.failed | length' "$WORK/o2b/scans.json")" "0" "a partial scan must not also be failed"
|
||||
|
||||
# Exit 2 that wrote nothing is a bad argument: no scan happened, so it must still fail.
|
||||
export STUB_RC=2 STUB_WRITE=0
|
||||
rc=$(run_real "$ONE" "$WORK/o2c")
|
||||
eq "$(jq '.scans | length' "$WORK/o2c/scans.json")" "0" \
|
||||
"exit 2 with no output must not be treated as a partial success"
|
||||
eq "$(jq '.failed | length' "$WORK/o2c/scans.json")" "1" "exit 2 with no output must be reported as failed"
|
||||
unset STUB_WRITE
|
||||
|
||||
# Exit 0 with no output file is the case the exit code alone cannot catch.
|
||||
export STUB_RC=0 STUB_WRITE=0
|
||||
rc=$(run_real "$ONE" "$WORK/o4")
|
||||
@@ -360,16 +383,34 @@ case "${GIT_STUB_MODE:-fail}" in
|
||||
done
|
||||
exit 0
|
||||
;;
|
||||
# The shapes the two prunes have to tell apart, as a real rule repository ships them: rules,
|
||||
# the repo's own CI config, a rule test fixture, a join-mode rule, a rule that only mentions
|
||||
# the words, and a non-YAML file that does the same.
|
||||
mixed)
|
||||
mkdir -p "$dest/rules" "$dest/.github/workflows"
|
||||
printf 'rules:\n - id: keep\n pattern: eval(...)\n' >"$dest/rules/good.yaml"
|
||||
# `on: pull_request:` is the null value semgrep rejects, which is what aborts the whole scan.
|
||||
printf 'on:\n pull_request:\njobs:\n test:\n runs-on: ubuntu-latest\n' \
|
||||
>"$dest/.github/workflows/ci.yml"
|
||||
printf 'x: 1\ny: 2\n' >"$dest/rules/good.test.yaml"
|
||||
printf 'rules:\n - id: joiner\n mode: join\n join:\n rules:\n - id: inner\n' \
|
||||
>"$dest/rules/join.yaml"
|
||||
printf 'rules:\n - id: decoy\n message: "do not write mode: join in a new rule"\n pattern: f(...)\n - id: sibling\n pattern: g(...)\n' \
|
||||
>"$dest/rules/decoy.yaml"
|
||||
printf 'Rules using mode: join are not supported here.\n' >"$dest/README.md"
|
||||
exit 0
|
||||
;;
|
||||
esac
|
||||
GITSTUB
|
||||
chmod +x "$WORK/gitbin/git"
|
||||
|
||||
REPO=$(plan repo '{"baseline":["p/security-audit"],"third_party":["https://github.com/x/rules"]}')
|
||||
|
||||
CLONE_ERR="$WORK/clone.err"
|
||||
run_clone() {
|
||||
rm -rf "$2"
|
||||
PATH="$WORK/gitbin:$WORK/bin:$PATH" GIT_STUB_MODE="$1" bash "$SCRIPT" --target "$TARGET" \
|
||||
--output-dir "$2" --mode run-all --rulesets "$REPO" --jobs 1 >/dev/null 2>&1
|
||||
--output-dir "$2" --mode run-all --rulesets "$REPO" --jobs 1 >/dev/null 2>"$CLONE_ERR"
|
||||
echo $?
|
||||
}
|
||||
|
||||
@@ -389,6 +430,36 @@ run_clone ok "$WORK/c3" >/dev/null
|
||||
eq "$(jq '.skipped | length' "$WORK/c3/scans.json")" "0" "a healthy clone must not be skipped"
|
||||
eq "$(jq '.scans | length' "$WORK/c3/scans.json")" "2" "a healthy clone must be scanned alongside the baseline"
|
||||
|
||||
# ------------------------------------------------------------------------ pruning a rule repo
|
||||
# semgrep parses every .yaml/.yml under a --config directory as a rule, so one file it cannot
|
||||
# parse aborts the scan and the whole ruleset contributes nothing. Both prunes delete files, so
|
||||
# both need a fixture: one proving they still hit what they are for, one proving they stop
|
||||
# there. Without it a prune that silently stopped matching looks exactly like a clean repo.
|
||||
echo "→ pruning a cloned rule repository"
|
||||
|
||||
run_clone mixed "$WORK/c4" >/dev/null
|
||||
CLONE="$WORK/c4/repos/x-rules"
|
||||
present() { # present <path> -> 1 when the file survived the prune
|
||||
[ -e "$CLONE/$1" ] && echo 1 || echo 0
|
||||
}
|
||||
|
||||
eq "$(present rules/good.yaml)" "1" "a rule file must survive the prune"
|
||||
eq "$(present .github/workflows/ci.yml)" "0" \
|
||||
"the repo's own CI workflow must be pruned: it is the file that aborts the scan with exit 7"
|
||||
eq "$(present rules/good.test.yaml)" "0" "a rule test fixture is not a rule and must be pruned"
|
||||
eq "$(present rules/join.yaml)" "0" "a mode: join rule must be pruned before it crashes semgrep"
|
||||
# The join prune deletes whole files, and one file holds many rules, so a match on prose costs
|
||||
# every sibling rule in it.
|
||||
eq "$(present rules/decoy.yaml)" "1" \
|
||||
"a rule whose message quotes mode: join must survive, and take its siblings with it"
|
||||
eq "$(present README.md)" "1" "the join prune must look at YAML only, not at every file"
|
||||
|
||||
contains "$(cat "$CLONE_ERR")" "pruned 2 non-rule YAML file(s) and 1 join-mode rule file(s)" \
|
||||
"the prune must report its counts, so a run that stopped pruning does not read as a clean repo"
|
||||
eq "$(jq '.skipped | length' "$WORK/c4/scans.json")" "0" "a pruned clone must still be scanned"
|
||||
eq "$(jq '.scans | length' "$WORK/c4/scans.json")" "2" \
|
||||
"pruning must leave the ruleset scannable alongside the baseline"
|
||||
|
||||
# ------------------------------------------------------------------------- coverage reporting
|
||||
# A ruleset whose --include globs match nothing exits 0 with an empty result, which reads in the
|
||||
# report exactly like a ruleset that ran and found nothing. That is how a plan naming the wrong
|
||||
|
||||
@@ -46,7 +46,7 @@ const DETECT = {
|
||||
frameworks: ["django"],
|
||||
};
|
||||
const SELECT = { rulesetsPath: "/proj/static_analysis_semgrep_1/rulesets.json", counts: { baseline: 2, language: 2, thirdParty: 1 } };
|
||||
const SCAN = { ok: true, scansJson: "/proj/static_analysis_semgrep_1/scans.json", succeeded: 4, failed: 0, skipped: 0, error: "" };
|
||||
const SCAN = { ok: true, scansJson: "/proj/static_analysis_semgrep_1/scans.json", succeeded: 4, partial: 1, failed: 0, skipped: 0, error: "" };
|
||||
const REPORT = { ok: true, resultsSarif: "/proj/static_analysis_semgrep_1/results/results.sarif", total: 7, report: "# Semgrep Scan Complete", error: "" };
|
||||
|
||||
// Each phase's reply is overridable; `null` stands for an agent that returned nothing.
|
||||
@@ -92,6 +92,9 @@ const SCENARIOS = {
|
||||
[out && out.total === 7, "the merged finding total must be returned"],
|
||||
[out && out.outputDir === DETECT.outputDir, "the resolved output directory must be returned"],
|
||||
[out && out.succeeded === 4, "the scan counts must be carried through"],
|
||||
// A partial scan is inside succeeded, so a caller reading only that count cannot tell a
|
||||
// ruleset that ran in full from one whose rules half compiled.
|
||||
[out && out.partial === 1, "the partial count must be carried through, not folded into succeeded"],
|
||||
[Object.keys(prompts).length === 4, "all four phases must run"],
|
||||
];
|
||||
},
|
||||
@@ -221,6 +224,7 @@ const SCENARIOS = {
|
||||
[/run-scans\.sh/.test(prompts.scan), "the scan phase must invoke run-scans.sh"],
|
||||
[/--rulesets "\/proj\/static_analysis_semgrep_1\/rulesets\.json"/.test(prompts.scan), "the ruleset file from the select phase must be passed through"],
|
||||
[/Do not add rulesets/.test(prompts.scan), "the agent must be told not to compose its own commands"],
|
||||
[/select\(\.partial\)/.test(prompts.scan), "the scan phase must be told to read the partial count"],
|
||||
];
|
||||
},
|
||||
|
||||
@@ -310,6 +314,9 @@ const SCENARIOS = {
|
||||
// so unless the report carries the line, the shortfall has nothing pointing at it.
|
||||
[/unparseable/.test(prompts.report), "SARIF files missing from the merge must be reported"],
|
||||
[/excludePattern/.test(prompts.report), "the pattern excluded from every scan must be reported"],
|
||||
// A partial ruleset is a success in scans.json with its findings in the merge, so the
|
||||
// rules that never compiled found nothing and nothing in the report says they did not run.
|
||||
[/Ran Partially/.test(prompts.report), "rulesets whose rules partly failed to compile must be reported"],
|
||||
];
|
||||
},
|
||||
};
|
||||
@@ -332,6 +339,8 @@ const MUTATIONS = [
|
||||
["stop reporting rulesets that covered nothing", (s) => s.replace(/\n\s*'\s*\.coveredNothing gets its own section[\s\S]*?clean audit\.',/, "")],
|
||||
["stop reporting unparseable SARIF", (s) => s.replace(/\n\s*'6\. Read the merge command[\s\S]*?not in the merge\.',/, "")],
|
||||
["stop reporting the exclude pattern", (s) => s.replace(/\n\s*'7\. If \.excludePattern[\s\S]*?clean coverage\.',/, "")],
|
||||
["stop reporting partial rulesets", (s) => s.replace(/\n\s*'\s*Rulesets with partial=true[\s\S]*?scans\.json"`,/, "")],
|
||||
["stop reading the partial count", (s) => s.replace(/\n\s*`\s*jq '\[\.scans\[\] \| select\(\.partial\)\][\s\S]*?scans\.json"`,/, "")],
|
||||
];
|
||||
|
||||
(async () => {
|
||||
|
||||
@@ -135,7 +135,7 @@ const SELECT_SCHEMA = {
|
||||
|
||||
const SCAN_SCHEMA = {
|
||||
type: 'object',
|
||||
required: ['ok', 'scansJson', 'succeeded', 'failed', 'skipped'],
|
||||
required: ['ok', 'scansJson', 'succeeded', 'partial', 'failed', 'skipped'],
|
||||
additionalProperties: false,
|
||||
properties: {
|
||||
ok: {
|
||||
@@ -145,6 +145,13 @@ const SCAN_SCHEMA = {
|
||||
},
|
||||
scansJson: { type: 'string', description: 'absolute path of scans.json, or "" when the script did not get that far' },
|
||||
succeeded: { type: 'integer', description: 'length of .scans in scans.json, or -1 if unreadable' },
|
||||
// Counted separately because a partial scan is inside .scans: its findings are real and its
|
||||
// coverage is not complete, so a bare success count reports the two as the same thing.
|
||||
partial: {
|
||||
type: 'integer',
|
||||
description:
|
||||
'how many of .scans have partial=true — rulesets that ran and wrote full output while some of their rules failed to compile. A subset of succeeded, not an addition to it; -1 if unreadable',
|
||||
},
|
||||
failed: { type: 'integer', description: 'length of .failed' },
|
||||
skipped: { type: 'integer', description: 'length of .skipped' },
|
||||
error: { type: 'string', description: 'the script stderr when ok is false, else ""' },
|
||||
@@ -384,9 +391,13 @@ const scanned = await agent(
|
||||
'It writes scans.json in the output directory and prints its path. Read the counts from that',
|
||||
'file with jq:',
|
||||
` jq '.scans | length' "${outputDir}/scans.json"`,
|
||||
` jq '[.scans[] | select(.partial)] | length' "${outputDir}/scans.json"`,
|
||||
` jq '.failed | length' "${outputDir}/scans.json"`,
|
||||
` jq '.skipped | length' "${outputDir}/scans.json"`,
|
||||
'',
|
||||
'The partial count is a subset of the first, not an addition to it: those rulesets ran and',
|
||||
'wrote full output with some of their rules failing to compile. Report it as partial.',
|
||||
'',
|
||||
'A non-zero exit means no scan succeeded. Report ok=false with the stderr, and do not retry',
|
||||
'with different arguments: the ruleset file is what produced them.',
|
||||
].join('\n'),
|
||||
@@ -397,7 +408,7 @@ if (!scanned || !scanned.ok) {
|
||||
const why = (scanned && scanned.error) || 'the scan agent returned nothing'
|
||||
throw new Error(`no scan succeeded: ${why}`)
|
||||
}
|
||||
log(`${scanned.succeeded} scans succeeded, ${scanned.failed} failed, ${scanned.skipped} skipped`)
|
||||
log(`${scanned.succeeded} scans succeeded (${scanned.partial} partial), ${scanned.failed} failed, ${scanned.skipped} skipped`)
|
||||
|
||||
phase('Report')
|
||||
const reported = await agent(
|
||||
@@ -462,6 +473,11 @@ const reported = await agent(
|
||||
' .alsoShared (rulesets not repeated per language because they already ran over the whole',
|
||||
' target — coverage is unaffected, but it explains why the ruleset and scan counts differ).',
|
||||
'',
|
||||
' Rulesets with partial=true get their own "Ran Partially" section on the same grounds.',
|
||||
' They are in .scans as successes and their findings are in the merge, but some of their',
|
||||
' rules never compiled, so the rules that did not run found nothing and cannot say so.',
|
||||
` jq -r '.scans[] | select(.partial) | .ruleset' "${outputDir}/scans.json"`,
|
||||
'',
|
||||
' .coveredNothing gets its own section too, and matters more than the other two: those are',
|
||||
' rulesets that opened zero files because their --include globs matched nothing. They',
|
||||
' report 0 findings exactly like a ruleset that ran and found nothing, so leaving them out',
|
||||
@@ -499,6 +515,7 @@ return {
|
||||
rulesetsPath: selected.rulesetsPath,
|
||||
scansJson: scanned.scansJson,
|
||||
succeeded: scanned.succeeded,
|
||||
partial: scanned.partial,
|
||||
failed: scanned.failed,
|
||||
skipped: scanned.skipped,
|
||||
resultsSarif: reported.resultsSarif,
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "supply-chain-risk-auditor",
|
||||
"version": "2.0.1",
|
||||
"version": "2.0.2",
|
||||
"description": "Audit a project's npm, PyPI, and Go dependencies for supply-chain risk: version-matched advisories for direct dependencies and the full lockfile tree, abandoned upstreams, npm publisher concentration, and install scripts",
|
||||
"author": {
|
||||
"name": "Eric Quintero"
|
||||
|
||||
+1
-1
@@ -150,7 +150,7 @@ class Http:
|
||||
return None
|
||||
try:
|
||||
stored = json.loads(path.read_text(encoding="utf-8"), strict=False)
|
||||
except (json.JSONDecodeError, OSError):
|
||||
except (json.JSONDecodeError, UnicodeDecodeError, OSError):
|
||||
# A run interrupted mid-write leaves a truncated file that would otherwise
|
||||
# crash every later run. Drop it and refetch.
|
||||
self.stats["errors"] += 1
|
||||
|
||||
+13
@@ -138,6 +138,19 @@ def test_truncated_cache_entry_is_dropped(tmp_path: Path):
|
||||
assert not path.exists()
|
||||
|
||||
|
||||
def test_non_utf8_cache_entry_is_dropped(tmp_path: Path):
|
||||
"""Undecodable cache bytes are corruption, not a permanent client crash."""
|
||||
http = Http(tmp_path, offline=True)
|
||||
path = seed(http, "GET", "https://example.com/x", {"ok": 1})
|
||||
path.write_bytes(b"\xff")
|
||||
|
||||
with pytest.raises(Unavailable):
|
||||
http.get_json("https://example.com/x")
|
||||
|
||||
assert http.stats["errors"] == 1
|
||||
assert not path.exists()
|
||||
|
||||
|
||||
def test_cache_owner_is_verified_on_posix(tmp_path: Path):
|
||||
"""The usual case: ownership checks out, so there is no caveat to report."""
|
||||
http = Http(tmp_path, offline=True)
|
||||
|
||||
Reference in New Issue
Block a user