Repository navigation
Conversation
egressAuthorityConfiguration answered nine faults (unknown key, missing or empty string, port type or range, relative file path, non-loopback development host) with one message about a loopback development listener, shared by drivers.credential_proxy.configuration.authority and repository.openShell.authority. As a plain Error it reached API and worker logs only as STARTUP_FAILED (finding 2216). The validator now takes the block's path and throws InstallationFieldError with one detail per refused field: its JSON Pointer, a schema error code and a message naming the field, never its value. startupConfigurationFailure maps it to DRIVER_CONFIGURATION_INVALID with those details, so the API and worker logs and the chart's upgrade check name the field. The shared-field module still imports only its reviewed dependencies.
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
Report an unknown key once, at the block's pointer, so a long, control or credential-like key cannot make the log filter drop the only detail. Bound the details at 32 like ConfigurationValidationError. Give an empty string INVALID_VALUE. The tests now assert what the log filter keeps, cover an absent and a non-object block, and check that no configured value appears.
clawsweeper
Bot
added
clawsweeper:intake:bug
|
Codex review: needs maintainer review before merge. What this changesThis PR reports bounded, value-free field diagnostics when an Installation authority block is invalid. Example: An operator sets drivers.credential_proxy.configuration.authority.privateKeyPath to tls.key.
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Complete Merge readiness✅ Ready for maintainer review Keep this PR open: it fixes actionable startup diagnostics still missing on current main, and review found no introduced correctness defect. Priority: P2 Before mergeNone. FindingsNone. Agent review detailsHow this fits togetherInstallation composition validates operator-supplied authority settings before startup; the API, worker, and Helm pre-upgrade check turn refusals into filtered diagnostics. flowchart TB
A[Installation configuration] --> B[Shared authority validator]
B --> C[Installation field error]
C --> D[Startup failure mapping]
D --> E[Diagnostic filter]
E --> F[API and worker logs]
E --> G[Helm upgrade check]
Technical reviewBest possible solution: Retain the shared validator and existing startup diagnostic pipeline while reporting each refused authority field without exposing its value. Do we have a high-confidence way to reproduce the issue? Current-main source directly shows that a relative privateKeyPath throws the generic authority Error and bypasses typed configuration diagnostics; no target code was run during this review. Is this the best way to solve the issue? The patch fixes the diagnostic owner shared by both authority blocks, preserves validation, and uses the existing filtered startup-error path. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against a651091ac5c4. Provenance checked
TestingProof path: in-process harness. SecurityNone. EvidenceWhat I checked:
LabelsLabel changes:
Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment Reviewed October 22, 2026, 10:34 PM ET / October 23, 2026, 02:34 UTC. |
Problem
egressAuthorityConfigurationvalidates bothdrivers.credential_proxy.configuration.authorityandrepository.openShell.authority. It gave the same message for nine different faults:The message read "Protected egress requires explicit TLS, issuer identity and a loopback development listener", even in production. Because it was a plain
Error, the API and worker logs showed onlySTARTUP_FAILED/WORKER_STARTUP_FAILED, so an operator could not tell which key was wrong (finding 2216).Change
REQUIRED,UNKNOWN_FIELD,INVALID_TYPE,INVALID_RANGE,INVALID_FORMATorINVALID_VALUE) and a message that names the field but never its value.InstallationFieldError.startupConfigurationFailuremaps it toDRIVER_CONFIGURATION_INVALIDwith those details, the same shape Driver configuration errors already use. The API and worker startup logs and the chart's upgrade check (formatStartupConfigurationFailure) therefore name the field.ChartListenerMismatchError, which now escapes~and/too.installation-fields.tsstill imports only its reviewed dependencies. The repository-credentials source-boundary check passes.Verification
New
configuration-startuptests:loadInstallationConfiguration: an absent block, a non-object block, an unknown key, and three relative paths in one refusal. Each case checks the pointer and code as they appear after the log filter, and checks that no configured value appears.DRIVER_CONFIGURATION_INVALIDwith the exact detail.repository.openShell.authoritynames its own block.On main, both tests fail.
Locally (Node 24), these all pass, 327 of 328 with one existing skip:
configuration-startup,sandbox-driver-startup,egress-connected-preflight,installation-change-preflightand the source-boundary conformance test.The touched tests passed 20/20 at 2 CPUs on the final head.
Internal review: approve with nits. All applied:
INVALID_VALUE;The reviewer ran the old and new predicates on 394 inputs in both modes. Every input got the same accept or refuse result.
tsc --build, eslint and Prettier are clean.Risks
STARTUP_FAILED/WORKER_STARTUP_FAILEDtoDRIVER_CONFIGURATION_INVALID. Validation itself is unchanged: the same inputs are refused.