security: Phase-2 hardening — close #73/#80/#71, advance #76 - #83
Merged
Conversation
…k TOCTOU, info disclosure, index-name boundary; perf: ONNX windowing Fixes the self-contained items from the post-audit backlog, each with a regression test where testable: - #73 (High) F-PATH-02 residual: a snapshot repository `settings.location` containing `..` that resolves to a nonexistent target escaped `data_dir` (canonicalize lexical fallback + component-based starts_with). Reject `..` path components outright. (+regression test: direct + allowlist-laundering.) - #80: wire IndexName::validate (now pub) at the es_compat create_index boundary — defense-in-depth, same accepted set. - #71: ONNX embed_semantic_jobs no longer pre-materializes/clones every window's passage texts; windows carry (start,end,passages) and texts are built lazily and moved into embed_batch (dual: <=2 in-flight). - #76 S5-5: enforce max_fields_per_index on the explicit add_fields path (PUT _mapping / schema evolve), not only dynamic ingest. (+regression test.) - #76 S5-3: serialize magic-link redeem check->consume behind a redeem gate so two concurrent redeems of one token cannot both mint a session. - #76 AUTHZ-2: drop node_id and exact build version from the unauthenticated cluster/info body; keep only mode + uptime. (+regression test.) Deferred (tracked): #76 S5-4 (ConnectInfo through shared serve/TLS path), #75 (cluster frame auth wire-format + fail-closed config), #79 (per-brain access model).
…egression guard) The #76 AUTHZ-2 fix removed node_id and the exact build version from the unauthenticated cluster/info response. Update the phase-1 integration tests to assert those fields are now absent (and keep asserting mode + uptime), so they double as a regression guard instead of asserting the old leaky shape.
This was referenced Aug 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Security & perf hardening (Phase 2, wave 1)
Fixes the tractable, self-contained items from the six open post-audit issues, each with a regression test. The three remaining items need invasive plumbing or a design decision and are called out below rather than rushed (correctness over breadth for security code).
Fixed
#73(High) — snapshotsettings.locationresidual..escape (F-PATH-02). A location whose fully-resolved target did not yet exist madecanonicalize()fail, so the lexical fallback kept the raw..components and a component-basedPath::starts_withtreated<data_dir>/../../escapeas insidedata_dir;create_dir_allthen wrote outside it. Now any..path component in a repo location is rejected up front. (engine.rs; +regression test covering the direct escape and the allowlist-laundering variant.)#80—create_indexpath-traversal defense-in-depth. The es_compatcreate_indexhandler now callsIndexName::validateat the boundary (madepub) instead of relying solely on the deeperEngine::create_indexguard. Same accepted set; the rejection is now local and obvious.#71— ONNX windowing memory regression.embed_semantic_jobsno longer pre-materializes (and clones) every window's passage texts up front. Windows carry(start, end, passages)only; texts are built lazily and moved intoembed_batch— sequential path moves per window, dual path materializes at most the two in-flight windows (collect_ordinal_buffered_twolaunches lazily).#76S5-5 — field-limit bypass on the explicit mapping path.Index::add_fields(PUT /:index/_mapping, schema evolve) now enforcesmax_fields_per_index, matching the dynamic-ingest guard. (+regression test.)#76S5-3 — magic-link redeem TOCTOU. The single-use check and themark_magic_link_usedcommit are now serialized by a redeem gate held across the whole check→consume, so two concurrent redeems of one token can no longer both mint a session.#76AUTHZ-2 — pre-auth info disclosure. The unauthenticatedcluster/infoendpoint no longer returnsnode_idor the exact buildversion(fingerprinting vectors); it keeps onlymode(needed for the SPA's boot decision) and benign uptime. (+regression test.)Deferred (tracked on their issues — each needs a dedicated, tested change)
#76S5-4 (x-forwarded-for trust) — a correct fix needs the real peer IP viaConnectInfo<SocketAddr>, which must be plumbed through the sharedserve()/TLS path (the console is served with plaininto_make_service()), plus a trusted-proxy CIDR config. Invasive; deserves its own change. security: console-API hardening — x-forwarded-for trust (S5-4), magic-link TOCTOU (S5-3), field-limit bypass (S5-5), cluster/info disclosure (AUTHZ-2) #76 stays open on this one item.#75(unauthenticated Raft frames) — needs a cluster shared-secret/HMAC (or mTLS) with a fail-closed config design and a consensus wire-format change. Operator-gated (cluster off by default).#79(per-brain authorization) — an access-model/RBAC design item, as the issue itself notes.All touched crates pass
fmt/clippy(-D warnings) and the new + existing tests.