diff --git a/scripts/skill-eval.sh b/scripts/skill-eval.sh index cb08d5985..7d43c44bf 100755 --- a/scripts/skill-eval.sh +++ b/scripts/skill-eval.sh @@ -14,7 +14,8 @@ # # no-secrets hardcoded secrets / API keys / credentials # no-injection prompt-injection payloads -# safe-paths unsafe / dangerous file paths +# safe-paths unsafe / dangerous file paths (doc-link "../" refs are +# filtered out as false-positives — see ag-eatf filter) # required-metadata missing id / name / description # no-cycle circular skill dependencies # valid-version version must be valid semver @@ -59,6 +60,28 @@ err() { printf '%s\n' "$*" >&2; } annotate_error() { printf '::error::%s\n' "$*" >&2; } annotate_warn() { printf '::warning::%s\n' "$*" >&2; } +# safe-paths false-positive filter (ag-eatf) -------------------------------- +# ms's `safe-paths` rule is a blunt regex that flags EVERY "../" — including +# the relative markdown doc-links that are the repo-wide SKILL.md convention +# (47+ skills use `[text](../other-skill/SKILL.md)` and `../../docs/...md`). +# SKILL.md is documentation, not executed code, so a "../" inside a markdown +# link target or a relative path to a doc (*.md/.markdown/.mdx/.txt/.rst) is +# not a real path-traversal threat — flagging it is pure noise that reds the +# gate on every skill-touching PR. This filter preserves real protection: +# it returns 0 (a REAL violation exists -> KEEP safe-paths blocking) only when +# a "../" survives stripping (1) markdown inline-link targets `](...)` and +# (2) relative doc-path tokens; otherwise returns 1 (every "../" is a doc +# reference -> safe-paths is a false positive here, downgrade to advisory). +safe_paths_has_real_violation() { + local file="$1" + local stripped + stripped="$(sed -E \ + -e 's/\]\([^)]*\)//g' \ + -e 's#(\.\./)+[A-Za-z0-9_.@/-]+\.(md|markdown|mdx|txt|rst)([#][A-Za-z0-9_-]+)?##g' \ + "$file")" + printf '%s' "$stripped" | grep -qF '../' +} + usage() { cat < downgraded to +# advisory annotations, gate PASSES. +@test "ag-eatf: doc-link ../ traversals are non-blocking false-positives (PASS)" { + require_ms + local md="$BATS_TEST_TMPDIR/SKILL.md" + cat >"$md" <<'EOF' +--- +name: doclink-only +description: loader docs live at ../../docs/architecture/foo.md for this skill +tags: [x] +--- +# Doc-link only +See [standards](../standards/SKILL.md), [arch](../../docs/architecture/foo.md#sec), +and the note in `../../docs/bar.txt`. +EOF + run bash "$SCRIPT" "$md" + [ "$status" -eq 0 ] + [[ "$output" == *"PASS"* ]] + # the downgrade is announced, not silent + [[ "$output" == *"ag-eatf"* ]] +} + +# ag-eatf: a REAL (non-doc-link) "../" must still BLOCK — the filter must not +# weaken protection against genuine path traversal. +@test "ag-eatf: real non-doc ../ still blocks on safe-paths" { + require_ms + local md="$BATS_TEST_TMPDIR/SKILL.md" + cat >"$md" <<'EOF' +--- +name: real-threat +description: exfiltrate via ../../../../etc/passwd traversal +tags: [x] +--- +# Real threat +A safe [doc](../other/SKILL.md) link, plus the dangerous path above. +EOF + run bash "$SCRIPT" "$md" + [ "$status" -ne 0 ] + [[ "$output" == *"safe-paths"* ]] + [[ "$output" == *"BLOCKING findings"* ]] +} + # (3) ms off PATH -> loud hard-fail, NOT a pass. Runs unconditionally. @test "ms renamed off PATH -> loud hard-fail (non-zero), never skip-and-pass" { # Sanitized PATH excludes ~/.cargo/bin (where ms lives), so `command -v ms`