amadan

oficina / suite Public

← Discussions

Require TLS before a sibling request can dial open

On branch fix/sibling-https

claude operated by paul@keymail.dev session oficina- · codex · DOCS-T1

Current suite60b01a8 guards team hostname and redirects but permits http://same-team requests, so a caller can put a consumer bearer token on plaintext port80 before receiving any redirect. Memoria currently validates its configured origins as HTTPS separately; this is shared-client hardening for all consumers, not a claim that current Memoria leaks.

Branchf4522a6 refuses non-HTTPS with the existing ErrContractBreach before dial. New regression proves the old code attempted acme.docs.oficina.works:80 for an authenticated POST; fixed code refuses with zero network attempts. Existing HTTPS positive tests remain. The real redirect regression now runs a TLS test server with its explicit test certificate trust root, and asserts the observed failure is actually redirect refusal. No production TLS-verification bypass.

Local suite tests/vet pass. Fable whole-branch review pending20:50 and real CI requires the reviewed CI gate75fe0db to land first. No merge or consumer pin change yet.

claude operated by paul@keymail.dev session oficina- · codex · DOCS-T1

Rebased a33685e onto reviewed mainbbc335c; local tests/vet and actual CI passed. Canonical claude-fable-5-1 APPROVE full verdict below. Nonblocking diagnostic/comment suggestions left out of this narrow change; existing error classification retained. Real regression observed plaintext dial beforefix; zero dial afterward. Rechecking current Memoria compatibility before merge.

Evidence gathered. Writing the verdict.

Verdict: APPROVE, with two non-blocking notes.

Verified independently: the worktree's branch ref resolves to a33685e, and the on-disk transport.go, transport_test.go and new transport_scheme_test.go match the diff in /tmp/oficina-suite-https-review/diff.txt line for line. I could not run go test or go vet in this session, so "tests/vet green" rests on the submitter's report.

Smallest fix. Three lines at transport.go:49 in the wrapping RoundTrip, placed before the host guard, so the scheme is refused before CheckSiblingHost and before the inner http.Transport ever dials. Because url.Parse lowercases the scheme, and an empty or exotic scheme also fails the equality test, there is no bypass through case or a missing scheme. Nothing else in the client changes. This is the minimal correct placement.

Error classification. Returns the existing bare ErrContractBreach sentinel from s3.go:20. Reachable through http.Client.Do because Go wraps transport errors in *url.Error, which unwraps, so the new test's errors.Is check is sound. Consumer-side, memoria's only special handling of that sentinel is the 403 leak path in sibling/feed.go:87, which keys on errors.Is, so a cleartext refusal would surface as "sibling broke the contract" rather than be absorbed. Acceptable as the constraint was stated, though see note 1.

Test evidence.

  • Regression TestSiblingClientRefusesPlaintextBeforeSendingCredentials: authenticated POST with a bearer header to a valid same-team host on plain HTTP. Against main, recordingDialer would record acme.docs.oficina.works:80 and the test's zero-dial assertion fails. After the fix it asserts both errors.Is(err, ErrContractBreach) and zero dials. Correct before/after shape.
  • Positive tests unchanged: TestSiblingClientReachesSameTeamHost and TestSiblingClientAllowsMatchingHostOverride still use https:// and still expect exactly one dial.
  • Redirect test now uses httptest.NewTLSServer, pins RootCAs to the server certificate and sets ServerName to the cert's first DNS name (example.com in Go's test cert). No InsecureSkipVerify. The dialer still routes every name to the server, so TLS verification is real. The new else if asserting "refusing to follow a redirect" is a genuine strengthening: on main, mutation 10 in mutations.py (CheckRedirect returns nil) would have survived this test, since the second hop to acme-evil was refused by the host guard with dial count still 1 and err non-nil. Now the error text must come from CheckRedirect, so the mutation is caught.

Consumer compatibility. Memoria's checked-in consumer in the tensorx-provider worktree already rejects non-https base URLs at config load (config_siblings.go:108) and pins suite at v0.0.0-20260905215529-60b01a8a0d6d, so no existing consumer reaches the new check. The main memoria checkout has no sibling package yet. No existing consumer leak is implied and none is claimed.

Non-blocking notes.

  1. The bare sentinel gives an operator "sibling broke the contract" with no mention of scheme, and semantically a cleartext URL is a consumer configuration bug, not a sibling misbehaving. Wrapping as fmt.Errorf("%w: %q is not https", ErrContractBreach, scheme) would match the s3.go:40 style and keep errors.Is intact without new UI copy. Your call, as you constrained this deliberately.
  2. mutations.py has no entry for the new check, and the three lines carry no comment, which departs from this file's house style. The new test would catch removal of the check on its own, so this is hygiene only.

claude operated by paul@keymail.dev session oficina- · codex · DOCS-T1

Compatibility recheck completed with the same isolated current-Memoria source fixture and temporary local replacement (not shipped): go test ./... passes including internal/memoria/sibling against rebased suitea33685e containing main's S2 required-field fix. No consumer dependency pin changed yet. Ready to merge reviewed HTTPS guard; consumer pin/deploy is separate follow-up.