Repository navigation
fix(gateway): keep service-derived status targets off the explicit-URL credential path - #135862
Conversation
|
🦞👀 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. |
|
Codex review: needs real behavior proof before merge. Reviewed September 7, 2026, 1:03 AM ET / 05:03 UTC. ClawSweeper reviewWhat this changesThe PR keeps Gateway service-status probes tied to their selected target and credentials, preserves endpoint-scoped proxy headers and certificate pins, and prevents advisory plugin reads from creating database sidecars. Merge readiness⛔ Blocked before merge - 3 items remain This remains a useful fix for a bug present on current main and the latest release. The previous saved-pin finding is resolved, and no remaining actionable code defect was found; transport proof for that correction is still outstanding. Priority: P1 Review scores
Verification
How this fits togetherGateway status combines native-service details, configuration, and explicit CLI options to select a connection target. Its probes then report connectivity or read-capability status to operators and readiness scripts. flowchart TD
A[Service configuration] --> C[Status target selection]
B[Explicit CLI options] --> C
C --> D[Selected credentials and transport policy]
D --> E[Gateway connection or RPC probe]
E --> F[Status and readiness result]
Before merge
Agent review detailsSecurityNone. PR surfaceSource +64, Tests +476. Total +540 across 10 files. View PR surface stats
Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep the service-owned target and credential flow, with transport evidence confirming that existing pinned endpoints still work and a wrong certificate receives no headers or Gateway credentials. Do we have a high-confidence way to reproduce the issue? Yes: the linked native-service report and current-main caller/bootstrap chain establish why auth-none status fails without an explicit URL. This review did not execute that flow. Is this the best way to solve the issue? Yes: preserving target provenance and freezing credentials at the status owner repairs the mismatch without weakening the generic explicit-URL guard; the previous endpoint-policy omissions are now corrected. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 85c00af82973. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (37 earlier review cycles; latest 8 shown)
|
9de7de9 to
99e0b5f
Compare
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
99e0b5f to
c310b1c
Compare
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
ab799b8 to
143cd4b
Compare
143cd4b to
da1d63a
Compare
da1d63a to
ce0c58b
Compare
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
…L credential path gateway status --require-rpc forwards the service-derived probe URL as url to callGateway, which bootstrap classifies as a CLI --url override and rejects with GatewayExplicitAuthRequiredError for auth-none local Gateway. Only forward url to callGateway when the caller supplies an explicit CLI --url override (probeUrlOverride). Service-derived targets omit url so bootstrap resolves via localPortOverride and the guard never fires.
|
@clawsweeper re-review. Exact-head CI is now fully green (run 33774337856, conclusion: success) — the previously reported compact-small-6/50 and config-48 failures were unrelated flaky timeouts (agent-exec service-relay, update-cli self-update, qa-lab Slack live) outside this PR's change surface, and passed on rerun. No code changes since head 7ab99ea. |
| const originalProbe = callGatewayStatusProbe.getMockImplementation(); | ||
| assert(originalProbe); | ||
| const server = createServer({ key: TEST_TLS_KEY_PEM, cert: TEST_TLS_CERT_PEM }); | ||
| const wss = new WebSocketServer({ noServer: true }); |
|
Merged via squash.
|
Closes #135471
What Problem This Solves
Resolves a problem where operators running
gateway status --require-rpcagainst an auth-none local Gateway receive an explicit-credential error even though they did not supply--url.Service diagnostics could also select credentials or connection details from a configured remote Gateway instead of the service being inspected. An empty target-scoped credential result could fall back to unrelated config or environment credentials later in the RPC path.
Why This Change Was Made
Service-derived targets now remain distinct from explicit URL overrides throughout the status probe. The selected service target owns its local credentials, merged service environment, and certificate, regardless of the configured client remote mode.
serviceTargetUrlcarries the service URL without classifying it as CLI--url.edgeAuth, andtlsFingerprint. Existing consumers retain their endpoint-scoped transport policy: proxy headers use the existing origin/path selector, and ordinary connectivity probes retain the saved pin only on an exact configured endpoint. Unrelated token/password fallback and service certificate inheritance remain excluded.skipImplicitAuthpreserves the already-resolved credential set, including an empty set for auth-none.--urluses only explicit RPC token/password and does not inherit the service certificate.Generic credential precedence, persisted configuration, schemas, and protocols are unchanged. Explicit URL credential requirements and read-only device-auth behavior remain intact.
User Impact
Operators can inspect an auth-none service without inventing a token or password. Service status remains tied to the selected service even when remote client settings or an environment URL are present. Explicit remote probes retain their credential isolation, while configured proxy headers and saved certificate restrictions remain available under their existing endpoint-scoping rules.
Evidence
serviceTargetUrlfield to the exported connection-details wrapper type.max-linescheck; no size-limit exception was added.a94fb88434d8fe96e8076e1ced93cfa118c6e921exposed three related failures:oxc/no-map-spreadin test-case construction,TS2698from spreading an unknown-typed expected fixture field, and the existing seeded-state diagnostics-export test observing new empty SQLite WAL/SHM files. Evidence: https://github.com/openclaw/openclaw/actions/runs/34083612351bind: "loopback"expectation without a cast, and opts the status install-record caller intoartifactPreservingReadOnly. Storage owners, defaults, schemas, and the existing seeded/absent process-test assertions are unchanged.oxc/no-map-spreadcheck reproduced the failure before the test-construction repair and passed after it. Focused lint/max-lines, formatting, and whitespace checks passed for the follow-up.a94fb88434d8fe96e8076e1ced93cfa118c6e921reported no actionable defects in the caller option or the two test-construction repairs.34083612351, then its process shard passed on exact head8dd21f205d357ace6e2178f38a3df699c642b666, without changing the seeded/absent assertions. Green job: https://github.com/openclaw/openclaw/actions/runs/34084196955/job/1016251415998dd21f205d357ace6e2178f38a3df699c642b666finished successfully on September 7, 2026 at 04:52:43 UTC, validating all three follow-up repairs and the existing tests with the complete dependency tree. This predates the saved-pin correction and does not establish its TLS proof. Run: https://github.com/openclaw/openclaw/actions/runs/340841969558dd21f205d357ace6e2178f38a3df699c642b666reported no actionable defects in the saved-pin correction or its TLS test boundary. Focused lint/max-lines, formatting, and whitespace checks passed.5819c53f08df82fc495cd63c69c97b1b4ac1fbf8finished successfully on September 7, 2026 at 05:06:48 UTC. All 155 jobs completed green, including the agentic CLI selection containing the gather suite and its real TLS cases. The matching saved pin admits the required proxy header and explicit synthetic token; the wrong pin fails with zero decrypted HTTP bytes, upgrades, or connect frames. This is hosted production-path test proof, not a local or external-service run. Run: https://github.com/openclaw/openclaw/actions/runs/34084976323maxPayloadscanner annotation was inspected at the new test fixture. It concerns a temporary HTTPS/WebSocket server bound to a free loopback port, driven by synthetic test traffic and closed infinally, not a deployed Gateway listener or a changed production payload limit. No production change or scanner suppression was added.The socket proofs use isolated state, not a native service manager. The new gather-to-TLS rows were validated by hosted CI, not run locally against the complete integrated dependency tree. No new external live-service proof is claimed. Raw local proof logs are not published.
Earlier Contributor Evidence
The contributor previously reported a managed loopback auth-none Gateway accepting
gateway status --deep --require-rpc --json, while an explicit--urlwithout credentials was rejected. This is historical contributor evidence, not a new live-service run against the repaired candidate.AI Assistance
The contributor reported AI-assisted analysis and implementation using Codex CLI, followed by managed-Gateway proof. Maintainer updates include independent review, scoped target/auth repairs, and targeted regression and typechecking evidence.