Skip to content

Commit fbd7843

Browse files
committed
fix findings
1 parent 94df5d5 commit fbd7843

5 files changed

Lines changed: 49 additions & 22 deletions

File tree

CHANGELOG.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2323
`authToken` is a SecretRef pointing at our own provider — that shape
2424
is only ever written by `writeStoredToken()` after device auth, so
2525
the plugin (not the user) owns recovery.
26+
- `pollDeviceAuth()` now `encodeURIComponent()`s the device-auth code
27+
before interpolating it into the poll URL. Defense-in-depth against
28+
a compromised or MITM-ed server returning a code with URL meta-chars
29+
that would silently redirect polling to a different endpoint.
30+
- `submitAudit()` now validates that `report.markdown` is a string on
31+
the success path. A malformed server response previously surfaced as
32+
a confusing `TypeError: Cannot read properties of undefined (reading
33+
'markdown')`; it now throws a clear
34+
"unexpected response shape" error.
35+
36+
### Changed
37+
38+
- Removed the unreachable `{ kind: "pending" }` variant from
39+
`DeviceAuthPollResult`. `pollDeviceAuth()` loops internally and only
40+
returns terminal states or `timeout`, so the `"pending"` branch in
41+
`runShellSecurityFlow` was dead code and confused the contract.
42+
- Renumbered the ordered list in `src/platform.ts`'s module doc
43+
comment. Signals 2–5 are now 1–4.
2644

2745
## [0.2.0]
2846

index.ts

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -291,23 +291,14 @@ async function runShellSecurityFlow(
291291
return "Authentication code expired. Run the security checkup again to get a fresh code.";
292292
}
293293

294-
if (pollResult.kind === "timeout") {
295-
// Our local poll deadline was hit while the server was still
296-
// returning pending. The code may still be valid server-side.
297-
// Leave the pending code in place so the next invocation picks up
298-
// where we left off, and tell the user to retry once they've
299-
// approved in the browser.
300-
return (
301-
"Still waiting for you to approve in the browser.\n\n" +
302-
"Once you've approved, run the security checkup again and we'll pick up where we left off."
303-
);
304-
}
305-
// pollResult.kind === "pending" (shouldn't reach here: pollDeviceAuth
306-
// loops internally until a terminal state or timeout). Fall through
307-
// to treat as timeout for safety.
294+
// pollResult.kind === "timeout": our local poll deadline was hit
295+
// while the server was still returning pending. The code may still
296+
// be valid server-side. Leave the pending code in place so the
297+
// next invocation picks up where we left off, and tell the user
298+
// to retry once they've approved in the browser.
308299
return (
309300
"Still waiting for you to approve in the browser.\n\n" +
310-
"Once you've approved, run the security checkup again."
301+
"Once you've approved, run the security checkup again and we'll pick up where we left off."
311302
);
312303
}
313304

src/auth/device-auth.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,13 @@ export type DeviceAuthStartResult = {
5050
* was still returning pending. The code may still be valid
5151
* server-side; caller should NOT clear pending code so the
5252
* next invocation can keep polling.
53+
*
54+
* `pending` is intentionally NOT in this union. `pollDeviceAuth()` loops
55+
* internally and never returns the transient pending state — it only
56+
* returns a terminal outcome or `timeout`.
5357
*/
5458
export type DeviceAuthPollResult =
5559
| { kind: "approved"; token: string }
56-
| { kind: "pending" }
5760
| { kind: "denied" }
5861
| { kind: "expired" }
5962
| { kind: "timeout" };
@@ -112,7 +115,11 @@ export async function pollDeviceAuth(
112115
logger?: PluginLogger,
113116
): Promise<DeviceAuthPollResult> {
114117
const fetchFn: typeof fetch = resolveFetch() ?? globalThis.fetch;
115-
const pollUrl = `${apiBase}/api/device-auth/codes/${code}`;
118+
// Defense-in-depth: the code is a server-issued opaque string, but if
119+
// the server ever returned one containing `/` or other URL meta-chars
120+
// an unencoded concat would silently redirect the poll to a different
121+
// endpoint under the same origin.
122+
const pollUrl = `${apiBase}/api/device-auth/codes/${encodeURIComponent(code)}`;
116123
const deadline = Date.now() + POLL_TIMEOUT_MS;
117124

118125
while (Date.now() < deadline) {

src/client.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,5 +106,16 @@ export async function submitAudit(
106106
);
107107
}
108108

109-
return (await resp.json()) as AnalyzeResponse;
109+
const body = (await resp.json()) as AnalyzeResponse;
110+
// Guard against an unexpected success shape (e.g. a partial rollout
111+
// or a proxy rewriting the response). Without this, a missing
112+
// `report.markdown` surfaces as a confusing
113+
// `TypeError: Cannot read properties of undefined (reading 'markdown')`
114+
// from the caller; this message is actionable.
115+
if (typeof body?.report?.markdown !== "string") {
116+
throw new Error(
117+
"KiloCode analysis API returned an unexpected response shape.",
118+
);
119+
}
120+
return body;
110121
}

src/platform.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,15 @@
1313
* circuits to "kiloclaw".
1414
*
1515
* Ordering (stopping at the first hit):
16-
* 2. openclaw.json has `plugins.entries["kiloclaw-customizer"].enabled`
16+
* 1. openclaw.json has `plugins.entries["kiloclaw-customizer"].enabled`
1717
* truthy — the kiloclaw controller writes this at boot for every
1818
* kiloclaw instance, predating any of the env-var signals. Most
1919
* durable universal signal today.
20-
* 3. openclaw.json `plugins.load.paths` contains the kiloclaw
20+
* 2. openclaw.json `plugins.load.paths` contains the kiloclaw
2121
* customizer install path — same writer, redundant cross-check.
22-
* 4. `process.env.KILOCLAW_SANDBOX_ID` is set — present on every
22+
* 3. `process.env.KILOCLAW_SANDBOX_ID` is set — present on every
2323
* kiloclaw instance since 2026-03-22.
24-
* 5. `process.env.KILOCODE_FEATURE === "kiloclaw"` — the original
24+
* 4. `process.env.KILOCODE_FEATURE === "kiloclaw"` — the original
2525
* env-var signal, present on kiloclaw since 2026-02-17.
2626
*
2727
* We intentionally do NOT add a loose `KILOCLAW_*`-prefix heuristic;

0 commit comments

Comments
 (0)