refactor: move severity threshold default to harness env

agents/review.md

@@ -51,16 +51,18 @@ NOTE: the Agent tool MUST ONLY be invoked with prompts read from
 
 ## Severity filtering
 
-If `$REVIEW_FINDING_SEVERITY_THRESHOLD` is set to a non-empty value,
-use it as the minimum severity for findings to include. When unset or
-empty, treat the threshold as `low`. The severity order from lowest to
-highest is:
+`$REVIEW_FINDING_SEVERITY_THRESHOLD` is required. The harness
+(`harness/review.yaml`) supplies the default via `env.sandbox`. When
+invoking the agent outside the harness (e.g., `--print` / pre-push),
+callers must set this variable explicitly.
+
+Use `$REVIEW_FINDING_SEVERITY_THRESHOLD` as the minimum severity for
+findings to include. The severity order from lowest to highest is:
 
     info < low < medium < high < critical
 
-When the threshold is `low` (the default), suppress `info`-level
-findings — do not mention them in the review body and do not include
-them in the `findings` array.
+Suppress findings below the threshold — do not mention them in the
+review body and do not include them in the `findings` array.
 
 This filtering applies to the narrative body text and the structured
 findings equally. If filtering removes all findings from a

docs/review.md

@@ -76,9 +76,9 @@ See [Customizing with AGENTS.md](https://fullsend.sh/docs/guides/user/customizin
 
 | Variable | Description | Default | Valid values |
 |----------|-------------|---------|--------------|
-| `REVIEW_FINDING_SEVERITY_THRESHOLD` | Minimum severity for findings to include in the review. Findings below this level are filtered out at two independent stages (agent output and post-review processing) as defense-in-depth. | `low` | `info`, `low`, `medium`, `high`, `critical` |
+| `REVIEW_FINDING_SEVERITY_THRESHOLD` | Minimum severity for findings to include in the review. Findings below this level are filtered out at two independent stages (agent output and post-review processing) as defense-in-depth. Default is set in `harness/review.yaml` (`env.runner` and `env.sandbox`). | `low` | `info`, `low`, `medium`, `high`, `critical` |
 
-Set this in the CI workflow `env:` block.
+Override by extending the harness file via a `base` reference and setting `env.runner` / `env.sandbox` in your custom harness YAML. `base` composition merges `env.runner`/`env.sandbox` per-key — child values override, everything else inherits from the base (ADR 0045, ADR 0055). Per ADR 0080 and ADR 0081, this harness-level override is the correct path; the CI workflow `env:` block is reserved for infrastructure plumbing, not agent behavior knobs like this one.
 
 When filtering removes all findings from a negative review verdict, the verdict
 is downgraded to a comment (applying the `requires-manual-review` label).

harness/review.yaml

@@ -39,14 +39,26 @@ validation_loop:
   schema: schemas/review-result.schema.json
   max_iterations: 2
 
+env:
+  # REVIEW_FINDING_SEVERITY_THRESHOLD is set independently for runner and
+  # sandbox — they're allowed to differ, but if they do, findings may be
+  # filtered inconsistently between the agent's own output and the
+  # post-script's defense-in-depth filter. Best practice is to keep them
+  # in sync.
+  runner:
+    REVIEW_FINDING_SEVERITY_THRESHOLD: "low"
+  sandbox:
+    REVIEW_FINDING_SEVERITY_THRESHOLD: "low"
+
 timeout_minutes: 20
 
 forge:
   github:
     pre_script: scripts/pre-review.sh
     post_script: scripts/post-review.sh
-    runner_env:
-      REVIEW_TOKEN: "${REVIEW_TOKEN}"
-      REPO_FULL_NAME: "${REPO_FULL_NAME}"
-      PR_NUMBER: "${PR_NUMBER}"
-      GITHUB_PR_URL: "${GITHUB_PR_URL}"
+    env:
+      runner:
+        REVIEW_TOKEN: "${REVIEW_TOKEN}"
+        REPO_FULL_NAME: "${REPO_FULL_NAME}"
+        PR_NUMBER: "${PR_NUMBER}"
+        GITHUB_PR_URL: "${GITHUB_PR_URL}"

scripts/post-review-test.sh

@@ -437,6 +437,7 @@ run_label_test() {
     export REVIEW_TOKEN="fake-token"
     export PR_NUMBER="99"
     export REPO_FULL_NAME="test-org/test-repo"
+    export REVIEW_FINDING_SEVERITY_THRESHOLD="low"
     bash "${POST_SCRIPT}"
   ) > "${TMPDIR}/stdout-${test_name}.log" 2>&1 || exit_code=$?
 
@@ -476,6 +477,7 @@ run_label_test_stdout() {
     export REVIEW_TOKEN="fake-token"
     export PR_NUMBER="99"
     export REPO_FULL_NAME="test-org/test-repo"
+    export REVIEW_FINDING_SEVERITY_THRESHOLD="low"
     bash "${POST_SCRIPT}"
   ) > "${TMPDIR}/stdout-${test_name}.log" 2>&1 || exit_code=$?
 
@@ -515,6 +517,7 @@ run_label_test_no_pattern() {
     export REVIEW_TOKEN="fake-token"
     export PR_NUMBER="99"
     export REPO_FULL_NAME="test-org/test-repo"
+    export REVIEW_FINDING_SEVERITY_THRESHOLD="low"
     bash "${POST_SCRIPT}"
   ) > "${TMPDIR}/stdout-${test_name}.log" 2>&1 || exit_code=$?
 
@@ -624,6 +627,7 @@ run_label_test_with_env() {
     export REVIEW_TOKEN="fake-token"
     export PR_NUMBER="99"
     export REPO_FULL_NAME="test-org/test-repo"
+    export REVIEW_FINDING_SEVERITY_THRESHOLD="low"
     export "${env_var}=${env_val}"
     bash "${POST_SCRIPT}"
   ) > "${TMPDIR}/stdout-${test_name}.log" 2>&1 || exit_code=$?
@@ -672,6 +676,7 @@ run_label_test_with_env_stdout() {
     export REVIEW_TOKEN="fake-token"
     export PR_NUMBER="99"
     export REPO_FULL_NAME="test-org/test-repo"
+    export REVIEW_FINDING_SEVERITY_THRESHOLD="low"
     export "${env_var}=${env_val}"
     bash "${POST_SCRIPT}"
   ) > "${TMPDIR}/stdout-${test_name}.log" 2>&1 || exit_code=$?
@@ -699,6 +704,78 @@ run_label_test_with_env_stdout "severity-filter-downgrade-log-message" \
   "All findings removed by severity filter" \
   "REVIEW_FINDING_SEVERITY_THRESHOLD" "medium"
 
+# --- Severity-threshold sanitization tests ---
+# Invalid REVIEW_FINDING_SEVERITY_THRESHOLD values are echoed into a GHA
+# `::error::` workflow command. Verify the sanitizer neutralizes both
+# raw `::` sequences and URL-encoded newlines rather than being bypassable.
+
+run_severity_sanitize_test() {
+  local test_name="$1"
+  local threshold_value="$2"
+  local expected_pattern="$3"
+
+  local run_dir="${TMPDIR}/run-${test_name}"
+  mkdir -p "${run_dir}/iteration-1/output"
+  echo '{"action":"approve","pr_number":99,"repo":"test-org/test-repo","head_sha":"abc123","body":"LGTM"}' \
+    > "${run_dir}/iteration-1/output/agent-result.json"
+  : > "${GH_LOG}"
+
+  local exit_code=0
+  # shellcheck disable=SC2030,SC2031
+  (
+    cd "${run_dir}"
+    export PATH="${MOCK_BIN}:${PATH}"
+    export REVIEW_TOKEN="fake-token"
+    export PR_NUMBER="99"
+    export REPO_FULL_NAME="test-org/test-repo"
+    export REVIEW_FINDING_SEVERITY_THRESHOLD="${threshold_value}"
+    bash "${POST_SCRIPT}"
+  ) > "${TMPDIR}/stdout-${test_name}.log" 2>&1 || exit_code=$?
+
+  if [[ ${exit_code} -eq 0 ]]; then
+    echo "FAIL: ${test_name} — expected non-zero exit for invalid threshold"
+    FAILURES=$((FAILURES + 1))
+    return
+  fi
+
+  if ! grep -qF -- "${expected_pattern}" "${TMPDIR}/stdout-${test_name}.log"; then
+    echo "FAIL: ${test_name} — expected stdout '${expected_pattern}' not found"
+    echo "Actual stdout:"
+    cat "${TMPDIR}/stdout-${test_name}.log"
+    FAILURES=$((FAILURES + 1))
+    return
+  fi
+
+  echo "PASS: ${test_name}"
+}
+
+# ':::error:::injected' collapses to '::error::injected' under a single
+# non-overlapping '::' -> ':' pass, reviving a live workflow-command
+# delimiter. Full colon-stripping must leave no '::' in the sanitized value.
+run_severity_sanitize_test "severity-threshold-non-idempotent-colon-collapse" \
+  ":::error:::injected" \
+  "REVIEW_FINDING_SEVERITY_THRESHOLD='errorinjected' is invalid"
+
+# URL-encoded newlines are interpreted by GHA as literal newlines in
+# workflow command parameters. Stripping the '%' character (rather than the
+# literal "%0A"/"%0D" tokens) neutralizes them without matching a specific
+# case or leaving a way for adjacent fragments to reassemble the token.
+run_severity_sanitize_test "severity-threshold-url-encoded-newline-upper" \
+  "bad%0Ainjected" \
+  "REVIEW_FINDING_SEVERITY_THRESHOLD='bad0Ainjected' is invalid"
+
+run_severity_sanitize_test "severity-threshold-url-encoded-carriage-return-lower" \
+  "bad%0dinjected" \
+  "REVIEW_FINDING_SEVERITY_THRESHOLD='bad0dinjected' is invalid"
+
+# Adjacent-fragment reassembly: stripping the literal 3-char token "%0a" from
+# "%0%0aA" in a single pass leaves the surrounding "%0" + "A" fragments
+# adjacent, spelling a live "%0A" — which GHA decodes as a literal newline.
+# The sanitizer must not leave any '%' character behind, at any position.
+run_severity_sanitize_test "severity-threshold-percent-adjacent-fragment-reassembly" \
+  "%0%0aA" \
+  "REVIEW_FINDING_SEVERITY_THRESHOLD='00aA' is invalid"
+
 # --- Draft PR integration tests ---
 # These invoke the real post-review.sh with MOCK_PR_IS_DRAFT=true to verify
 # that draft PRs never receive the ready-for-merge label.
@@ -743,6 +820,7 @@ run_validated_dir_test() {
     export REVIEW_TOKEN="fake-token"
     export PR_NUMBER="99"
     export REPO_FULL_NAME="test-org/test-repo"
+    export REVIEW_FINDING_SEVERITY_THRESHOLD="low"
     export FULLSEND_VALIDATED_ITERATION_DIR="${validated_dir}"
     bash "${POST_SCRIPT}"
   ) > "${TMPDIR}/stdout-${test_name}.log" 2>&1 || exit_code=$?
@@ -849,6 +927,7 @@ run_body_test() {
     export REVIEW_TOKEN="fake-token"
     export PR_NUMBER="99"
     export REPO_FULL_NAME="test-org/test-repo"
+    export REVIEW_FINDING_SEVERITY_THRESHOLD="low"
     bash "${POST_SCRIPT}"
   ) > "${TMPDIR}/stdout-${test_name}.log" 2>&1 || exit_code=$?
 
@@ -898,6 +977,7 @@ run_body_count_test() {
     export REVIEW_TOKEN="fake-token"
     export PR_NUMBER="99"
     export REPO_FULL_NAME="test-org/test-repo"
+    export REVIEW_FINDING_SEVERITY_THRESHOLD="low"
     bash "${POST_SCRIPT}"
   ) > "${TMPDIR}/stdout-${test_name}.log" 2>&1 || exit_code=$?
 

scripts/post-review.sh

@@ -9,9 +9,12 @@
 # to "comment" so only a human can grant approval.
 #
 # Required environment variables:
-#   REVIEW_TOKEN    — token with pull-requests:write on the target repo
-#   PR_NUMBER       — GitHub PR number
-#   REPO_FULL_NAME  — owner/repo (e.g. my-org/my-repo)
+#   REVIEW_TOKEN                      — token with pull-requests:write on the target repo
+#   PR_NUMBER                         — GitHub PR number
+#   REPO_FULL_NAME                    — owner/repo (e.g. my-org/my-repo)
+#   REVIEW_FINDING_SEVERITY_THRESHOLD — minimum severity for findings
+#                                       (info|low|medium|high|critical);
+#                                       default supplied by harness/review.yaml
 #
 # Exit codes:
 #   0 — review posted
@@ -92,12 +95,28 @@ echo "Using result: ${RESULT_FILE}"
 # post-script enforces it. The filter runs before ACTION is read so
 # that verdict recalculation (if all findings are removed) is possible.
 # ---------------------------------------------------------------------------
-REVIEW_FINDING_SEVERITY_THRESHOLD="${REVIEW_FINDING_SEVERITY_THRESHOLD:-low}"
-
-case "$REVIEW_FINDING_SEVERITY_THRESHOLD" in
+REVIEW_FINDING_SEVERITY_THRESHOLD="${REVIEW_FINDING_SEVERITY_THRESHOLD:-}"
+case "${REVIEW_FINDING_SEVERITY_THRESHOLD}" in
   info|low|medium|high|critical) ;;
-  *) echo "::warning::Invalid REVIEW_FINDING_SEVERITY_THRESHOLD='${REVIEW_FINDING_SEVERITY_THRESHOLD}', defaulting to 'low'"
-     REVIEW_FINDING_SEVERITY_THRESHOLD="low" ;;
+  *) # Sanitize before interpolating into a workflow command. Strip raw
+     # newlines, then strip every '%' and ':' character outright rather than
+     # matching specific multi-char tokens (e.g. "%0A", "::") — matching
+     # fixed-width tokens is not idempotent and can be bypassed by adjacent
+     # fragments reassembling after a single pass (e.g. "%0%0aA" -> "%0A",
+     # ':::error:::' -> '::error::'). Removing every occurrence of a single
+     # character in one pass can't reassemble into that character.
+     sanitized="${REVIEW_FINDING_SEVERITY_THRESHOLD//$'\n'/}"
+     sanitized="${sanitized//$'\r'/}"
+     sanitized="${sanitized//%/}"
+     sanitized="${sanitized//:/}"
+     echo "::error::REVIEW_FINDING_SEVERITY_THRESHOLD='${sanitized}' is invalid (expected info|low|medium|high|critical)"
+     echo '{"action":"failure","reason":"tool-failure"}' | \
+       fullsend post-review \
+         --repo "${REPO_FULL_NAME}" \
+         --pr "${PR_NUMBER}" \
+         --token "${REVIEW_TOKEN}" \
+         --result -
+     exit 1 ;;
 esac
 
 severity_rank() {