Skip to content

Commit 006c4a0

Browse files
committed
fix(onboard): address web search review follow-ups
1 parent 1f3f0ff commit 006c4a0

7 files changed

Lines changed: 230 additions & 127 deletions

File tree

bin/lib/onboard.js

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -985,6 +985,10 @@ function normalizeWebSearchConfigValue(config) {
985985
return webSearch.normalizeWebSearchConfig(config);
986986
}
987987

988+
function normalizePersistedWebSearchConfigValue(config) {
989+
return webSearch.normalizePersistedWebSearchConfig(config);
990+
}
991+
988992
function getWebSearchProviderMetadata(provider) {
989993
return webSearch.getWebSearchProvider(provider);
990994
}
@@ -1103,6 +1107,10 @@ async function ensureValidatedWebSearchCredential(provider) {
11031107

11041108
while (true) {
11051109
if (!apiKey) {
1110+
if (isNonInteractive()) {
1111+
console.error(` ${credentialEnv} is required for ${label} in non-interactive mode.`);
1112+
return null;
1113+
}
11061114
apiKey = await promptWebSearchApiKey(provider);
11071115
usingSavedKey = false;
11081116
}
@@ -1121,6 +1129,7 @@ async function ensureValidatedWebSearchCredential(provider) {
11211129
if (validation.message) {
11221130
console.error(` ${validation.message}`);
11231131
}
1132+
if (isNonInteractive()) return null;
11241133

11251134
const action = await promptWebSearchRecovery(provider, validation);
11261135
if (action === "back") return null;
@@ -4192,8 +4201,16 @@ async function onboard(opts = {}) {
41924201
break;
41934202
}
41944203

4195-
const persistedWebSearchConfig = normalizeWebSearchConfigValue(webSearchConfig);
4196-
if (persistedWebSearchConfig) {
4204+
const persistedWebSearchConfigRaw = normalizePersistedWebSearchConfigValue(webSearchConfig);
4205+
const persistedWebSearchConfig = normalizeWebSearchConfigValue(persistedWebSearchConfigRaw);
4206+
if (persistedWebSearchConfigRaw && persistedWebSearchConfigRaw.fetchEnabled === false) {
4207+
webSearchConfig = persistedWebSearchConfigRaw;
4208+
onboardSession.updateSession((current) => {
4209+
current.webSearchConfig = webSearchConfig;
4210+
return current;
4211+
});
4212+
note(" [resume] Keeping Web Search disabled.");
4213+
} else if (persistedWebSearchConfig) {
41974214
const { label } = getWebSearchProviderMetadata(persistedWebSearchConfig.provider);
41984215
note(` [resume] Revalidating ${label} configuration.`);
41994216
const apiKey = await ensureValidatedWebSearchCredential(persistedWebSearchConfig.provider);

docs/reference/commands.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,10 @@ Supported non-experimental choices include NVIDIA Endpoints, OpenAI, Anthropic,
7171
Credentials are stored in `~/.nemoclaw/credentials.json`.
7272
The legacy `nemoclaw setup` command is deprecated; use `nemoclaw onboard` instead.
7373

74-
If you enable Brave Search during onboarding, NemoClaw currently stores the Brave API key in the sandbox's OpenClaw configuration.
74+
If you enable web search during onboarding, NemoClaw currently stores the selected provider key in the sandbox's OpenClaw configuration as that provider's `apiKey`.
7575
That means the OpenClaw agent can read the key.
76-
NemoClaw explores an OpenShell-hosted credential path first, but the current OpenClaw Brave runtime does not consume that path end to end yet.
77-
Treat Brave Search as an explicit opt-in and use a dedicated low-privilege Brave key.
76+
NemoClaw explores an OpenShell-hosted credential path first, but the current OpenClaw web-search runtime does not consume that path end to end yet.
77+
Treat web search as an explicit opt-in and use a dedicated low-privilege provider key.
7878

7979
For non-interactive onboarding, you must explicitly accept the third-party software notice:
8080

@@ -97,6 +97,7 @@ $ BRAVE_API_KEY=... \
9797

9898
Supported keys are `BRAVE_API_KEY`, `GEMINI_API_KEY`, and `TAVILY_API_KEY`.
9999
If more than one is set, NemoClaw prefers Brave, then Gemini, then Tavily unless `NEMOCLAW_WEB_SEARCH_PROVIDER` is set explicitly to `brave`, `gemini`, or `tavily`.
100+
Whichever provider wins that selection has its key copied into the sandbox OpenClaw config, so agents can read the selected provider's `apiKey`.
100101
Enabling web search also enables `web_fetch`.
101102

102103
The wizard prompts for a sandbox name.

src/lib/onboard-session.test.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,15 +120,15 @@ describe("onboard session", () => {
120120
expect(loaded.metadata.token).toBeUndefined();
121121
});
122122

123-
it("normalizes legacy web search configs and clears explicit disable updates", () => {
123+
it("normalizes legacy web search configs and preserves explicit disable updates", () => {
124124
session.saveSession(session.createSession({ webSearchConfig: { fetchEnabled: true } }));
125125

126126
let loaded = session.loadSession();
127127
expect(loaded.webSearchConfig).toEqual({ provider: "brave", fetchEnabled: true });
128128

129129
session.completeSession({ webSearchConfig: { provider: "brave", fetchEnabled: false } });
130130
loaded = session.loadSession();
131-
expect(loaded.webSearchConfig).toBeNull();
131+
expect(loaded.webSearchConfig).toEqual({ provider: "brave", fetchEnabled: false });
132132
});
133133

134134
it("does not clear existing metadata when updates omit whitelisted metadata fields", () => {
@@ -204,14 +204,18 @@ describe("onboard session", () => {
204204
session.saveSession(session.createSession());
205205
session.markStepFailed(
206206
"inference",
207-
"provider auth failed with NVIDIA_API_KEY=nvapi-secret TAVILY_API_KEY=tvly-secret Bearer topsecret sk-secret-value ghp_1234567890123456789012345",
207+
"provider auth failed with NVIDIA_API_KEY=nvapi-secret BRAVE_API_KEY=brv-secret GEMINI_API_KEY=gem-secret TAVILY_API_KEY=tvly-secret Bearer topsecret sk-secret-value ghp_1234567890123456789012345",
208208
);
209209

210210
const loaded = session.loadSession();
211211
expect(loaded.steps.inference.error).toContain("NVIDIA_API_KEY=<REDACTED>");
212+
expect(loaded.steps.inference.error).toContain("BRAVE_API_KEY=<REDACTED>");
213+
expect(loaded.steps.inference.error).toContain("GEMINI_API_KEY=<REDACTED>");
212214
expect(loaded.steps.inference.error).toContain("TAVILY_API_KEY=<REDACTED>");
213215
expect(loaded.steps.inference.error).toContain("Bearer <REDACTED>");
214216
expect(loaded.steps.inference.error).not.toContain("nvapi-secret");
217+
expect(loaded.steps.inference.error).not.toContain("brv-secret");
218+
expect(loaded.steps.inference.error).not.toContain("gem-secret");
215219
expect(loaded.steps.inference.error).not.toContain("tvly-secret");
216220
expect(loaded.steps.inference.error).not.toContain("topsecret");
217221
expect(loaded.steps.inference.error).not.toContain("sk-secret-value");

src/lib/onboard-session.ts

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@
1010
import fs from "node:fs";
1111
import path from "node:path";
1212

13-
import { normalizeWebSearchConfig, type WebSearchConfig } from "./web-search";
13+
import {
14+
getWebSearchCredentialEnvNames,
15+
normalizePersistedWebSearchConfig,
16+
type PersistedWebSearchConfig,
17+
} from "./web-search";
1418

1519
export const SESSION_VERSION = 1;
1620
export const SESSION_DIR = path.join(process.env.HOME || "/tmp", ".nemoclaw");
@@ -55,7 +59,7 @@ export interface Session {
5559
credentialEnv: string | null;
5660
preferredInferenceApi: string | null;
5761
nimContainer: string | null;
58-
webSearchConfig: WebSearchConfig | null;
62+
webSearchConfig: PersistedWebSearchConfig | null;
5963
policyPresets: string[] | null;
6064
metadata: SessionMetadata;
6165
steps: Record<string, StepState>;
@@ -84,7 +88,7 @@ export interface SessionUpdates {
8488
credentialEnv?: string;
8589
preferredInferenceApi?: string;
8690
nimContainer?: string;
87-
webSearchConfig?: WebSearchConfig | null;
91+
webSearchConfig?: PersistedWebSearchConfig | null;
8892
policyPresets?: string[];
8993
metadata?: { gatewayName?: string };
9094
}
@@ -119,13 +123,30 @@ export function isObject(value: unknown): value is Record<string, unknown> {
119123
return typeof value === "object" && value !== null && !Array.isArray(value);
120124
}
121125

126+
function escapeRegExp(value: string): string {
127+
return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
128+
}
129+
130+
const REDACTED_CREDENTIAL_ENV_NAMES = Array.from(
131+
new Set([
132+
"NVIDIA_API_KEY",
133+
"OPENAI_API_KEY",
134+
"ANTHROPIC_API_KEY",
135+
"COMPATIBLE_API_KEY",
136+
"COMPATIBLE_ANTHROPIC_API_KEY",
137+
...getWebSearchCredentialEnvNames(),
138+
]),
139+
);
140+
141+
const CREDENTIAL_ASSIGNMENT_RE = new RegExp(
142+
`(${REDACTED_CREDENTIAL_ENV_NAMES.map(escapeRegExp).join("|")})=\\S+`,
143+
"gi",
144+
);
145+
122146
export function redactSensitiveText(value: unknown): string | null {
123147
if (typeof value !== "string") return null;
124148
return value
125-
.replace(
126-
/(NVIDIA_API_KEY|OPENAI_API_KEY|ANTHROPIC_API_KEY|GEMINI_API_KEY|COMPATIBLE_API_KEY|COMPATIBLE_ANTHROPIC_API_KEY|BRAVE_API_KEY|TAVILY_API_KEY)=\S+/gi,
127-
"$1=<REDACTED>",
128-
)
149+
.replace(CREDENTIAL_ASSIGNMENT_RE, "$1=<REDACTED>")
129150
.replace(/Bearer\s+\S+/gi, "Bearer <REDACTED>")
130151
.replace(/nvapi-[A-Za-z0-9_-]{10,}/g, "<REDACTED>")
131152
.replace(/ghp_[A-Za-z0-9]{20,}/g, "<REDACTED>")
@@ -192,7 +213,7 @@ export function createSession(overrides: Partial<Session> = {}): Session {
192213
credentialEnv: overrides.credentialEnv || null,
193214
preferredInferenceApi: overrides.preferredInferenceApi || null,
194215
nimContainer: overrides.nimContainer || null,
195-
webSearchConfig: normalizeWebSearchConfig(overrides.webSearchConfig),
216+
webSearchConfig: normalizePersistedWebSearchConfig(overrides.webSearchConfig),
196217
policyPresets: Array.isArray(overrides.policyPresets)
197218
? overrides.policyPresets.filter((value) => typeof value === "string")
198219
: null,
@@ -223,7 +244,7 @@ export function normalizeSession(data: unknown): Session | null {
223244
preferredInferenceApi:
224245
typeof d.preferredInferenceApi === "string" ? d.preferredInferenceApi : null,
225246
nimContainer: typeof d.nimContainer === "string" ? d.nimContainer : null,
226-
webSearchConfig: normalizeWebSearchConfig(d.webSearchConfig),
247+
webSearchConfig: normalizePersistedWebSearchConfig(d.webSearchConfig),
227248
policyPresets: Array.isArray(d.policyPresets)
228249
? (d.policyPresets as unknown[]).filter((value) => typeof value === "string") as string[]
229250
: null,
@@ -410,14 +431,9 @@ export function filterSafeUpdates(updates: SessionUpdates): Partial<Session> {
410431
if (updates.webSearchConfig === null) {
411432
safe.webSearchConfig = null;
412433
} else {
413-
const normalizedWebSearchConfig = normalizeWebSearchConfig(updates.webSearchConfig);
434+
const normalizedWebSearchConfig = normalizePersistedWebSearchConfig(updates.webSearchConfig);
414435
if (normalizedWebSearchConfig) {
415436
safe.webSearchConfig = normalizedWebSearchConfig;
416-
} else if (
417-
isObject(updates.webSearchConfig) &&
418-
updates.webSearchConfig.fetchEnabled === false
419-
) {
420-
safe.webSearchConfig = null;
421437
}
422438
}
423439
}

src/lib/web-search.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ describe("web-search helpers", () => {
3333
});
3434
});
3535

36+
it("rejects persisted configs with unsupported providers", () => {
37+
expect(normalizeWebSearchConfig({ provider: "duckduckgo", fetchEnabled: true })).toBeNull();
38+
});
39+
3640
it("builds the Brave Search OpenClaw config fragment", () => {
3741
expect(
3842
buildWebSearchConfigFragment({ provider: "brave", fetchEnabled: true }, "brv-x"),
@@ -92,6 +96,38 @@ describe("web-search helpers", () => {
9296
});
9397
});
9498

99+
it("encodes Tavily Search docker config using the Tavily plugin entry", () => {
100+
const encoded = buildWebSearchDockerConfig(
101+
{ provider: "tavily", fetchEnabled: true },
102+
"tvly-x",
103+
);
104+
expect(JSON.parse(Buffer.from(encoded, "base64").toString("utf8"))).toEqual({
105+
plugins: {
106+
entries: {
107+
tavily: {
108+
enabled: true,
109+
config: {
110+
webSearch: {
111+
apiKey: "tvly-x",
112+
},
113+
},
114+
},
115+
},
116+
},
117+
tools: {
118+
web: {
119+
search: {
120+
enabled: true,
121+
provider: "tavily",
122+
},
123+
fetch: {
124+
enabled: true,
125+
},
126+
},
127+
},
128+
});
129+
});
130+
95131
it("includes provider-specific exposure caveats in the warning text", () => {
96132
const warning = getWebSearchExposureWarningLines("tavily").join(" ");
97133
expect(warning).toContain("Tavily API key");

src/lib/web-search.ts

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@ export interface WebSearchConfig {
88
fetchEnabled: boolean;
99
}
1010

11+
export interface DisabledWebSearchConfig {
12+
provider?: WebSearchProvider;
13+
fetchEnabled: false;
14+
}
15+
16+
export type PersistedWebSearchConfig = WebSearchConfig | DisabledWebSearchConfig;
17+
1118
export interface WebSearchProviderMetadata {
1219
provider: WebSearchProvider;
1320
label: string;
@@ -74,14 +81,36 @@ export function getWebSearchProvider(provider: WebSearchProvider): WebSearchProv
7481
return WEB_SEARCH_PROVIDERS[provider];
7582
}
7683

77-
export function normalizeWebSearchConfig(value: unknown): WebSearchConfig | null {
78-
if (!isObject(value) || value.fetchEnabled !== true) return null;
84+
export function normalizePersistedWebSearchConfig(
85+
value: unknown,
86+
): PersistedWebSearchConfig | null {
87+
if (!isObject(value) || typeof value.fetchEnabled !== "boolean") return null;
88+
89+
if (value.fetchEnabled === false) {
90+
const provider =
91+
value.provider === undefined ? undefined : parseWebSearchProvider(value.provider);
92+
if (value.provider !== undefined && !provider) return null;
93+
return provider ? { provider, fetchEnabled: false } : { fetchEnabled: false };
94+
}
95+
96+
const provider =
97+
value.provider === undefined ? "brave" : parseWebSearchProvider(value.provider);
98+
if (!provider) return null;
7999
return {
80-
provider: parseWebSearchProvider(value.provider) || "brave",
100+
provider,
81101
fetchEnabled: true,
82102
};
83103
}
84104

105+
export function normalizeWebSearchConfig(value: unknown): WebSearchConfig | null {
106+
const normalized = normalizePersistedWebSearchConfig(value);
107+
return normalized?.fetchEnabled === true ? normalized : null;
108+
}
109+
110+
export function getWebSearchCredentialEnvNames(): string[] {
111+
return listWebSearchProviders().map((provider) => provider.credentialEnv);
112+
}
113+
85114
export function getWebSearchExposureWarningLines(provider: WebSearchProvider): string[] {
86115
const { label } = getWebSearchProvider(provider);
87116
return [

0 commit comments

Comments
 (0)