fix(server): log engine failures by their class outside the Blob routes

[ragnorc]

Summary

This audits every log site in crates/omnigraph-server/src, and the server-facing error paths it reaches, for the leak the Blob routes already close: engine error text written to a server log. That text can hold object URIs, external Blob URIs, credentials or entity IDs. RFC 0033 §11 forbids logging URI credentials or complete sensitive URIs.

Five paths outside the Blob routes leaked. Each now logs the error's class through the Blob routes' RedactedCause, which moves to its own module because it is no longer Blob-specific, or logs a redacted value. Responses are unchanged.

Path	Before	After
Change routes (GET /changes, POST /changes/baseline, GET /commits/{commit_id}/changes), internal failure	The response was collapsed to a constant, but error!(error = %other) logged the full OmniError.	Logs error_kind="change_route_internal" with error_variant, storage_kind and manifest_kind.
Change routes, RecoveryRequired	warn! logged reason, engine text the response deliberately hides.	Logs operation_id only.
Change routes, Manifest::NotFound	debug! logged the internal message the response deliberately replaces.	Logs the public message only.
Export and change-baseline streams that fail after the 200 headers	The raw OmniError text became the body's io::Error. The client never reads it, but tower-http's TraceLayer logs a body error's Display at ERROR (classification=Error: …).	ExportSender::finish takes the OmniError, logs error_kind="served_stream_failed" with stream and the class, and ends the body with a constant. Because the redaction lives in the transport, no producer can pass raw text again.
POST /queries/{name}	info!(graph = %handle.uri) logged the graph's storage root on every invocation.	Logs graph_id.
A graph that fails to open at startup	The cause embedded the raw root (open graph … at {uri}), and graph blocked during startup logged it.	The root goes through redacted_storage_uri, as other server diagnostics already do.
Lance's lance::dataset_events and lance::file_audit	At the default info level the server logged every table load, write, commit, compaction and cleanup with its full storage URI, every file Lance created or deleted, and each delete predicate, which names entity IDs.	server_log_subscriber holds both targets to WARN/ERROR whatever RUST_LOG says, like rmcp.
Cluster embedding config, resolve() of an inline api_key	The refusal echoed the value (got '…'), and settings.rs logs resolve() errors at WARN.	The refusal states the rule, not the value. Validation already refuses inline keys earlier, so this is defense in depth.

Reviewed, no change needed

• HTTP trace layer. Its DEBUG span records method, URI and version, never headers. No query string carries a credential or URI: only Blob selectors, branch names, change cursors and page tokens. Bearer and data tokens travel in Authorization.
• Request logs. log_policy_decision, graph routed and commit lookup branch disappeared log graph vocabulary only: actor ID, branch names, commit IDs.
• Lifecycle logs. Startup, shutdown and watchdog logs are constants, signal-handler errors or redacted_storage_uri roots.
• settings.rs. Cluster diagnostics carry config paths and messages; cluster validation redacts roots and refuses inline secrets. Stored-query parse errors and log_registry_warnings are query-text diagnostics from operator config.
• deployment.rs uncertain(). It logs and returns the same reconciliation message to the authorized cluster administrator. Its parts are cluster and config errors, and the storage crate has already redacted any storage error among them.
• Other server logs. workload.rs logs numeric limits from env; oidc_identity.rs logs a constant.
• Other Lance targets. lance::io_events carries index part IDs and lance::execution carries plan counters. lance::object_store::throttle logs object-store error text only at DEBUG.
• MCP. Failures return the same ApiError body to the same requester, and rmcp payload logs stay filtered.
• CLI. It is the requester: it prints server error bodies to its own user, installs no tracing subscriber, and client.rs tests already assert a bearer never appears in its errors.

Findings left for a decision (not changed here)

• 500 bodies on other routes. On routes other than Blob and changes (/query, /mutate, /load, /export before headers, schema, branches, commits), 500 bodies return OmniError::Storage text, BlobIntegrity reasons and Io text to an authenticated caller. The storage crate strips credentials when it builds those messages, but they still name physical object paths. The Blob and change routes collapse 500s to constants. Making that uniform changes the wire contract of every route and needs its own decision.
• StoredExternalBlobDenied on /mutate. This 400 includes the stored external URI. /mutate checks only change, so an actor granted change without read on a branch can learn a stored URI. RFC 0033 §7.3 grants URI disclosure to Blob readers, and §11 says this error names "type, id, and property".
• Credential-bearing S3 roots. normalize_root_uri refuses neither userinfo nor a query on an s3:// graph root; Azure roots refuse userinfo. Redacting diagnostics mitigates this. Refusing such roots at config would remove the source.
• Startup refusals. InsertError::DuplicateUri and InvalidUri show a raw root in a startup refusal or deployment response to the operator. Exposure is low, and the root cause is the one above.

Tests

The log assertions live in-source, because test_log_capture is crate-internal. Each new assertion was checked to fail with the old behavior restored.

• handlers::change_route_error_tests (existing owner): every test now captures the log at DEBUG. It asserts that the class fields are present and that the text the response withholds is absent from the log.
• export_transport::tests::completed_producer_transfers_root_cut_to_terminal_frame (existing owner): an engine failure carrying a bucket, a key and a token reaches the body as the constant and the log as its class. As before, a failure after the client has gone is not logged.
• log_filter_tests::restricted_dependency_logs_remain_filtered_under_specific_directives (renamed from verbose_sdk_payloads_…): covers the Lance targets under trace and target-specific directives. Sibling targets such as lance::execution stay visible.
• log_filter_tests::stored_query_invocation_logs_the_graph_id_not_its_storage_root: drives the real route through build_app. The whole log is checked, so real Lance events also prove the root never reaches it, which guards against a Lance target rename.
• external_blob_startup_tests::blocked_startup_logs_the_root_without_credentials: a file:// root with a query and fragment.
• omnigraph-cluster rejects_inline_api_key: the refusal does not echo the value.

[ragnorc]

Recommendation: approve. I found no blocking defect in aa6229f2989499a3ee3b27e84eea1604b01b3833.

This PR keeps private engine text and storage identifiers out of several server log paths. It logs a typed error class, uses the public graph ID for stored-query calls, and gives a failed response stream a fixed error message. It also blocks verbose Lance dataset and file events even when RUST_LOG requests them. Clients still get the existing change-route status and error contract. Operators retain the failure class, stream name, and recovery operation ID.

The fix addresses the cause at shared boundaries. RedactedCause retains no engine message and unwraps completion evidence to classify its cause. ExportSender::finish accepts the typed failure, so export and change-baseline producers share the same rule. This matters because tower-http classifies a body error through its Display text. Redacting only the producer's own log would leave that second path open.

The subscriber filter is independent of the environment filter. It matches complete target components and preserves WARN/ERROR and unrelated execution events. I checked the pinned Lance 11.0.0 implementation: dataset loading emits the full URI at INFO under lance::dataset_events; file creation/deletion uses lance::file_audit. This is a target-level rule for those event families, rather than a replacement rule for selected URI strings.

Tradeoff and liability: logs lose free-form engine detail and verbose Lance audit events. That reduces diagnostic detail, but removes multiple disclosure paths. Moving the existing class helper to one owner and moving stream redaction into the shared sender reduce maintenance liability. There is no new durable state, storage protocol, or public API. The static stream label adds a small diagnostic obligation. New error variants must choose a class, and changes to upstream log targets need another audit. Five more stream producers can reuse the same terminal boundary without five local sanitizers. I found no avoidable new abstraction in this change.

Validation:

• Local exact-head checks passed: four change_route_error_tests, two log_filter_tests, the existing terminal-frame/cut-lifetime test, and the startup credential-redaction test.
• Before/after proof: I temporarily restored only change_route_error from pre-fix commit 4c7a160acd1ae689773613c30069d4fed27f19e0, retaining the reviewed tests. cargo test -p omnigraph-server --locked --lib change_route_error_tests -- --nocapture ran four tests and failed all four. The captured output exposed the old private paths/recovery reason and lacked the new class fields. Restoring exact head and running the same command passed all four. This is a controlled old-function comparison, not a test run of the entire pre-fix tree. Temporary edits are restored.
• Local commands used Rust 1.97.1, RUSTFLAGS='--cfg tokio_unstable --cfg tokio_unstable', and a reused target directory. The three other focused filters ran through the resulting exact-head test binary. Docs and AGENTS link checks, plus git diff --check, passed.
• Exact-head server CI passed: 198 library tests, zero failures, two ignored, plus the integration suites. I checked the job log for the new assertions. Ordinary GQT and workspace CI were still pending at this review; DST and GQT-DST had passed.

GQT cannot capture server tracing fields or inject this native stream failure. These minimal assertions belong in the existing Rust owners. The local before/after proof covers the change-route mapper; the other paths have exact-head local/CI evidence, not local old-tree proof. I did not measure performance or run a cloud deployment. This review does not establish universal redaction of all server error responses or all dependency logs. Embedders that install their own subscriber must apply the documented target restrictions.