fix(gateway): resolve {} empty auth trap causing scope rejection in loopback probes#48950
fix(gateway): resolve {} empty auth trap causing scope rejection in loopback probes#48950waitingsnow wants to merge 3 commits intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR fixes a subtle but impactful bug in Key points:
Confidence Score: 5/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/gateway/probe.ts
Line: 50
Comment:
**Missing semicolon on return statement**
The `return` statement on this line lacks a terminating semicolon. While TypeScript's ASI will handle this correctly, the trailing semicolon in the inline comment (`(e.g. {});`) is comment text, not a statement terminator — which makes this easy to mistake for an intentional omission. Every other statement in the file uses a semicolon; this one should too for consistency.
```suggestion
return isLoopbackHost(hostname) && opts.auth === undefined; // Preserves device identity for empty but present auth objects (e.g. {})
```
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: 40daf06 |
src/gateway/probe.ts
Outdated
| // Local authenticated probes should stay device-bound so read/detail RPCs | ||
| // are not scope-limited by the shared-auth scope stripping hardening. | ||
| return isLoopbackHost(hostname) && !(opts.auth?.token || opts.auth?.password); | ||
| return isLoopbackHost(hostname) && opts.auth === undefined // Preserves device identity for empty but present auth objects (e.g. {}); |
There was a problem hiding this comment.
Missing test for the exact fixed scenario
The existing tests in probe.test.ts cover:
- Loopback with
auth: { token: "secret" }→deviceIdentitypreserved ✓ - Loopback with no
auth(undefined) →deviceIdentityisnull✓
But there is no test for the specific bug case: a loopback probe with auth: {} (empty object). This is the exact scenario described in the PR as the root cause. Without it, a future regression (e.g., reverting to a truthy-check) would go undetected. Consider adding:
it("keeps device identity enabled for authenticated loopback probes with empty auth object", async () => {
await probeGateway({
url: "ws://127.0.0.1:18789",
auth: {},
timeoutMs: 1_000,
});
expect(gatewayClientState.options?.deviceIdentity).toBeUndefined();
});Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gateway/probe.ts
Line: 50
Comment:
**Missing test for the exact fixed scenario**
The existing tests in `probe.test.ts` cover:
- Loopback with `auth: { token: "secret" }` → `deviceIdentity` preserved ✓
- Loopback with no `auth` (undefined) → `deviceIdentity` is `null` ✓
But there is **no test for the specific bug case**: a loopback probe with `auth: {}` (empty object). This is the exact scenario described in the PR as the root cause. Without it, a future regression (e.g., reverting to a truthy-check) would go undetected. Consider adding:
```typescript
it("keeps device identity enabled for authenticated loopback probes with empty auth object", async () => {
await probeGateway({
url: "ws://127.0.0.1:18789",
auth: {},
timeoutMs: 1_000,
});
expect(gatewayClientState.options?.deviceIdentity).toBeUndefined();
});
```
How can I resolve this? If you propose a fix, please make it concise.|
@greptile-apps I have added the exact unit test you requested for the |
src/gateway/probe.ts
Outdated
| // Local authenticated probes should stay device-bound so read/detail RPCs | ||
| // are not scope-limited by the shared-auth scope stripping hardening. | ||
| return isLoopbackHost(hostname) && !(opts.auth?.token || opts.auth?.password); | ||
| return isLoopbackHost(hostname) && opts.auth === undefined // Preserves device identity for empty but present auth objects (e.g. {}); |
There was a problem hiding this comment.
Missing semicolon on return statement
The return statement on this line lacks a terminating semicolon. While TypeScript's ASI will handle this correctly, the trailing semicolon in the inline comment ((e.g. {});) is comment text, not a statement terminator — which makes this easy to mistake for an intentional omission. Every other statement in the file uses a semicolon; this one should too for consistency.
| return isLoopbackHost(hostname) && opts.auth === undefined // Preserves device identity for empty but present auth objects (e.g. {}); | |
| return isLoopbackHost(hostname) && opts.auth === undefined; // Preserves device identity for empty but present auth objects (e.g. {}) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gateway/probe.ts
Line: 50
Comment:
**Missing semicolon on return statement**
The `return` statement on this line lacks a terminating semicolon. While TypeScript's ASI will handle this correctly, the trailing semicolon in the inline comment (`(e.g. {});`) is comment text, not a statement terminator — which makes this easy to mistake for an intentional omission. Every other statement in the file uses a semicolon; this one should too for consistency.
```suggestion
return isLoopbackHost(hostname) && opts.auth === undefined; // Preserves device identity for empty but present auth objects (e.g. {})
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
Confirmed locally on Qubes OS / Debian 13 AppVM with OpenClaw 2026.3.13. Repro:
Local files are correct after openclaw devices rotate:
This points to the status / probe auth path rather than pairing or local token persistence. |
4ae4fcb to
d932e1c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d932e1c119
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Local authenticated probes should stay device-bound so read/detail RPCs | ||
| // are not scope-limited by the shared-auth scope stripping hardening. | ||
| return isLoopbackHost(hostname) && !(opts.auth?.token || opts.auth?.password); | ||
| return isLoopbackHost(hostname) && opts.auth === undefined; // Preserves device identity for empty but present auth objects (e.g. {}) |
There was a problem hiding this comment.
Keep anonymous probes anonymous when auth is empty
This condition now treats any present auth object as authenticated, but several callers build auth objects even when both fields are empty (for example unresolved SecretRef paths that explicitly say they are probing without credentials). With opts.auth === undefined as the only anonymous check, loopback probes in those paths will attach device identity and can fall back to cached device-token auth, which changes semantics from an anonymous reachability probe to an authenticated probe and can hide credential-resolution problems in status output.
Useful? React with 👍 / 👎.
d932e1c to
d0ab570
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0ab57009f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Local authenticated probes should stay device-bound so read/detail RPCs | ||
| // are not scope-limited by the shared-auth scope stripping hardening. | ||
| return isLoopbackHost(hostname) && !(opts.auth?.token || opts.auth?.password); | ||
| return isLoopbackHost(hostname) && opts.auth === undefined; // Preserves device identity for empty but present auth objects (e.g. {}) |
There was a problem hiding this comment.
Keep empty-auth loopback probes anonymous
The new opts.auth === undefined check misclassifies empty auth objects as authenticated, even though multiple call paths intentionally produce empty auth when credentials are unavailable (for example src/commands/gateway-status.ts:175-183 always builds an auth object, and src/gateway/probe-auth.ts:79-82 returns {} with a warning that it is probing without credentials). For local loopback URLs this now preserves device identity and can fall back to cached device-token auth, changing an anonymous probe into an authenticated one and hiding credential-resolution problems in status diagnostics.
Useful? React with 👍 / 👎.
3467363 to
d0c9b4d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0c9b4d8c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Local authenticated probes should stay device-bound so read/detail RPCs | ||
| // are not scope-limited by the shared-auth scope stripping hardening. | ||
| return isLoopbackHost(hostname) && !(opts.auth?.token || opts.auth?.password); | ||
| return isLoopbackHost(hostname) && opts.auth === undefined; // Preserves device identity for empty but present auth objects (e.g. {}) |
There was a problem hiding this comment.
Preserve anonymous mode when auth object has no credentials
Changing the loopback check to opts.auth === undefined makes empty auth objects ({} or { token: undefined, password: undefined }) count as authenticated, which enables device identity and allows cached device-token fallback in GatewayClient.selectConnectAuth even when no probe credentials were resolved. That regresses the intended anonymous-probe semantics for callers that always pass an auth object (for example status.scan.shared.ts and gateway-status.ts), and can suppress unresolved-SecretRef diagnostics because those paths only emit auth warnings on probe failure. Fresh evidence: status-all.ts also always passes probeAuth from resolveGatewayProbeAuthSafe, so default local status probes now lose the anonymous path when no token/password is available.
Useful? React with 👍 / 👎.
d0c9b4d to
17d6e62
Compare
|
We're running a 5-agent OpenClaw fleet across two Linux hosts (Ubuntu, systemd user services, loopback bind, token auth) managed by a custom orchestration daemon. The daemon uses On 2026.3.12/2026.3.13, every CLI↔gateway call hangs for 30+ seconds before timing out — not intermittent, effectively 100% failure rate in our environment. Gateway logs show We're pinned to 2026.3.8 across all agents because of this. The 2026.3.22 handshake timeout raise (3s→10s) doesn't help — our CLI never sends the This PR's root cause analysis (empty |
Summary
Describe the problem and fix in 2–5 bullets:
openclaw statuslocally against a running gateway with authentication enabled fails withunreachable (missing scope: operator.read).{}trap): Instatus.scan.shared.ts, when local auth credentials are not explicitly provided via flags, the resolution logic returns an empty object (auth: {}). The original logic!(opts.auth?.token || opts.auth?.password)evaluated totruewhen given{}, incorrectly stripping the device identity and triggering scope rejection.opts.auth === undefined(a literal anonymous probe) to accurately distinguish an empty auth object from a lack of auth.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Closes #46014
Closes #45835
Closes #48538
Closes #46000
Closes #46358
Closes #47113
Closes #48007
Closes #48444
Closes #46897
Closes #47307
Closes #46117
Closes #46568
Closes #45908
Closes #48113
Closes #46422
Closes #47640
Closes #47987
Closes #46689
Closes #46583
Closes #47650
Closes #46716
Closes #48167
Closes #46100
Closes #46650
Closes #45945
Closes #46821
Closes #48002
Closes #17745
User-visible / Behavior Changes
openclaw statusto display diagnostic details instead of scope errors.Security Impact (required)
No)No)No)No)No)Yes, explain risk + mitigation: n/aRepro + Verification
Environment
Steps
openclaw statuslocally on a machine with a paired device.missing scope: operator.readbefore fix.Evidence
Attach at least one:
Human Verification (required)
What you personally verified (not just CI), and how:
openclaw statusreturns full JSON details when authenticated via token on loopback.Review Conversations
AI Assistance Transparency