Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion mcp/lib/guard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export function isAllowedPath(abs: string): boolean {
* Used by the `bash` pre-flight, which sees a shell string rather than a path.
*/
export const SECRET_NAME_RE =
/(^|[^\w.-])\.(ssh|hermes|openclaw|codex|gnupg|aws|kube|env|envrc|netrc|npmrc|pypirc|pgpass|git-credentials|session-secret|mcp-token|local-ai-token|hermes-dashboard-pw)(?![\w-])|(^|[^\w-])id_(rsa|ecdsa|ed25519)(?![\w-])/i;
/(^|[^\w.-])\.(ssh|hermes|openclaw|clawkeep|codex|gnupg|aws|kube|env|envrc|netrc|npmrc|pypirc|pgpass|git-credentials|session-secret|mcp-token|local-ai-token|hermes-dashboard-pw)(?![\w-])|(^|[^\w-])id_(rsa|ecdsa|ed25519)(?![\w-])/i;

/**
* Throw a BLOCKED_PATH the agent can act on. The message deliberately names no
Expand Down
15 changes: 9 additions & 6 deletions src/app/setup-api/ai-models/configure/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import { NextResponse } from "next/server";
import { spawn } from "child_process";
import fs from "fs/promises";
import path from "path";
import { DATA_DIR, getAll, setMany } from "@/lib/config-store";
import { getAll, setMany } from "@/lib/config-store";
import { HANDOFF_TOKENS_PATH, HANDOFF_TTL_MS } from "@/lib/oauth-handoff";
import {
restartGateway,
findOpenclawBin,
Expand Down Expand Up @@ -483,7 +484,6 @@ export async function POST(request: Request) {
// 15-minute TTL rather than forcing a full re-auth.
let pendingHandoffTokensPath: string | null = null;
if (body.authMode === "subscription" && body.oauthHandoff) {
const tokensPath = path.join(DATA_DIR, "oauth-device-tokens.json");
let handoff: {
provider?: string;
access_token?: string;
Expand All @@ -493,19 +493,22 @@ export async function POST(request: Request) {
createdAt?: number;
};
try {
handoff = JSON.parse(await fs.readFile(tokensPath, "utf-8"));
handoff = JSON.parse(await fs.readFile(HANDOFF_TOKENS_PATH, "utf-8"));
} catch {
return NextResponse.json(
{ error: "No pending OAuth tokens. Restart the sign-in flow." },
{ status: 400 },
);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
// A file with no `createdAt` has no age we can check, so it cannot be
// shown to be inside the TTL — treat it the same as one that is past it.
if (
!handoff.access_token ||
(handoff.createdAt && Date.now() - handoff.createdAt > 15 * 60 * 1000)
!handoff.createdAt ||
Date.now() - handoff.createdAt > HANDOFF_TTL_MS
) {
// Stale/invalid credential material — consume it so it can't linger.
await fs.unlink(tokensPath).catch(() => {});
await fs.unlink(HANDOFF_TOKENS_PATH).catch(() => {});
Comment on lines 505 to +511

@coderabbitai coderabbitai Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Validate the timestamp value at runtime.

A truthy non-number value produces NaN during subtraction. A future timestamp produces a negative age. Both values bypass this expiry check and let the handoff proceed.

Require a finite numeric createdAt that is not later than the current time before applying HANDOFF_TTL_MS. Add tests for malformed and future timestamps.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/app/setup-api/ai-models/configure/route.ts` around lines 505 - 511,
Strengthen the credential validation around handoff.createdAt so it must be a
finite number no later than the current time before applying HANDOFF_TTL_MS;
reject and unlink the handoff for malformed, NaN, or future timestamps. Add
tests covering non-numeric and future createdAt values while preserving existing
stale-token cleanup behavior.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taken — this was a real hole in the line the branch had just changed.

Both cases you name slip through for the same reason: Date.now() - createdAt is NaN for a non-numeric value and negative for a future one, and neither is > TTL, so the file was accepted no matter how old it was. Replacing !handoff.createdAt with a presence check alone would not have covered either.

The age is now computed only from a createdAt that is a finite number, and a null age or a negative one is refused and removed exactly like an over-age file. Tests cover a string, NaN and a future timestamp.

Fixed in 6162632.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

git show --stat --oneline 6162632
git show --format=fuller --find-renames --find-copies -- \
  src/app/setup-api/ai-models/configure/route.ts \
  src/tests/routes/ai-models/configure.test.ts \
  6162632

printf '\n--- Current validation hunk ---\n'
rg -n -A22 -B8 'createdAt|HANDOFF_TTL_MS|handoffAge' \
  src/app/setup-api/ai-models/configure/route.ts

printf '\n--- Relevant tests ---\n'
rg -n -A15 -B8 'createdAt|future|NaN|non-numeric|handoff' \
  src/tests/routes/ai-models/configure.test.ts

Length of output: 2147


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- Current OAuth handoff validation ---'
rg -n -A30 -B10 'createdAt|HANDOFF_TTL_MS|handoffAge' \
  src/app/setup-api/ai-models/configure/route.ts || true

printf '%s\n' '--- Configure route test structure ---'
ast-grep outline src/tests/routes/ai-models/configure.test.ts --items all --type function

printf '%s\n' '--- Timestamp-related regression tests ---'
rg -n -A20 -B10 'createdAt|future|NaN|non-numeric|handoff|expired' \
  src/tests/routes/ai-models/configure.test.ts || true

Length of output: 12933


@KrasimirKralev, verification is inconclusive for commit 6162632 because that commit is not available in the checked-out repository.

The current configure/route.ts still uses:

!handoff.createdAt ||
Date.now() - handoff.createdAt > HANDOFF_TTL_MS

This code still accepts truthy non-numeric timestamps and future timestamps. The current tests cover missing and expired timestamps only. They do not include the stated string, NaN, or future-timestamp cases.

Please retry after the PR branch contains 6162632. The finding remains unresolved.


🧬 Code Graph Analysis Results

src/lib/oauth-handoff.ts:18,24

export const HANDOFF_TOKENS_PATH = path.join(DATA_DIR, "oauth-device-tokens.json");
export const HANDOFF_TTL_MS = 15 * 60 * 1000;

Defines the OAuth handoff file location and its 15-minute expiration interval.

src/app/setup-api/ai-models/oauth/device-start/route.ts:85-106

await fs.writeFile(
  tmpPath,
  JSON.stringify({
    provider: providerName,
    device_id: deviceId,
    user_code: userCode,
    interval,
    createdAt: Date.now(),
  }),
  { mode: 0o600 }
);
await fs.rename(tmpPath, STATE_PATH);

Creates device-auth state with createdAt as a numeric millisecond timestamp and writes it atomically.

src/app/setup-api/ai-models/oauth/device-poll/route.ts:195-210

// 15-minute expiry
if (Date.now() - stored.createdAt > HANDOFF_TTL_MS) {
  await fs.unlink(STATE_PATH).catch(() => {});
  return NextResponse.json(
    { error: "Device auth session expired. Please start again." },
    { status: 400 }
  );
}

Provides the analogous device-auth expiry check and removes expired state files.

You are interacting with an AI system.

return NextResponse.json(
{ error: "OAuth tokens missing or expired. Restart the sign-in flow." },
{ status: 400 },
Expand All @@ -519,7 +522,7 @@ export async function POST(request: Request) {
body.idToken = handoff.id_token;
body.refreshToken = handoff.refresh_token;
body.expiresIn = handoff.expires_in;
pendingHandoffTokensPath = tokensPath;
pendingHandoffTokensPath = HANDOFF_TOKENS_PATH;
}

const { provider, apiKey, authMode = "token", idToken, refreshToken, expiresIn, projectId, scope = "primary", model: bodyModel } = body;
Expand Down
35 changes: 29 additions & 6 deletions src/app/setup-api/ai-models/oauth/device-poll/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@ import crypto from "crypto";
import fs from "fs/promises";
import path from "path";
import { DATA_DIR } from "@/lib/config-store";
import {
HANDOFF_TOKENS_PATH,
HANDOFF_TTL_MS,
clearHandoffTokens,
sweepStaleHandoffTokens,
} from "@/lib/oauth-handoff";
import {
OPENAI_CLIENT_ID,
OPENAI_DEVICE_TOKEN_URL,
Expand All @@ -13,7 +19,22 @@ import {
export const dynamic = "force-dynamic";

const STATE_PATH = path.join(DATA_DIR, "oauth-device-state.json");
const TOKENS_PATH = path.join(DATA_DIR, "oauth-device-tokens.json");

/**
* Drop both halves of a sign-in the provider ended without completing: the
* in-flight state, and any handoff tokens an earlier attempt left behind.
*
* Used on the provider-failure branches only. The expiry branch in POST drops
* the state alone, because a device-code state can be expired while the handoff
* file holds fresh tokens from the *other* (authorization-code) flow; those are
* left for the age sweep to judge on their own timestamp.
*/
async function discardFlow(): Promise<void> {
await Promise.all([
fs.unlink(STATE_PATH).catch(() => {}),
clearHandoffTokens(),
]);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

interface DeviceTokens {
access_token?: string;
Expand All @@ -32,13 +53,13 @@ async function persistTokensAndAck(
tokens: DeviceTokens,
): Promise<NextResponse> {
await fs.mkdir(DATA_DIR, { recursive: true });
const tmpPath = `${TOKENS_PATH}.tmp.${crypto.randomBytes(8).toString("hex")}`;
const tmpPath = `${HANDOFF_TOKENS_PATH}.tmp.${crypto.randomBytes(8).toString("hex")}`;
await fs.writeFile(
tmpPath,
JSON.stringify({ provider, ...tokens, createdAt: Date.now() }),
{ mode: 0o600 },
);
await fs.rename(tmpPath, TOKENS_PATH);
await fs.rename(tmpPath, HANDOFF_TOKENS_PATH);
return NextResponse.json({ status: "complete" });
}

Expand Down Expand Up @@ -108,7 +129,7 @@ async function pollOpenAI(stored: StoredState): Promise<NextResponse> {
const verifier = pollData.code_verifier;
if (!verifier) {
console.error("[device-poll/openai] No code_verifier in poll response:", pollData);
await fs.unlink(STATE_PATH).catch(() => {});
await discardFlow();
return NextResponse.json(
{ error: "OpenAI did not return code_verifier" },
{ status: 502 }
Expand Down Expand Up @@ -141,7 +162,7 @@ async function pollOpenAI(stored: StoredState): Promise<NextResponse> {
exchangeRes.status,
errText
);
await fs.unlink(STATE_PATH).catch(() => {});
await discardFlow();
return NextResponse.json(
{ error: `Token exchange failed (${exchangeRes.status})` },
{ status: 502 }
Expand Down Expand Up @@ -173,6 +194,8 @@ async function pollOpenAI(stored: StoredState): Promise<NextResponse> {

export async function POST() {
try {
await sweepStaleHandoffTokens();

let stored: StoredState;
try {
const raw = await fs.readFile(STATE_PATH, "utf-8");
Expand All @@ -189,7 +212,7 @@ export async function POST() {
if (!stored.device_id && stored.device_auth_id) stored.device_id = stored.device_auth_id;

// 15-minute expiry
if (Date.now() - stored.createdAt > 15 * 60 * 1000) {
if (Date.now() - stored.createdAt > HANDOFF_TTL_MS) {
await fs.unlink(STATE_PATH).catch(() => {});
return NextResponse.json(
{ error: "Device auth session expired. Please start again." },
Expand Down
4 changes: 2 additions & 2 deletions src/app/setup-api/ai-models/oauth/device-start/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,19 @@ import crypto from "crypto";
import fs from "fs/promises";
import path from "path";
import { DATA_DIR } from "@/lib/config-store";
import { clearHandoffTokens } from "@/lib/oauth-handoff";
import { DEVICE_AUTH_PROVIDERS } from "@/lib/oauth-config";

export const dynamic = "force-dynamic";

const STATE_PATH = path.join(DATA_DIR, "oauth-device-state.json");
const TOKENS_PATH = path.join(DATA_DIR, "oauth-device-tokens.json");

export async function POST(request: Request) {
try {
// A new sign-in flow supersedes any prior one. Best-effort clear a stale
// token-handoff file left behind by an abandoned earlier flow so it can
// never be consumed by a later configure call.
await fs.unlink(TOKENS_PATH).catch(() => {});
await clearHandoffTokens();

let body: { provider?: string } = {};
try {
Expand Down
11 changes: 6 additions & 5 deletions src/app/setup-api/ai-models/oauth/exchange/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@ import crypto from "crypto";
import fs from "fs/promises";
import path from "path";
import { DATA_DIR } from "@/lib/config-store";
// Server-only handoff file the configure route reads on the `oauthHandoff`
// path — the SAME file the device-code flow (device-poll) uses, so both take
// its path from the one module that owns it.
import { HANDOFF_TOKENS_PATH } from "@/lib/oauth-handoff";
import { OAUTH_PROVIDERS, isGoogleConfigured } from "@/lib/oauth-config";
import { discoverGoogleProject } from "@/lib/google-project";

const STATE_PATH = path.join(DATA_DIR, "oauth-state.json");
// Server-only handoff file the configure route reads on the `oauthHandoff`
// path — the SAME file the device-code flow (device-poll) uses.
const TOKENS_PATH = path.join(DATA_DIR, "oauth-device-tokens.json");

// Persist the freshly-issued provider tokens to a 0600 server file and return
// just a status, so the access/refresh/id tokens never travel back through the
Expand All @@ -24,13 +25,13 @@ async function persistTokensAndAck(
extra?: { projectId?: string },
): Promise<NextResponse> {
await fs.mkdir(DATA_DIR, { recursive: true });
const tmpPath = `${TOKENS_PATH}.tmp.${crypto.randomBytes(8).toString("hex")}`;
const tmpPath = `${HANDOFF_TOKENS_PATH}.tmp.${crypto.randomBytes(8).toString("hex")}`;
await fs.writeFile(
tmpPath,
JSON.stringify({ provider, ...tokens, createdAt: Date.now() }),
{ mode: 0o600 },
);
await fs.rename(tmpPath, TOKENS_PATH);
await fs.rename(tmpPath, HANDOFF_TOKENS_PATH);
return NextResponse.json({ status: "complete", ...(extra?.projectId ? { projectId: extra.projectId } : {}) });
}

Expand Down
6 changes: 6 additions & 0 deletions src/app/setup-api/ai-models/oauth/start/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import crypto from "crypto";
import fs from "fs/promises";
import path from "path";
import { DATA_DIR } from "@/lib/config-store";
import { clearHandoffTokens } from "@/lib/oauth-handoff";
import { OAUTH_PROVIDERS, isGoogleConfigured } from "@/lib/oauth-config";

const STATE_PATH = path.join(DATA_DIR, "oauth-state.json");
Expand All @@ -15,6 +16,11 @@ function base64url(buf: Buffer): string {

export async function POST(request: Request) {
try {
// A new sign-in supersedes any prior one — same rule the device-code entry
// point applies, so the handoff file always belongs to the flow in progress
// rather than to whichever one was abandoned last.
await clearHandoffTokens();
Comment thread
coderabbitai[bot] marked this conversation as resolved.

let body: { provider?: string } = {};
try {
body = await request.json();
Expand Down
14 changes: 11 additions & 3 deletions src/app/setup-api/preferences/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as config from "@/lib/config-store";
import { getActiveHarness } from "@/lib/harness";
import { sanitizePreferences, validatePreference } from "@/lib/preference-schema";
import { personaFilesFor, writeLanguagePersona } from "@/lib/language-persona";
import { logSafe } from "@/lib/log-safe";

export const dynamic = "force-dynamic";

Expand All @@ -27,7 +28,11 @@ export async function GET(req: Request) {
if (allParam) {
// Return all preferences
const allConfig = await config.getAll();
const result: Record<string, unknown> = {};
// Null-prototype accumulator: the names come from outside this function, so
// an assignment here should always define an own property and never reach
// an inherited one such as `__proto__`. Same below, and in
// sanitizePreferences, which is where these objects end up.
const result: Record<string, unknown> = Object.create(null);
for (const [key, value] of Object.entries(allConfig)) {
if (key.startsWith("pref:")) {
result[key.slice(5)] = value;
Expand All @@ -41,7 +46,7 @@ export async function GET(req: Request) {
return NextResponse.json({ error: "keys or all param required" }, { status: 400 });
}
const keys = keysParam.split(",").filter(isAllowed);
const result: Record<string, unknown> = {};
const result: Record<string, unknown> = Object.create(null);
for (const key of keys) {
result[key] = await config.get(`pref:${key}`);
}
Expand All @@ -61,7 +66,10 @@ export async function POST(req: Request) {
if (!isAllowed(key)) continue;
const check = validatePreference(key, value);
if (!check.ok) {
console.error(`[preferences] Rejected write: ${check.reason}`);
// The reason is built from the rejected key, which is caller-supplied
// and only prefix-checked — bound and sanitise it like any other
// request-derived log field.
console.error(`[preferences] Rejected write: ${logSafe(check.reason ?? "")}`);
return NextResponse.json({ error: check.reason ?? "Invalid preference value" }, { status: 400 });
}
entries[`pref:${key}`] = value;
Expand Down
19 changes: 15 additions & 4 deletions src/app/setup-api/wifi/update/route.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
import { NextResponse } from "next/server";
import { execFile } from "child_process";
import { promisify } from "util";
import { logSafe } from "@/lib/log-safe";

const execFileAsync = promisify(execFile);

export const dynamic = "force-dynamic";

const AP_PROFILE = "ClawBox-Setup";

// 802.11 defines the SSID element as at most 32 octets, so a longer value
// cannot name a network nmcli could act on.
const SSID_MAX_OCTETS = 32;

export async function POST(request: Request) {
let body: { ssid?: string; password?: string; action?: "update" | "forget" };
try { body = await request.json(); } catch { return NextResponse.json({ error: "Invalid JSON" }, { status: 400 }); }
Expand All @@ -18,6 +23,9 @@
}
const normalizedSsid = (body.ssid ?? "").trim();
if (!normalizedSsid) return NextResponse.json({ error: "Network name is required" }, { status: 400 });
if (Buffer.byteLength(normalizedSsid, "utf8") > SSID_MAX_OCTETS) {
return NextResponse.json({ error: `Network name must be at most ${SSID_MAX_OCTETS} bytes` }, { status: 400 });
}
if (normalizedSsid === AP_PROFILE) return NextResponse.json({ error: "Cannot modify the hotspot profile here" }, { status: 400 });

try {
Expand All @@ -42,16 +50,19 @@
} catch (err) {
connected = false;
reactivateError = err instanceof Error ? err.message : "Failed to reconnect";
console.warn(`[wifi/update] reactivate ${normalizedSsid} failed:`, err);
console.warn(`[wifi/update] reactivate ${logSafe(normalizedSsid)} failed: ${logSafe(reactivateError)}`);
Comment thread
KrasimirKralev marked this conversation as resolved.
Dismissed
}
return NextResponse.json({ success: true, action: "update", connected, reactivateError });
} catch (err) {
// The `connection modify` argv includes `wifi-sec.psk <password>`, which
// execFile embeds into its error message — scrub the PSK before logging so
// it doesn't land in the journal in cleartext.
// it doesn't land in the journal in cleartext. The scrubbed text still
// carries the SSID nmcli echoed back, so it goes through logSafe too.
const raw = err instanceof Error ? err.message : String(err);
const safe = body.password ? raw.split(String(body.password)).join("***") : raw;
console.warn(`[wifi/update] ${action} ${normalizedSsid} failed: ${safe}`);
// replaceAll rather than split/join: the message is bounded by execFile's
// 1 MB maxBuffer, and split would allocate an array of every fragment of it.
const safe = body.password ? raw.replaceAll(String(body.password), "***") : raw;
console.warn(`[wifi/update] ${action} ${logSafe(normalizedSsid)} failed: ${logSafe(safe)}`);
Comment thread
KrasimirKralev marked this conversation as resolved.
Dismissed
return NextResponse.json({ error: "Failed to update WiFi network" }, { status: 500 });
}
}
Loading
Loading