Skip to content

fix(serve,judge): exact-origin check, strict Host, PW_SERVE_ALLOWED_HOSTS; country inside spotlight - #43

Merged
wikithoughts merged 2 commits into
mainfrom
overnight/serve-origin-port
Oct 11, 2026
Merged

wikithoughts merged 2 commits into
mainfrom
overnight/serve-origin-port

Conversation

@wikithoughts

Copy link
Copy Markdown
Owner

What changed

Follow-up to #39, from the independent review of #37/#39 (overnight run run-20261011-overnight, task W6-passiveworkers-3739fu).

  • passiveworkers/serve.py (middleware):
    • Exact-origin check. When an Origin header is present, it must equal the desk's own origin: scheme, host and port, taken from request.url.scheme + Host, with default ports 80/443 filled in. Previously only the hostname was compared, so a page on http://localhost:3000 (any other local dev server) could POST /research and /cancel/{id}.
    • Strict Host. Host must be exactly host[:port] (a name, or a bracketed IPv6 address). Userinfo (evil@localhost), paths, spaces, localhost: and localhost:80:80 are refused. An absent Host was already 403 and is now pinned by a test.
    • PW_SERVE_ALLOWED_HOSTS: a comma list of extra hostnames or IPs for LAN or Tailscale use. Entries are lowercased, and junk or wildcard entries (*, 0.0.0.0, bad@x) are ignored. The default is unchanged: loopback names plus PW_SERVE_HOST.
  • README.md: one line documenting the override, with the note that a TLS proxy must pass Host and X-Forwarded-Proto through.
  • passiveworkers/judge.py (compile_report country label only). The country comes from a node's payload, so it is untrusted. It now sits inside the spotlight, as a first line Country: … that is stripped of invisibles, collapsed to one line and capped at 40 characters. The block header is now --- Contribution N ---. The first commit only one-lined the label; the adversarial pass showed that still let about 40 characters of node text sit outside the delimiters, so commit 2 moves it inside.
  • Tests:
    • tests/test_serve.py:
      • a different port, a different scheme, a trailing slash, userinfo or no scheme in the Origin;
      • same-origin passes, including IPv6, an implicit :80 and a Docker -p 9000:8770 remap;
      • a userinfo or junk Host;
      • an absent Host, using a raw ASGI scope with no headers;
      • the override: it admits listed names and refuses unlisted names, cross-origin requests between two allowed names, and the default once unset.
    • tests/test_judge_spotlight.py: the country label is inside the spotlight.
    • One existing expectation changed. Origin: http://localhost:8770 against Host: 127.0.0.1:8770 is now 403, because it is a different origin; no real browser on the desk sends that pair.

Findings addressed (claude-review-pw-37-39.md, PR #39 section)

  • Addressed: 1 (port ignored), 2 (userinfo Host), 3 (no override), 5 (country label outside the spotlight), and 7 (test gaps: a different-port Origin, a userinfo Host, an absent Host).
  • Not changed: 4 (an absent Origin on POST still passes, by design, for non-browser clients) and 6 (indentation flattening in judge blocks).

Verification

  • ruff check . && pytest tests/ -q --cov=passiveworkers --cov-fail-under=75 → exit 0: All checks passed!, 704 passed, 9 skipped, total coverage 80.36%. This was the declared gate, run at head c837677 in a .[dev] venv.

  • python -m py_compile $(git ls-files '*.py') → exit 0. bash scripts/check_app_js.sh && node scripts/fe_test.js → exit 0, ALL FRONT-END FLOW CHECKS PASSED.

  • The new tests fail on the old code. Restoring serve.py and judge.py to origin/main makes 17 tests fail:

    • 8 Origin cases;
    • 6 userinfo/junk Host cases;
    • the override test;
    • the judge country test;
    • the changed localhost-vs-127.0.0.1 expectation.

    The absent-Host and same-origin tests pass on both versions; they pin existing behaviour.

  • Live check over a real socket. I ran python -m passiveworkers.serve on 127.0.0.1:18771, with PW_HOME and PW_REPORTS_DIR set to a scratch directory and PW_SERVE_ALLOWED_HOSTS=desk.tailnet.example, then sent requests with curl:

    request status
    /reports on 127.0.0.1 or localhost 200
    Host: evil.example 403
    Host: evil@localhost:18771 403
    Host: desk.tailnet.example:18771 200
    POST /research with Origin: http://localhost:3000 403
    POST /cancel/nope with the same-origin Origin 404 (reached the route)
    POST /cancel/nope with Origin: http://localhost:3000 403
    Origin: null 403
  • Browser. Headless Chrome via chrome-devtools, at http://127.0.0.1:18771/:

    • The desk rendered: brief box, depth, analysts and sources controls, and the "Past reports" list.
    • Clicking the seeded report rendered "Sample report / Hello from the live check."
    • A same-origin fetch('/cancel/nope', {method:'POST'}) from the page, which sends an Origin header, returned 404 from the route, not 403. The page's own POSTs still pass the new check.
    • The only console errors were 404s for the favicon and my deliberate /nope probes.

    This check ran on commit 1. Commit 2 does not touch serve.py.

  • An adversarial review pass ran on commit 1 (read-only subagent). It found no Host or Origin bypass across these probes:

    • trailing dots, leading-zero ports, IPv4-mapped and long-form IPv6, decimal IPs and percent-encoding;
    • duplicate-looking or comma-joined Host values;
    • Origin variants: null, uppercase scheme, a trailing slash, userinfo, a trailing space.

    Its label finding is fixed in commit 2.

Not verified / follow-ups

  • Follow-up, not in this PR (outside the "country label only" ownership): judge.compile_report still writes the raw node country/model into the final report byline and the ### {country} — {model} headings (judge.py, about lines 418-436). A node could forge a markdown heading in the report. This is not a prompt-injection path, but it needs a one-line sanitize in a separate PR.
  • PW_SERVE_ALLOWED_HOSTS accepts harmless junk names (., -) that match the name regex. No browser sends such a Host.
  • TLS reverse-proxy flows (X-Forwarded-Proto) are documented but not tested.
  • CI was not polled, per the run rules.

🤖 Generated with Claude Code

wikithoughts and others added 2 commits October 11, 2026 07:49
…OSTS; one-line country label

Follow-up to #39's review (findings 1, 2, 3, 5, 7):
- Origin must equal the desk's own origin (scheme, host and port, default
  ports filled in), so another app on localhost:3000 can no longer POST
  /research or /cancel.
- Host is matched as a strict host[:port]; userinfo ("evil@localhost"),
  paths and other junk are refused.
- PW_SERVE_ALLOWED_HOSTS (comma list) adds LAN/Tailscale names; the default
  stays loopback + PW_SERVE_HOST. README documents it in one line.
- judge.compile_report: the node-supplied country label is stripped of
  invisibles, collapsed to one line and capped at 40 chars.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…E proxy note

Adversarial review of the previous commit: a one-line label outside the
delimiters still carried ~40 chars of node text. The block header is now
"Contribution N" and "Country: ..." is the first line inside the data block.
README: a TLS proxy in front of the desk must pass Host and
X-Forwarded-Proto through, or the exact-origin check refuses POSTs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@wikithoughts
wikithoughts merged commit f497835 into main Oct 11, 2026
11 checks passed
@wikithoughts
wikithoughts deleted the overnight/serve-origin-port branch October 11, 2026 07:58
wikithoughts added a commit that referenced this pull request Oct 11, 2026
…eport labels, cap kept joiners per run (#45)

* fix(research,judge,sanitize): drop multi-line result urls, one-line report labels, cap kept joiners per run

- research: a search result whose href holds whitespace or an invisible/control
  character is dropped (_result_url). urlparse() ignores an embedded newline,
  U+2028 or U+200B, so "https://good.example/a\n[S9] ..." passed the host checks
  and forged a source line in _clean() and search_structured().
- judge: node-supplied country/model in the final report (byline and per-
  contribution headings) are one plain line: invisibles stripped, markdown/HTML
  control chars dropped, "://" broken, length capped (_report_label).
- sanitize: a run keeps at most 8 ZWJ/VS16/ZWNJ/RGI-flag forms; extra ones are
  stripped. Stripped invisibles don't break a run.
- D55: correct the covert-channel figure (ZWJ and VS16 are independent per
  emoji gap = 2 bits; ZWNJ 1 bit per Arabic letter gap) and record the cap and
  what it does not bound.

Follow-ups from the independent review of #42/#43.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs(D55): record the unspaced-row cost of the run cap; per-flag and run-reset wording

From the adversarial review of the previous commit: a row of more than 8 joined
emoji with no space splits (pinned by a test), a black flag carries up to 2 bits,
and any visible character outside the run classes starts a new run.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
wikithoughts added a commit that referenced this pull request Oct 11, 2026
…EADME FORWARDED_ALLOW_IPS note (#46)

- serve: an override entry that is not a plain host/IP (unbracketed IPv6 like
  fd7a::1, wildcards, 0.0.0.0, userinfo) was dropped silently, so a typo gave a
  bare 403 "forbidden host". Each such entry now prints one stderr warning
  (the repo's convention), checked once at startup in main() too.
- README: a TLS proxy on another machine also needs FORWARDED_ALLOW_IPS, since
  uvicorn only trusts X-Forwarded-Proto from loopback by default; otherwise
  every POST is 403 "forbidden origin".

Follow-up from the independent review of #43.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant