Make the two taint engines agree

CLAUDE.md

@@ -129,7 +129,7 @@ include/naab/       All headers
 - `src/runtime/governance_engine.cpp` — main engine, signature verification
 - `src/runtime/governance_checks.cpp` — 50+ individual checks
 - `src/runtime/governance_config.cpp` — config loading from govern.json
-- `src/runtime/governance_taint.cpp` — taint tracking (interpreter path)
+- `src/interpreter/governance_taint.cpp` — taint tracking (tree-walker path; the VM carries taint on `taint_stack_` in `vm.cpp`). The two are independent implementations and must agree — `tests/governance_v4/test_taint_engine_parity.sh` is the only thing that checks it. `tests/differential/` cannot: `diff_runner.py` runs both engines with `--no-governance`, so taint is switched off there by construction. `expressionContainsTaint()` walks the AST and returns **false for any node type it does not handle**, so a new value-producing `Expr` subclass silently launders taint until added. Violations print in TWO formats — `checkTaintedSink()` (named variable, rule `taint_tracking.sink_violation`) and `checkExpressionTaintedSink()`'s expression path (`Taint tracking violation: expression contains untrusted data...`) — grepping for only one makes a working engine look broken.
 - `src/runtime/trust_store.cpp` — Ed25519 trusted key management
 - `src/runtime/crypto_utils.cpp` — Ed25519 sign/verify, SHA-256
 - Behavioral contracts: `must_call` (function must call specified functions — regex `\bname\s*\(` on body text, non-transitive), `must_contain` (function body must match syntax patterns), `must_produce` (golden tests with type-strict comparison — string "0" does not match int 0), `min_arity`/`max_arity` (parameter count enforcement)

docs/governance-campaign-findings.md

@@ -346,6 +346,58 @@ script reported something other than what happened.
 
 ---
 
+## Taint tracking: two defects that hid each other
+
+Found by tracing the taint tracker cold, after the propose/commit path was
+exhausted. Taint is implemented **twice** — the tree-walker walks the AST
+(`expressionContainsTaint`), the VM carries it on `taint_stack_` — and nothing
+checked that the two agree. A differential run over 12 scenarios found two
+divergences, in opposite directions, each masking the other.
+
+**14. The declaration after a taint source inherited its taint.**
+`checkRhsTainted()` clears `lastReturnWasTainted` on entry, then *sets it again*
+when it identifies a direct source, so `VarDeclStmt` can read `lastTaintSource()`
+for lineage. Nothing cleared it afterwards, so the next declaration's step-3
+check picked it up:
+
+```naab
+let t = env.get("HOME")      // legitimately tainted
+let c = "a clean literal"    // <- silently tainted by the stale flag
+```
+
+A false positive on clean data, which makes every taint count untrustworthy —
+including `L25-03`'s baseline. It presented intermittently: with more statements
+in between, the stale flag lands on a throwaway variable that never reaches a
+sink. The identical hazard was already fixed for `ExprStmt` (`V13-S7`); the
+declaration path was missed.
+
+**15. `try` as an expression laundered taint.** `expressionContainsTaint()` did
+not handle `TryCatchExpr`, so `let x = try { tainted } catch (e) { "" }`
+produced an untainted `x`. The walk returns **false for any node type it does
+not handle**, so this is fail-open by construction — `YieldExpr` was missing too.
+
+**They cancelled.** Defect 14 was tainting the try-expression anyway, so 15 was
+invisible; a test of the try case passed *because of a bug*. Fixing 14 exposed
+15 as a fresh divergence. The vacuity check shows it directly: with both
+reverted `try_expr` **passes**, and it only fails once the stale-flag fix is in.
+
+**Why nothing caught them.** `tests/differential/` exists precisely to catch
+VM/tree-walker divergence, but `diff_runner.py` runs `--no-governance` — taint
+is switched off there by construction. The property suite's
+`test_taint_monotonicity.naab` runs on one engine, so a divergence is invisible
+to it too. `test_taint_engine_parity.sh` closes the gap, and its pass condition
+is agreement **plus** a per-engine expectation: two engines that both miss a
+taint agree perfectly.
+
+**A retraction.** A third finding — "the tree-walker misses inline subscripts at
+sinks" — was wrong, and was a measurement artifact. Violations print in two
+formats (`checkTaintedSink` names the variable; `checkExpressionTaintedSink`'s
+expression path does not), and the detector matched only one, so a working
+engine looked broken. Fourth instance in this campaign of a *detector* carrying
+the defect it was hunting; see the method note on keyword filters.
+
+---
+
 ## Recorded, deliberately not changed
 
 Each of these is a real observation that did **not** justify a change. They are

run-all-tests.sh

@@ -1787,6 +1787,21 @@ else
 fi
 
 # Evidence chain hardening (cross-run continuity, decision snapshots, transcript refs)
+# Taint tracking is implemented twice (AST walk vs VM taint_stack_); this is the
+# only thing that checks the two agree. tests/differential/ cannot — it runs
+# --no-governance, so taint is switched off there.
+TAINT_PARITY_SCRIPT="tests/governance_v4/test_taint_engine_parity.sh"
+if [ -f "$TAINT_PARITY_SCRIPT" ]; then
+    if run_shell_test "$TAINT_PARITY_SCRIPT" 2>&1; then
+        echo "  test_taint_engine_parity.sh: ALL PASSED"
+    else
+        FAILED=$((FAILED + 1))
+        FAILED_TESTS+=("test_taint_engine_parity.sh")
+    fi
+else
+    echo "  test_taint_engine_parity.sh: not found, skipping"
+fi
+
 EVIDENCE_CHAIN_SCRIPT="tests/governance_v4/test_evidence_chain.sh"
 if [ -f "$EVIDENCE_CHAIN_SCRIPT" ]; then
     if run_shell_test "$EVIDENCE_CHAIN_SCRIPT" 2>&1; then

src/interpreter/governance_taint.cpp

@@ -130,8 +130,25 @@ bool Interpreter::expressionContainsTaint(ast::Expr* expr) {
                expressionContainsTaint(rng->getEnd());
     }
 
+    // try/catch as an EXPRESSION: `let x = try { tainted } catch (e) { "" }`
+    // produces a value, so taint must flow through both arms exactly as it does
+    // through IfExpr above. This was missing, and was masked by a second defect:
+    // VarDeclStmt left lastReturnWasTainted set, so the next declaration was
+    // tainted by the stale flag whatever it contained — which happened to cover
+    // this case. Fixing that leak exposed this one. Two bugs cancelling.
+    if (auto* tc = dynamic_cast<ast::TryCatchExpr*>(expr)) {
+        return expressionContainsTaint(tc->getTryExpr()) ||
+               expressionContainsTaint(tc->getCatchExpr());
+    }
+
+    // Yield carries its operand's value out of a generator.
+    if (auto* yl = dynamic_cast<ast::YieldExpr*>(expr)) {
+        return expressionContainsTaint(yl->getExpr());
+    }
+
     // InlineCodeExpr: handled by isTaintSource("polyglot_output") in VarDeclStmt/Assignment
     // LambdaExpr: body is a block, not a value expression — N/A
+    // ThrowExpr: transfers control, never yields a value to an assignment — N/A
 
     // MatchExpr: check all arm body expressions for tainted returns (FIX for BUG-MatchExpr)
     if (auto* match = dynamic_cast<ast::MatchExpr*>(expr)) {

src/interpreter/interpreter.cpp

@@ -2557,6 +2557,22 @@ void Interpreter::visit(ast::VarDeclStmt& node) {
         if (checkRhsSanitized(node.getInit())) {
             governance_->clearTaint(node.getName());
         }
+        // Consume the stale lastReturnWasTainted flag, same reason as the V13-S7
+        // fix on ExprStmt above. checkRhsTainted() CLEARS the flag on entry and
+        // then SETS it again when it identifies a direct source, so that the
+        // lineage lookup above can read lastTaintSource(). Nothing cleared it
+        // afterwards, so the next declaration's step-3 check inherited it and
+        // was marked tainted regardless of its own expression:
+        //
+        //     let t = env.get("HOME")      // legitimately tainted
+        //     let c = "a clean literal"    // <- silently tainted by the stale flag
+        //
+        // It presented intermittently: with more statements in between, the flag
+        // lands on some throwaway variable that never reaches a sink, so the leak
+        // was invisible unless the very next declaration was the one that did.
+        // The VM is unaffected — it carries taint on taint_stack_ rather than
+        // through a cross-statement flag.
+        governance_->setLastReturnTainted(false);
     }
 
     // Phase 2.4.4: Type inference - if no type annotation, infer from value

tests/governance_v4/test_taint_engine_parity.sh

@@ -0,0 +1,215 @@
+#!/usr/bin/env bash
+# ============================================================
+# test_taint_engine_parity.sh — VM vs tree-walker taint agreement
+#
+# Taint tracking is implemented TWICE: the tree-walker walks the AST
+# (governance_taint.cpp) while the VM carries taint on taint_stack_. CLAUDE.md
+# says changes "must be made in both paths", and nothing checked that.
+#
+# tests/differential/ cannot: diff_runner.py runs both engines with
+# --no-governance, so it compares language semantics with taint switched off.
+# This is the governance-on counterpart.
+#
+# Two real defects were found by running this matrix, and they were cancelling
+# each other:
+#   1. VarDeclStmt left lastReturnWasTainted set, so the declaration AFTER a
+#      taint source was marked tainted whatever it contained — a false positive
+#      on clean data (clean_after_source below).
+#   2. expressionContainsTaint() did not handle TryCatchExpr, so a value
+#      produced by `try {} catch {}` lost its taint — a false negative
+#      (try_expr below), invisible while (1) was tainting it anyway.
+#
+# The pass condition is AGREEMENT plus a per-engine expectation, not agreement
+# alone: two engines that both miss a taint agree perfectly.
+# ============================================================
+set -uo pipefail
+
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+NAAB="$SCRIPT_DIR/../../build/naab-lang"
+
+if [ -d "/data/data/com.termux/files/usr/tmp" ]; then
+    _SYSTMP="${TMPDIR:-/data/data/com.termux/files/usr/tmp}"
+else
+    _SYSTMP="${TMPDIR:-/tmp}"
+fi
+TEST_TMP="${_SYSTMP}/taint-parity-$$"
+
+RED='\033[0;31m'; GREEN='\033[0;32m'; YELLOW='\033[1;33m'; CYAN='\033[0;36m'; NC='\033[0m'
+PASS_COUNT=0; FAIL_COUNT=0; SKIP_COUNT=0; FAILURES=""
+pass() { PASS_COUNT=$((PASS_COUNT + 1)); echo -e "  ${GREEN}PASS${NC} [$1] $2"; }
+fail() { FAIL_COUNT=$((FAIL_COUNT + 1)); echo -e "  ${RED}FAIL${NC} [$1] $2"; [ -n "${3:-}" ] && echo -e "       ${RED}-> $3${NC}"; FAILURES="${FAILURES}\n  [$1] $2"; }
+skip() { SKIP_COUNT=$((SKIP_COUNT + 1)); echo -e "  ${YELLOW}SKIP${NC} [$1] $2"; }
+
+source "$SCRIPT_DIR/../helpers/trust_setup.sh"
+setup_isolated_trust
+cleanup() { teardown_isolated_trust; rm -rf "$TEST_TMP"; }
+trap cleanup EXIT
+mkdir -p "$TEST_TMP"
+
+"$NAAB" --keygen "$TEST_TMP/k.pem" >/dev/null 2>&1
+"$NAAB" --trust-key "$TEST_TMP/k.pem.pub" 2>/dev/null
+export NAAB_SIGNING_KEY="$TEST_TMP/k.pem"
+
+cat > "$TEST_TMP/govern.json" << 'EOF'
+{
+  "version": "5.0",
+  "mode": "monitor",
+  "security": { "sandbox_level": "elevated" },
+  "taint_tracking": {
+    "enabled": true,
+    "level": "advisory",
+    "sources": ["env.get", "io.read_line", "file.read", "polyglot_output"],
+    "sinks": ["shell_exec", "python_exec", "file.write", "file.append"],
+    "sanitizers": ["validate_", "sanitize_", "escape_"]
+  }
+}
+EOF
+(cd "$TEST_TMP" && NAAB_SIGNING_KEY="$NAAB_SIGNING_KEY" "$NAAB" --sign-governance >/dev/null 2>&1) || true
+
+echo ""
+echo -e "${CYAN}+==============================================================+${NC}"
+echo -e "${CYAN}|   Taint tracking: VM vs tree-walker must agree               |${NC}"
+echo -e "${CYAN}+==============================================================+${NC}"
+echo ""
+
+# Violations are reported in TWO formats: checkTaintedSink() names the variable
+# ("taint_tracking.sink_violation"), while checkExpressionTaintedSink()'s
+# expression path emits "Taint tracking violation: ...". Matching only the first
+# made a working engine look broken and produced a bogus finding — the detector
+# has to cover both.
+count_violations() {  # $1=file $2=extra flags
+    (cd "$TEST_TMP" && timeout 30s "$NAAB" ${2:-} "$1.naab" 2>&1) \
+        | grep -ciE "sink_violation|Taint tracking violation" || true
+}
+
+# scenario|expectation  (tainted = must flag, clean = must not)
+SCENARIOS="
+direct|tainted
+concat|tainted
+interp|tainted
+list_elem|tainted
+dict_val|tainted
+if_expr|tainted
+try_expr|tainted
+match_arm|tainted
+null_coalesce|tainted
+clean_only|clean
+clean_after_source|clean
+sanitized|clean
+"
+
+w() { cat > "$TEST_TMP/$1.naab"; }
+w direct        <<'EOF'
+use env
+use file
+main { let t = env.get("HOME") let x = t file.write("o.txt", x) }
+EOF
+w concat        <<'EOF'
+use env
+use file
+main { let t = env.get("HOME") let x = "p" + t file.write("o.txt", x) }
+EOF
+w interp        <<'EOF'
+use env
+use file
+main { let t = env.get("HOME") let x = "v=${t}" file.write("o.txt", x) }
+EOF
+w list_elem     <<'EOF'
+use env
+use file
+main { let t = env.get("HOME") let l = [t, "c"] file.write("o.txt", l[0]) }
+EOF
+w dict_val      <<'EOF'
+use env
+use file
+main { let t = env.get("HOME") let d = {k: t} file.write("o.txt", d.get("k")) }
+EOF
+w if_expr       <<'EOF'
+use env
+use file
+main { let t = env.get("HOME") let x = if true { t } else { "c" } file.write("o.txt", x) }
+EOF
+# The TryCatchExpr gap: value-producing try, taint must flow through both arms.
+w try_expr      <<'EOF'
+use env
+use file
+main { let t = env.get("HOME") let x = try { t } catch (e) { "" } file.write("o.txt", x) }
+EOF
+w match_arm     <<'EOF'
+use env
+use file
+main { let t = env.get("HOME") let x = match 1 { 1 => t, _ => "c" } file.write("o.txt", x) }
+EOF
+w null_coalesce <<'EOF'
+use env
+use file
+main { let t = env.get("HOME") let x = t ?? "d" file.write("o.txt", x) }
+EOF
+w clean_only    <<'EOF'
+use file
+main { let c = "clean" let x = c file.write("o.txt", x) }
+EOF
+# The stale-flag leak: the declaration immediately AFTER a taint source must not
+# inherit taint from it. Adjacency matters — with more statements in between the
+# leak lands on a variable that never reaches a sink and hides.
+w clean_after_source <<'EOF'
+use env
+use file
+main { let t = env.get("HOME") let c = "clean" let x = c file.write("o.txt", x) }
+EOF
+w sanitized     <<'EOF'
+use env
+use file
+fn sanitize_it(s) { return "safe:" + s.length() }
+main { let t = env.get("HOME") let x = sanitize_it(t) file.write("o.txt", x) }
+EOF
+
+TAINTED_SEEN=0
+for entry in $SCENARIOS; do
+    [ -z "$entry" ] && continue
+    name="${entry%%|*}"; expect="${entry##*|}"
+    vm=$(count_violations "$name" "")
+    tw=$(count_violations "$name" "--tree-walk")
+
+    if [ "$vm" != "$tw" ]; then
+        fail "PARITY-$name" "engines disagree (VM=$vm tree-walk=$tw)" \
+             "taint is implemented twice; a divergence means one path is wrong"
+        continue
+    fi
+    # Agreement alone is not enough — two engines that both miss it agree.
+    if [ "$expect" = "tainted" ]; then
+        if [ "$vm" -gt 0 ]; then
+            TAINTED_SEEN=$((TAINTED_SEEN + 1))
+            pass "PARITY-$name" "both engines flag the taint ($vm)"
+        else
+            fail "PARITY-$name" "both engines MISSED the taint" \
+                 "agreement on a false negative — taint reached a sink unflagged"
+        fi
+    else
+        if [ "$vm" -eq 0 ]; then
+            pass "PARITY-$name" "both engines leave clean data alone"
+        else
+            fail "PARITY-$name" "both engines flagged CLEAN data ($vm)" \
+                 "a false positive makes every taint count untrustworthy"
+        fi
+    fi
+done
+
+# Control: if the harness never produced a single violation, every "clean" case
+# above passed for the wrong reason and the file proves nothing.
+if [ "$TAINTED_SEEN" -ge 5 ]; then
+    pass "PARITY-control" "taint tracking is live in this environment ($TAINTED_SEEN positive cases)"
+else
+    fail "PARITY-control" "taint never fired — the clean cases pass vacuously" \
+         "only $TAINTED_SEEN positive detections; expected >= 5"
+fi
+
+echo ""
+echo -e "${CYAN}+==============================================================+${NC}"
+TOTAL=$((PASS_COUNT + FAIL_COUNT + SKIP_COUNT))
+echo -e "  Total: $TOTAL | ${GREEN}Pass: $PASS_COUNT${NC} | ${RED}Fail: $FAIL_COUNT${NC} | ${YELLOW}Skip: $SKIP_COUNT${NC}"
+if [ "$FAIL_COUNT" -gt 0 ]; then
+    echo -e "${RED}Failures:${NC}$FAILURES"
+    exit 1
+fi
+exit 0