nemoclaw/src/security/secret-scanner.test.ts (1)

24-97: Missing test coverage for Discord, AWS secret key, and Authorization header patterns.

The test suite covers 12 of the 14 secret patterns but is missing tests for:

• Discord bot token
• AWS secret key (the lookbehind pattern)
• Authorization header (the lookbehind pattern)

Since the lookbehind patterns have more complex matching logic, they warrant explicit test coverage to ensure they work as intended.

📝 Suggested additional test cases

Add to the FAKE object:

discord: "ABCDEFGHIJKLMNOPQRSTUVWx" + ".abc123" + ".ABCDEFGHIJKLMNOPQRSTUVWXYZa",
awsSecret: "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY",
authHeader: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0",

Add test cases:

it("Discord bot token", () => {
  const matches = scanForSecrets(`DISCORD_TOKEN=${FAKE.discord}`);
  expect(matches).toHaveLength(1);
  expect(matches[0].pattern).toBe("Discord bot token");
});

it("AWS secret key", () => {
  const matches = scanForSecrets(`aws_secret_access_key = ${FAKE.awsSecret}`);
  expect(matches).toHaveLength(1);
  expect(matches[0].pattern).toBe("AWS secret key");
});

it("Authorization header", () => {
  const matches = scanForSecrets(`Authorization: Bearer ${FAKE.authHeader}`);
  expect(matches).toHaveLength(1);
  expect(matches[0].pattern).toBe("Authorization header");
});

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@nemoclaw/src/security/secret-scanner.test.ts` around lines 24 - 97, The test
suite misses coverage for the Discord bot token, AWS secret key (lookbehind),
and Authorization header (lookbehind); update the FAKE test data object to
include keys discord, awsSecret, and authHeader with the suggested values and
add three tests calling scanForSecrets that assert one match and that
matches[0].pattern equals "Discord bot token", "AWS secret key", and
"Authorization header" respectively; place the new tests alongside the other
pattern tests so the lookbehind regexes exercised by scanForSecrets are covered.

nemoclaw/src/register.test.ts (1)

82-207: Good test coverage, but notebook_edit tool is not tested.

The test suite thoroughly covers write, edit, and apply_patch tools, but the notebook_edit tool (listed in WRITE_TOOL_NAMES in index.ts line 254) lacks explicit test coverage. Consider adding a test case for notebook_edit to ensure consistent behavior.

📝 Suggested test case for notebook_edit

it("blocks notebook_edit to memory path containing secrets", () => {
  const api = createMockApi();
  const handler = getHookHandler(api);
  const fakeKey = "nvapi-" + "abcdefghijklmnopqrstuvwxyz";
  const result = handler({
    toolName: "notebook_edit",
    params: {
      file_path: "/sandbox/.openclaw-data/memory/notebook.ipynb",
      content: `api key: ${fakeKey}`,
    },
  });
  expect(result).toMatchObject({ block: true });
});

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@nemoclaw/src/register.test.ts` around lines 82 - 207, Add a test covering the
"notebook_edit" tool similar to existing tests for "write"/"edit"/"apply_patch":
call getHookHandler(api) after register(api) and invoke the handler with
toolName "notebook_edit" and params containing a memory path (e.g.
"/sandbox/.openclaw-data/memory/notebook.ipynb") and secret-containing content;
assert the handler returns { block: true } (and optionally that api.logger.warn
was called). This ensures notebook_edit (one of WRITE_TOOL_NAMES referenced in
index.ts) is handled the same as other write tools by the register/on
before_tool_call hook.

nemoclaw/src/index.ts (1)

300-301: Unnecessary String() conversion.

matches.length is already a number, and template literals handle number-to-string coercion automatically. The explicit String() is redundant.

🧹 Minor cleanup

       return {
         block: true,
         blockReason:
-          `Memory write blocked: detected ${String(matches.length)} likely secret(s):\n${summary}\n\n` +
+          `Memory write blocked: detected ${matches.length} likely secret(s):\n${summary}\n\n` +
           "Remove secrets before saving to persistent memory. " +
           "Use environment variables or credential stores instead.",
       };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@nemoclaw/src/index.ts` around lines 300 - 301, The template literal for
blockReason uses an unnecessary String() conversion around matches.length;
remove the explicit String() so the expression becomes `${matches.length}` (or
just `${matches.length}` within the existing template) while keeping the summary
concatenation intact—update the code that builds blockReason (the blockReason
property that references matches and summary) to rely on template literal
coercion instead of String().

nemoclaw/src/security/secret-scanner.ts (1)

44-45: Consider tightening the Discord bot token pattern to reduce false positives.

The Discord bot token regex [A-Za-z0-9]{24}\.[A-Za-z0-9_-]{6}\.[A-Za-z0-9_-]{27,} could match non-secret strings that happen to have two dots separating alphanumeric segments (e.g., some base64-encoded JWTs or other structured identifiers).

Since the PR goal is high-confidence detection with low false positives, consider whether this pattern is narrow enough for the use case.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@nemoclaw/src/security/secret-scanner.ts` around lines 44 - 45, The current
"Discord bot token" entry in secret-scanner.ts is too permissive and can match
JWT/base64-like strings; update that entry (the object with name "Discord bot
token") to tighten detection by requiring contextual anchors (e.g., a positive
lookbehind for common labels like "discord", "bot", "DISCORD_TOKEN", "BOT_TOKEN"
or the literal "Bot " prefix) and disallow base64-only characters (exclude '='
'+' '/' in the token character classes) so the pattern only matches when the
token appears with typical surrounding text or env var names; ensure the
modified regex still matches legitimate Discord tokens but reduces false
positives by combining the contextual lookbehind and a stricter character class
for the three dot-separated segments.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

nemoclaw/src/security/secret-scanner.ts (2)

36-39: ⚠️ Potential issue | 🟠 Major

Make the context-anchored secret regexes case-insensitive.

AWS_SECRET_ACCESS_KEY= and authorization: bearer ... both bypass the scanner today because these two patterns are exact-case. That weakens the main block-on-write path for very common secret spellings.

🔐 Narrow fix

   {
     name: "AWS secret key",
-    regex: /(?<=aws_secret_access_key\s*[=:]\s*)[A-Za-z0-9/+=]{40}\b/,
+    regex: /(?<=aws_secret_access_key\s*[=:]\s*)[A-Za-z0-9/+=]{40}\b/i,
   },
@@
   {
     name: "Authorization header",
-    regex: /(?<=(?:Authorization\s*:\s*Bearer|Bearer\s*[=:])\s*["']?)[A-Za-z0-9._~+/=-]{40,}/,
+    regex: /(?<=(?:Authorization\s*:\s*Bearer|Bearer\s*[=:])\s*["']?)[A-Za-z0-9._~+/=-]{40,}/i,
   },

Also applies to: 61-64

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@nemoclaw/src/security/secret-scanner.ts` around lines 36 - 39, The regexes
for context-anchored secrets (e.g., the object with name "AWS secret key" whose
regex is /(?<=aws_secret_access_key\s*[=:]\s*)[A-Za-z0-9/+=]{40}\b/ and the
similar "authorization: bearer" pattern) are currently case-sensitive; update
those regex literals to be case-insensitive by adding the i flag (e.g.,
/(?<=aws_secret_access_key\s*[=:]\s*)[A-Za-z0-9/+=]{40}\b/i) and do the same for
the other context-anchored rules (the ones around the authorization/bearer
pattern and the rule at the 61-64 region) so the scanner matches common
upper/lowercase spellings.

---

106-120: ⚠️ Potential issue | 🟠 Major

Tighten isMemoryPath() to the actual persistent roots.

The bare "/MEMORY.md" substring overmatches unrelated files, while /.openclaw/workspace/, /.openclaw/agents/, /.openclaw/skills/, and /.openclaw/hooks/ are not protected at all. Right now isMemoryPath("/sandbox/.openclaw/workspace/USER.md") is false, but isMemoryPath("/sandbox/project/docs/MEMORY.md") is true.

📁 Suggested matcher update

 const MEMORY_PATH_SEGMENTS = [
   "/.openclaw-data/memory/",
   "/.openclaw/memory/",
-  "/MEMORY.md",
+  "/.openclaw/MEMORY.md",
+  "/.openclaw-data/MEMORY.md",
+  "/.openclaw/workspace/",
   "/.openclaw-data/workspace/",
+  "/.openclaw/agents/",
   "/.openclaw-data/agents/",
+  "/.openclaw/skills/",
   "/.openclaw-data/skills/",
+  "/.openclaw/hooks/",
   "/.openclaw-data/hooks/",
 ];

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@nemoclaw/src/security/secret-scanner.ts` around lines 106 - 120, The current
matcher overmatches "/MEMORY.md" and misses root ".openclaw" dirs; update
MEMORY_PATH_SEGMENTS to remove the bare "/MEMORY.md" entry and add the missing
root-scoped segments ("/.openclaw/workspace/", "/.openclaw/agents/",
"/.openclaw/skills/", "/.openclaw/hooks/") and then change
isMemoryPath(filePath) so it checks those segments with filePath.includes(...)
but handles the repository root MEMORY.md as an exact match (e.g. filePath ===
"/MEMORY.md" or normalize and compare to the repo root path) instead of
substring matching. Ensure you modify the MEMORY_PATH_SEGMENTS constant and the
isMemoryPath function accordingly.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

nemoclaw/src/security/secret-scanner.ts (1)

87-89: Static analysis false positive — patterns are static, not user-controlled.

The ast-grep ReDoS warning flags new RegExp(regex.source, flags) as risky, but this is a false positive since SECRET_PATTERNS is a hardcoded array in the same module, not derived from user input.

That said, recreating RegExp objects on every scanForSecrets() call is slightly wasteful. Consider precompiling the patterns with the g flag at module load time.

♻️ Optional: precompile patterns with global flag

-const SECRET_PATTERNS: SecretPattern[] = [
+const SECRET_PATTERNS: Array<{ name: string; regex: RegExp }> = [
   // NVIDIA
-  { name: "NVIDIA API key", regex: /\bnvapi-[A-Za-z0-9_-]{20,}\b/ },
+  { name: "NVIDIA API key", regex: /\bnvapi-[A-Za-z0-9_-]{20,}\b/g },
   // ... apply /g to all patterns ...
 ];

 export function scanForSecrets(content: string): SecretMatch[] {
   const matches: SecretMatch[] = [];
   const seen = new Set<string>();

   for (const { name, regex } of SECRET_PATTERNS) {
-    const flags = regex.flags.includes("g") ? regex.flags : `${regex.flags}g`;
-    for (const match of content.matchAll(new RegExp(regex.source, flags))) {
+    regex.lastIndex = 0; // reset stateful global regex
+    for (const match of content.matchAll(regex)) {
       const value = match[0];

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@nemoclaw/src/security/secret-scanner.ts` around lines 87 - 89,
SECRET_PATTERNS is a static array so you should avoid recreating RegExp
instances in scanForSecrets(); instead precompile each pattern once at module
load with the global flag and use those compiled regexes in scanForSecrets() to
eliminate the new RegExp(regex.source, flags) call and prevent repeated
allocation—ensure the precompilation logic (e.g., build a compiledSecrets array
from SECRET_PATTERNS where each entry has the RegExp with 'g' set) and update
scanForSecrets() to iterate over compiledSecrets and call content.matchAll on
the precompiled regex objects.

nemoclaw/src/index.ts (1)

282-284: Minor: Consider narrowing the type guard for stronger type safety.

The Partial<BeforeToolCallEvent> cast with optional chaining works, but you could strengthen the guard by explicitly checking the shape before narrowing:

if (
  typeof event !== "object" ||
  event === null ||
  typeof event.toolName !== "string" ||
  typeof event.params !== "object"
) {
  return undefined;
}

This is optional — the current approach is functionally safe since you check event?.toolName and event.params before use.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@nemoclaw/src/index.ts` around lines 282 - 284, The current guard in the
api.on("before_tool_call", ...) handler uses a Partial<BeforeToolCallEvent> cast
and optional chaining; replace it with an explicit runtime shape check to narrow
types more safely: verify event is a non-null object, event.toolName is a
string, and event.params is an object before proceeding, so the handler
(returning BeforeToolCallResult | undefined) can use event.toolName and
event.params with stronger type safety in the before_tool_call callback.