fix(access): validate redirect and credential boundaries reproducibly #133

Merged

ClaudiuSchuster merged 7 commits from oss-oo/platform-access-repro-20261014 into main 2026-10-15 10:23:49 +00:00

Conversation 0 Commits 7 Files changed 13 +2502 -87

ClaudiuSchuster commented 2026-10-14 10:34:20 +00:00

Owner

• Copy link
• Copy Markdown

Access validation must report the transport and identity that were actually tested. The clients now handle malformed HTTP redirects deliberately, apply credential checks independently of header casing, and retain redirect evidence in MCP failures. Authenticated Git probes and all HTTPS Git probes require canonical URLs and refuse redirects, preventing credential forwarding and unencrypted fallback.

Git probes use an owned neutral working directory and HOME, so caller repository settings, helpers, hooks, askpass programs and netrc entries cannot supply unintended credentials. Push preparation preserves all explicitly addressable local refs, including tracking refs and notes, while updating only the requested destination ref. The source repository remains unchanged. Existing cancellation paths retain honest interrupted/not-run reporting.

Validation at 0b94ab52ae87e21e239c7bbb50a83feb9e8481c4, tree 21fda3f4c717df071581349c759097645adbc8dc, integrated with current Main:

• Independent review reproduced the original credential/redirect issues, checked their corrections and verified explicit-ref transfers against a real loopback Git HTTP receiver, including received objects, source preservation and destination isolation.
• All 402 offline cases across 17 files pass locally. The PVE suite passes 401 with one expected root-permission skip; that exact permission case additionally passes unprivileged.
• The complete PVE end-to-end gate passes on the same combined source tree, including the real public/authenticated MCP edge, project lifecycle, events, registration and console flows. Test resources are removed; existing runner and dependency images remain unchanged.
• Report entrypoints pass: Stage1 has 24 checks passed. Stage2 and Stage3 were deliberately run without targets and report 28 and 19 not-run checks. This does not claim real production-route acceptance for those stages.

This change improves the validation tooling and its contracts. It does not deploy application changes or run mutating production canaries.

Access validation must report the transport and identity that were actually tested. The clients now handle malformed HTTP redirects deliberately, apply credential checks independently of header casing, and retain redirect evidence in MCP failures. Authenticated Git probes and all HTTPS Git probes require canonical URLs and refuse redirects, preventing credential forwarding and unencrypted fallback. Git probes use an owned neutral working directory and HOME, so caller repository settings, helpers, hooks, askpass programs and netrc entries cannot supply unintended credentials. Push preparation preserves all explicitly addressable local refs, including tracking refs and notes, while updating only the requested destination ref. The source repository remains unchanged. Existing cancellation paths retain honest interrupted/not-run reporting. Validation at `0b94ab52ae87e21e239c7bbb50a83feb9e8481c4`, tree `21fda3f4c717df071581349c759097645adbc8dc`, integrated with current Main: - Independent review reproduced the original credential/redirect issues, checked their corrections and verified explicit-ref transfers against a real loopback Git HTTP receiver, including received objects, source preservation and destination isolation. - All 402 offline cases across 17 files pass locally. The PVE suite passes 401 with one expected root-permission skip; that exact permission case additionally passes unprivileged. - The complete PVE end-to-end gate passes on the same combined source tree, including the real public/authenticated MCP edge, project lifecycle, events, registration and console flows. Test resources are removed; existing runner and dependency images remain unchanged. - Report entrypoints pass: Stage1 has 24 checks passed. Stage2 and Stage3 were deliberately run without targets and report 28 and 19 not-run checks. This does not claim real production-route acceptance for those stages. This change improves the validation tooling and its contracts. It does not deploy application changes or run mutating production canaries.

ClaudiuSchuster added 6 commits 2026-10-14 10:34:20 +00:00

access-validation: refuse cross-origin credential redirects ... 3da0c13d93

Replace urllib's default redirect following with an explicit policy in
av_common/clients/http.py (shared engine, open_with_policy): same-origin
redirects are followed with the Authorization header preserved under a
small hop bound (DEFAULT_MAX_REDIRECTS=3, overridable per call); a
redirect that would carry credentials to a different origin (scheme,
host, or port) is refused with a deliberate 'unsafe-redirect' error
carrying the 3xx answer; https-to-http downgrades are refused for any
request; Locations with userinfo or non-http(s) schemes and 3xx without
a Location are refused. The followed chain lands in ClientResult.meta as
scrubbed scheme://host[:port]/path entries. mcp.py applies the identical
engine to authenticated JSON-RPC POSTs. The opener is built per
conversation because urllib's HTTPS handler snapshots its SSL context
at construction time.

git_https pins http.followRedirects=initial in the constructed argv
(git's own documented default, pinned explicitly) and gains the _execute
injection seam mirroring git_ssh for hermetic recorder tests.

Tests (stage1_contract/tests/test_redirects.py, loopback fakes in the
87xx/89xx bands driving the real clients): same-origin follow with chain
in meta, cross-origin-with-credentials refusal proven by a recording
sink on the far origin, https-to-http downgrade refusal against the
fixture TLS cert, loop and hop-limit refusals with an explicit override,
userinfo Location refusal without leaking the userinfo, the
unauthenticated loopback cross-origin carve-out, the authenticated MCP
POST refusal, and the git argv pin for ls-remote and push. test_tls.py
additionally pins that the TLS probe connects to exactly the named
host:port (no HTTP layer, hence no redirect surface).

access-validation: honest operator cancellation for the runners ... 64f40aba16

New av_common/cancellation.py: install() converts SIGTERM into
OperatorCancelled (SIGINT already raises KeyboardInterrupt);
CANCELLATION_EXCEPTIONS is what a runner's top-level except catches.
OperatorCancelled derives from BaseException on purpose — every probe
boundary in the lane catches Exception, and a swallowed cancellation
would be mislabeled as a failed check, exactly the fake verdict this
seam forbids.

All three stage runners install the hook first thing in main() and wrap
their check phase: on interruption the report is finalized immediately
via cancellation.finalize_report — the in-flight row (stage-2/3 shape:
added before its probe ran) and every inventory row that never ran
record notrun with the reason cancelled-by-operator, completed rows
keep their real verdicts, pre-existing honest notrun reasons are never
relabeled — then the report is written and the process exits 130
(128+SIGINT convention, carried for both signals). Stage 1 gains the
INVENTORY constant the finalize path needs (pinned against a complete
run by Stage1InventoryTests) and a cancellation-safe fake boot;
stage 3 cancelled before any check ran writes no report rather than
inventing one. Temp hygiene survives: the clients' askpass/known_hosts
cleanups live in finally blocks, which BaseException unwinding still
runs — proven by the legs.

Tests (test_redirects.py::OperatorCancellationTests + Stage1InventoryTests):
finalize semantics incl. never-relabel, the real SIGTERM signal path, a
real mid-run SIGTERM through the stage-1 plumbing with askpass hygiene,
an end-to-end runner subprocess exiting 130 with a finalized report and
zero faked verdicts, git_ssh known_hosts cleanup under cancellation,
and the inventory drift guard.

access-validation: route contract inventory at 4cc434a ... 73403b72a9

New docs/ACCESS-VALIDATION-ROUTES.md: the source-defined access
contracts at base 4cc434a with an exact file:line anchor per asserted
contract — public/authenticated HTTPS Git (incl. the live canonical
Caddy redirects and the git http.followRedirects=initial pin with its
verbatim git-config(1) citation), pinned-host SSH Git on deployed port
2222 with [host]:port known-hosts normalization, public REST reads,
the authenticated MCP surface (stateless POST JSON-RPC, 405/400/415,
429 budget shapes), MQTT-WS, presence-only LFS/registry probes, and
the PLANNED public MCP endpoint explicitly marked not deployed / not
accepted (candidate lives on the parallel branch; recorded as a Root
integration touch point only). Also records what each existing
validator proves (e2e-local.sh, watchdog + mqtt_probe, verify-live,
agent-gateway tests, lane stages 1-3), the redirect-policy semantics
incl. the precise loopback carve-out and hop-limit rationale, the
operator-cancellation semantics (exit 130), a timestamped
historical-evidence table (Root live acceptance 2026-10-14T05:47:19Z
of PR131 == the base commit), and an honest gaps section (SSH
handshake VM1005-only, browser unsupported, LFS/registry content and
client receipts pending).

IMPLEMENTATION doc and lane README gain the dated round section with
the verified suite counts (before 15 files/353 tests measured on
4cc434a — the previously recorded 346 had drifted; after 16/370) and
one pointer each to the ROUTES doc; history sections untouched.

access-validation: refuse redirect Locations with invalid ports ... 5c53b1c12a

A Location with a syntactically invalid port (https://host:bad/) or an
out-of-range port (http://h:99999/) was ACCEPTED by _redirect_target
(it checked scheme/hostname/userinfo but never the port) and then
crashed with an uncaught ValueError at _origin_of's parts.port inside
the cross-origin/downgrade checks — escaping rest_request/mcp_raw_post,
whose contract is that status/error/data are reported, never raised
(independent review rev-01, finding REV-01, runtime-proven on loopback).
The port is now validated once inside _redirect_target (accessing
urlsplit(...).port raises there), refusing with the reason "redirect
Location port is invalid" — the refusal text never quotes the Location.
Operator-supplied base URLs keep exactly their base-4cc434a behavior
(A/B-probed: nonnumeric port still raises http.client.InvalidURL from
urllib's own parsing, out-of-range port still maps to a status-0
ClientResult); the new check only guards redirect Location values.
No other unvalidated .port access remains on the redirect path: after
this change _origin_of/_chain_entry only ever see operator URLs or
targets that already passed _redirect_target.

_chain_entry now re-brackets IPv6 literal hosts (urlsplit's .hostname
strips the brackets), so scrubbed meta["redirects"] entries like
https://[::1]:8443/a stay re-parseable (review finding REV-06).

Tests (test_redirects.py): authenticated 302 with Location
http://127.0.0.1:notaport/x and with :99999 both refuse unsafe-redirect
with the sink empty and only the origin request sent; the refusal text
provably does not contain the Location value; a unit leg pins the
re-bracketed IPv6 chain entry form. Red/green evidence for the port
legs lives in the round evidence dir (10-red-green/red-rev01-port.txt
against the unfixed engine, green after this fix).

access-validation: pin redirect method and auth-preservation semantics ... 06136f88cf

The redirect policy's ALLOW path and method rewrite were code-true but
unpinned (independent review rev-01, findings REV-02/REV-03): a future
regression that drops the Authorization header on a same-origin follow,
or replays a POST+body across a 301/302/303, would have kept the whole
suite green. New legs in test_redirects.py, all asserted from the
scripted server's journal (what actually arrived — method, body bytes,
Content-Type header), never from client-side echoes:

- same-origin 301 while token-authenticated: the followed hop's
  journal shows Authorization: token <synthetic>, the final 200
  arrives, and the scrubbed chain lands in meta["redirects"];
- POST with a JSON body through same-origin 303 and 302: the followed
  request is GET with an empty body and no Content-Type;
- POST with a JSON body through same-origin 307 and 308: method, body
  bytes, and Content-Type are preserved exactly;
- a 3xx without a Location header refuses (reason names it);
- a Location with a non-http(s) scheme (ftp://) refuses.

The scripted server's journal now records body and content_type per
request (additive keys; existing legs unaffected).

access-validation: record the review-fix round in docs and docstrings ... c0b6ad7df3

Doc-only amendments closing the review's info/minor findings that need
no engine change:

- cancellation.py install()/restore() docstrings corrected per a
  python3 -I probe on this station (3.12.3): signal.signal(SIGTERM,
  SIG_IGN) over a previously-default disposition RETURNS
  Handlers.SIG_DFL (an enum, restorable — the review's premise that a
  previous-default handler can never be restored is false here); only
  a None return marks a C-installed disposition Python cannot read
  back, which restore() leaves as is. No behavior change (rev-01
  finding REV-04).
- cancellation.py module docstring + the IMPLEMENTATION round section
  record the canary residue window: an operator cancellation landing
  inside a LIVE gated canary's push-to-delete window can leave the
  disposable canary ref behind (OperatorCancelled is deliberately
  uncatchable at probe boundaries and therefore skips execute_canary's
  cleanup loop); check via the branch API 404 probe documented in the
  canary plan's cleanup steps. Pre-round SIGTERM was strictly worse
  (REV-05), and the uncaught-traceback startup/final-write windows
  with no verdict at stake (REV-07, deliberately not engineered
  around).
- ROUTES policy list and the lane README now state the new refusal
  shape (syntactically invalid / out-of-range redirect Location ports)
  and the journal-pinned ALLOW-path and method-rewrite semantics;
  the IMPLEMENTATION doc gains the dated impl-02 round section with
  measured verification: full offline suite 16 files / 380 tests all
  OK (stage1 109, stage2 105, stage3 166 — up from 370 by the ten new
  redirect legs); stage runners 24/24 passed, 28 notrun, 19 notrun,
  all exit 0.

ClaudiuSchuster added 1 commit 2026-10-15 10:15:20 +00:00

fix(access): isolate credentials and preserve explicit probe refs ... 0b94ab52ae

Integrate the accepted public MCP base. Refuse credential-bearing and HTTPS redirects, handle malformed locations and header casing, isolate caller Git and netrc state, and preserve all explicitly addressed source refs without widening destination updates. Independent review, 402 cases including the nonroot permission supplement, and the complete combined PVE end-to-end gate pass.

ClaudiuSchuster changed title from WIP: access-validation redirect boundaries, operator cancellation, route inventory to fix(access): validate redirect and credential boundaries reproducibly 2026-10-15 10:16:06 +00:00

ClaudiuSchuster merged commit a02eb9d828 into main 2026-10-15 10:23:49 +00:00

ClaudiuSchuster referenced this pull request from a commit 2026-10-15 10:23:49 +00:00

fix(access): validate redirect and credential boundaries reproducibly (#133)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

OSS-OO/laboratory!133

No description provided.