From 21e3b1d286d6cafd2c5c775c04e8d95069156888 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 20 Aug 2026 19:04:06 +0900 Subject: [PATCH] fix(strix): isolate scanner output from scan target --- CHANGELOG.md | 7 ++ docs/doctoring/strix-scan-working-boundary.md | 56 +++++++++++++++ scripts/ci/strix_quick_gate.sh | 62 +++++++++++++++-- scripts/ci/test_strix_quick_gate.sh | 68 ++++++++++++++++++- 4 files changed, 185 insertions(+), 8 deletions(-) create mode 100644 docs/doctoring/strix-scan-working-boundary.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d2f9f24dc..b4e0d809a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,13 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- Run Strix with an explicit canonical scan target from a temporary working + directory outside that target, so scanner state and relative reports cannot + become self-scanned source findings; preserve those reports as gate evidence. + PR-scoped Python scans also include the PostgreSQL introspection security + helpers when that package exists in the target repository. PR scopes now live + below the gate's private runtime directory so unrelated temporary-file + cleanup cannot remove scan input during PR-head materialization. - Parsed `opencode.jsonc` as JSONC (stripping `//` and `/* */` comments outside string literals) in the reasoning-effort guard and its contract tests, instead of raw `json.loads`, which rejected the file the moment it carried its first explanatory comment (added for the `contextual-orchestrator` provider block) with `Expecting property name enclosed in double quotes`. Comment markers inside string values, such as the `$schema` URL, are left untouched. - Download the pinned `uv` 0.12.1 exporter from the official GitHub Releases URL instead of `releases.astral.sh`, which now returns HTTP 403 and blocks org-wide OpenCode `coverage-evidence`. The SHA-256 pin is unchanged. The opener may follow one hop onto `release-assets.githubusercontent.com` or `objects.githubusercontent.com` and still rejects every other host, userinfo, non-HTTPS scheme, and nondefault port (ContextualWisdomLab/.github#1109). - Compared the trusted `uv` executable's post-install `--version` output against the real GitHub Releases build's full string, `uv 0.12.1 (x86_64-unknown-linux-gnu)`, instead of the bare `uv 0.12.1` the prior check required; the genuine release binary always prints the target triple, so every installation was failing the pin check immediately after the archive download itself was fixed (ContextualWisdomLab/.github#1109). diff --git a/docs/doctoring/strix-scan-working-boundary.md b/docs/doctoring/strix-scan-working-boundary.md new file mode 100644 index 0000000000..f73644c56f --- /dev/null +++ b/docs/doctoring/strix-scan-working-boundary.md @@ -0,0 +1,56 @@ +# Strix scan working-directory boundary + +## Problem + +The organization Strix gate bounded pull-request scans to a temporary scope, +but launched Strix with that scope as its current working directory. Strix +could therefore create `strix_runs/` and state files inside the tree it was +scanning. A self-generated state file was reported as a critical hard-coded +credential in a current-head `pg-erd-cloud` scan, while another scan reported a +missing unchanged DSN guard because the bounded scope omitted an imported +security helper. + +## Decision + +The gate now passes the canonical target directory as Strix's absolute `-t` +argument and runs the process from a fresh runner-temporary directory outside +the target. The temporary `strix_runs/` output is copied into the existing +active report directory after each attempt, so report classification and +artifact publication retain their previous evidence contract. The target is +never inferred from the working directory. + +When a changed backend Python file belongs to a repository that contains +`backend/app/pg_introspect`, the bounded scope includes the package's available +trusted base helpers, including `dsn_guard.py` and `introspect.py`. Repositories +without that package are unchanged. + +The bounded scope itself is created below the gate's private runtime directory. +The gate therefore owns the scope lifetime and an unrelated temporary-file +cleanup cannot remove scan input during PR-head blob materialization. + +## Verification and rollback + +`scripts/ci/test_strix_quick_gate.sh` verifies both the absolute target and the +outside working directory. It also verifies that a PostgreSQL DSN guard is +available to a scoped introspection scan. Run the shell syntax check and the +Strix quick-gate harness before publishing a central workflow change. Rollback +is a normal revert of the central PR; do not suppress changed-file attribution +or ignore scanner output to make a check green. + +The fix addresses the trust boundary between untrusted scan input and scanner +output. It does not replace exact-head review, vulnerability remediation, or +the required security workflow. + +## References + +National Institute of Standards and Technology. (2022). *Secure software +development framework (SSDF) version 1.1: Recommendations for mitigating the +risk of software vulnerabilities* (NIST Special Publication 800-218). +https://doi.org/10.6028/NIST.SP.800-218 + +MITRE. (n.d.). *CWE-22: Improper limitation of a pathname to a restricted +directory ('Path traversal')*. Common Weakness Enumeration. +https://cwe.mitre.org/data/definitions/22.html + +MITRE. (n.d.). *CWE-367: Time-of-check time-of-use (TOCTOU) race condition*. +Common Weakness Enumeration. https://cwe.mitre.org/data/definitions/367.html diff --git a/scripts/ci/strix_quick_gate.sh b/scripts/ci/strix_quick_gate.sh index 0f37f34605..ab3ad64c54 100755 --- a/scripts/ci/strix_quick_gate.sh +++ b/scripts/ci/strix_quick_gate.sh @@ -28,6 +28,8 @@ STRIX_RUNTIME_DIR="$(mktemp -d /tmp/strix-runtime.XXXXXX)" STRIX_LOG="$STRIX_RUNTIME_DIR/strix.log" ACTIVE_REPORTS_DIR="$STRIX_RUNTIME_DIR/reports" ATTEMPT_LOGS_DIR="$STRIX_RUNTIME_DIR/gate-attempts" +STRIX_SCAN_WORKING_DIR="$STRIX_RUNTIME_DIR/scan-cwd" +STRIX_SCAN_OUTPUT_DIR="$STRIX_SCAN_WORKING_DIR/strix_runs" STRIX_REPORTS_DIR="$ACTIVE_REPORTS_DIR" STRIX_PROCESS_TIMEOUT_SECONDS="${STRIX_PROCESS_TIMEOUT_SECONDS:-1200}" STRIX_TOTAL_TIMEOUT_SECONDS="${STRIX_TOTAL_TIMEOUT_SECONDS:-0}" @@ -136,6 +138,9 @@ publish_artifact_reports() { cp -R -- "$scope_reports_dir"/. "$ARTIFACT_REPORTS_DIR"/ fi done + if [ -d "$STRIX_SCAN_OUTPUT_DIR" ] && [ ! -L "$STRIX_SCAN_OUTPUT_DIR" ]; then + cp -R -- "$STRIX_SCAN_OUTPUT_DIR"/. "$ARTIFACT_REPORTS_DIR"/ + fi } preserve_attempt_log() { @@ -233,6 +238,16 @@ cleanup_runtime() { trap cleanup_runtime EXIT INT TERM +make_pull_request_scope_dir() { + local scope_parent="$STRIX_RUNTIME_DIR/pr-scopes" + if [ -L "$scope_parent" ]; then + echo "ERROR: pull request scope parent must not be a symlink." >&2 + return 2 + fi + mkdir -p -- "$scope_parent" + mktemp -d "$scope_parent/strix-pr-scope.XXXXXX" +} + STRIX_LLM_FILE="${STRIX_LLM_FILE:-}" if [ -z "$STRIX_LLM_FILE" ]; then echo "ERROR: STRIX_LLM_FILE must reference a regular file containing the model." >&2 @@ -1255,6 +1270,22 @@ backend/services/llm_provider_urls.py backend/services/text_safety.py backend/services/threading_service.py EOF + # PostgreSQL introspection helpers are a security boundary for repositories + # that expose this package. Include their trusted base copies when present; + # the conditional keeps the shared gate usable by repositories without it. + local context_file + for context_file in \ + backend/app/pg_introspect/__init__.py \ + backend/app/pg_introspect/column_examples.py \ + backend/app/pg_introspect/dsn_guard.py \ + backend/app/pg_introspect/forward_ddl.py \ + backend/app/pg_introspect/introspect.py \ + backend/app/pg_introspect/queries.py \ + backend/app/pg_introspect/snapshot_collect.py; do + if [ -f "$REPO_ROOT/$context_file" ] && [ ! -L "$REPO_ROOT/$context_file" ]; then + printf '%s\n' "$context_file" + fi + done fi if [ "$needs_frontend_email_api_context" -eq 1 ]; then @@ -1302,7 +1333,7 @@ changed_file_list_contains() { build_pull_request_scope_dir() { local scope_dir - scope_dir="$(mktemp -d "${TMPDIR:-/tmp}/strix-pr-scope.XXXXXX")" + scope_dir="$(make_pull_request_scope_dir)" || return 2 scope_dir="$({ CDPATH='' && cd -P -- "$scope_dir" && pwd -P; })" PULL_REQUEST_SCOPE_DIRS+=("$scope_dir") @@ -1475,7 +1506,7 @@ PY build_pull_request_head_tree_scope_dir() { local scope_dir - scope_dir="$(mktemp -d "${TMPDIR:-/tmp}/strix-pr-scope.XXXXXX")" + scope_dir="$(make_pull_request_scope_dir)" || return 2 scope_dir="$({ CDPATH='' && cd -P -- "$scope_dir" && pwd -P; })" PULL_REQUEST_SCOPE_DIRS+=("$scope_dir") @@ -2375,7 +2406,7 @@ run_strix_once() { STRIX_CHILD_EXECUTABLE_ROOT="$STRIX_EXECUTABLE_ROOT" \ STRIX_CHILD_EXECUTABLE_SHA256="$STRIX_EXECUTABLE_SHA256" \ STRIX_CHILD_REQUIRE_EXECUTABLE_INTEGRITY="${IS_PR_EVIDENCE_RUN:-false}" \ - python3 - "$timeout_seconds" "$resolved_target_path" "$SCAN_MODE" "$STRIX_LOG" <<'PY' +python3 - "$timeout_seconds" "$resolved_target_path" "$SCAN_MODE" "$STRIX_LOG" "$STRIX_SCAN_WORKING_DIR" <<'PY' import hashlib import hmac import os @@ -2389,6 +2420,7 @@ timeout_seconds = int(sys.argv[1]) target_path = sys.argv[2] scan_mode = sys.argv[3] log_path = pathlib.Path(sys.argv[4]) +scan_working_dir = pathlib.Path(sys.argv[5]) # Failure classifiers read this path even when trusted executable or target # validation fails before a child process starts. Materialize it first so the # primary log shows one configuration error instead of repeated grep noise. @@ -2528,12 +2560,29 @@ if any(ch in str(target_cwd) for ch in ("\x00", "\n", "\r")): sys.stderr.write("ERROR: Strix target path contains unsupported control characters.\n") raise SystemExit(2) -command = [resolved_strix_bin, "-n", "-t", ".", "--scan-mode", scan_mode] +if scan_working_dir.is_symlink(): + sys.stderr.write("ERROR: Strix scan working directory must not be a symlink.\n") + raise SystemExit(2) +scan_working_dir.mkdir(parents=True, exist_ok=True) +scan_output_dir = scan_working_dir / "strix_runs" +if scan_output_dir.is_symlink(): + sys.stderr.write("ERROR: Strix scan output directory must not be a symlink.\n") + raise SystemExit(2) +if scan_output_dir.exists(): + import shutil + + shutil.rmtree(scan_output_dir) +scan_output_dir.mkdir() + +# Keep scanner-created state and relative report files outside the untrusted +# scan target. The target remains explicit and absolute, so changing cwd cannot +# change which source tree is scanned. +command = [resolved_strix_bin, "-n", "-t", str(target_cwd), "--scan-mode", scan_mode] try: process = subprocess.Popen( command, - cwd=str(target_cwd), + cwd=str(scan_working_dir), stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True, @@ -2566,6 +2615,9 @@ except subprocess.TimeoutExpired: PY rc=$? set -e + if [ -d "$STRIX_SCAN_OUTPUT_DIR" ] && [ ! -L "$STRIX_SCAN_OUTPUT_DIR" ]; then + cp -R -- "$STRIX_SCAN_OUTPUT_DIR"/. "$ACTIVE_REPORTS_DIR"/ + fi local end_epoch end_epoch="$(date +%s)" local elapsed=$((end_epoch - start_epoch)) diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index ac9ce1d8bd..8f4ab3367d 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -478,9 +478,12 @@ assert_strix_llm_file_read_is_literal_data() { } assert_strix_child_target_uses_constant_argument() { - assert_file_contains "$GATE_SCRIPT" 'command = [resolved_strix_bin, "-n", "-t", ".", "--scan-mode", scan_mode]' "strix gate passes a constant target argument to the child process" - assert_file_contains "$GATE_SCRIPT" 'cwd=str(target_cwd)' "strix gate runs the child process from the canonical target directory" - assert_file_not_contains "$GATE_SCRIPT" 'command = [resolved_strix_bin, "-n", "-t", target_path, "--scan-mode", scan_mode]' "strix gate must not forward raw target paths as child arguments" + assert_file_contains "$GATE_SCRIPT" 'command = [resolved_strix_bin, "-n", "-t", str(target_cwd), "--scan-mode", scan_mode]' "strix gate passes the canonical target argument to the child process" + assert_file_contains "$GATE_SCRIPT" 'cwd=str(scan_working_dir)' "strix gate runs the child process outside the scan target" + assert_file_contains "$GATE_SCRIPT" 'make_pull_request_scope_dir()' "strix gate creates PR scopes under its private runtime directory" + assert_file_contains "$GATE_SCRIPT" 'scope_parent="$STRIX_RUNTIME_DIR/pr-scopes"' "strix gate keeps PR scopes inside the private runtime directory" + assert_file_not_contains "$GATE_SCRIPT" 'command = [resolved_strix_bin, "-n", "-t", ".", "--scan-mode", scan_mode]' "strix gate must not rely on the child cwd as its scan target" + assert_file_not_contains "$GATE_SCRIPT" 'cwd=str(target_cwd)' "strix gate must not run the child process inside the scan target" } assert_opencode_review_uses_codegraph_and_gpt5_fallback() { @@ -3302,6 +3305,18 @@ success|runtime-env-forwarding|vertex-primary-success-timing-message|direct-open echo "scan ok" exit 0 ;; + scan-working-directory-isolated) + if [ "$PWD" = "$target_path" ] || [[ "$PWD" == "$target_path"/* ]]; then + echo "Error: Strix process inherited the untrusted scan target as cwd" >&2 + exit 81 + fi + if [ ! -f "$target_path/backend/app/pg_introspect/dsn_guard.py" ]; then + echo "Error: PostgreSQL DSN guard context missing from PR scope" >&2 + exit 82 + fi + echo "scan ok with isolated Strix working directory" + exit 0 + ;; success-with-critical-report) mkdir -p "$STRIX_REPORTS_DIR/fake-success/vulnerabilities" cat >"$STRIX_REPORTS_DIR/fake-success/vulnerabilities/vuln-0001.md" <<'REPORT' @@ -5404,6 +5419,10 @@ EOS for large_scope_index in $(seq 1 38); do printf 'file %s\n' "$large_scope_index" >"$repo_root_dir/backend/large-scope/file-$large_scope_index.py" done + elif [ "$scenario" = "scan-working-directory-isolated" ]; then + mkdir -p "$repo_root_dir/backend/app/pg_introspect" + printf '%s\n' 'HEAD_INTROSPECT_SHOULD_BE_SCANNED' >"$repo_root_dir/backend/app/pg_introspect/introspect.py" + printf '%s\n' 'TRUSTED_DSN_GUARD_CONTEXT_SHOULD_BE_SCANNED' >"$repo_root_dir/backend/app/pg_introspect/dsn_guard.py" fi local scenario_base_sha="" @@ -6276,6 +6295,28 @@ run_filtered_gate_case_if_requested() { "Materialized PR-head changed-file scope" \ "repository_dispatch" ;; + scan-working-directory-isolated) + run_gate_case "scan-working-directory-isolated" \ + "openai/gpt-4o-mini" \ + "" \ + "0" \ + "scan ok with isolated Strix working directory" \ + "1" \ + "openai/gpt-4o-mini" \ + "https://example.invalid" \ + "vertex_ai" \ + "__DEFAULT__" \ + "" \ + "0" \ + "CRITICAL" \ + "0" \ + "" \ + "" \ + "1200" \ + "0" \ + "pull_request" \ + "backend/app/pg_introspect/introspect.py" + ;; *) record_failure "unknown STRIX_TEST_CASE_FILTER '${STRIX_TEST_CASE_FILTER:-}'" ;; @@ -10707,6 +10748,27 @@ run_gate_case "pr-changed-scope-bounded" \ "pull_request" \ "sync-module-system/smart-crawling-biz/src/main/java/org/empasy/sync/modules/system/controller/SysPositionController.java" +run_gate_case "scan-working-directory-isolated" \ + "openai/gpt-4o-mini" \ + "" \ + "0" \ + "scan ok with isolated Strix working directory" \ + "1" \ + "openai/gpt-4o-mini" \ + "https://example.invalid" \ + "vertex_ai" \ + "__DEFAULT__" \ + "" \ + "0" \ + "CRITICAL" \ + "0" \ + "" \ + "" \ + "1200" \ + "0" \ + "pull_request" \ + "backend/app/pg_introspect/introspect.py" + run_gate_case "pr-python-scope-context" \ "openai/gpt-4o-mini" \ "" \