chore(policy): harden loadPresetFromFile against oversize and symlinked inputs (#3020)

## Summary

Three follow-up hardening items from PR #2077 review against
`loadPresetFromFile` in `src/lib/policies.ts`: a 10 MB file-size guard,
symlink rejection via `O_NOFOLLOW`, and tmp-dir cleanup in the
corresponding test block. The size-and-symlink checks now use an atomic
`openSync(O_RDONLY | O_NOFOLLOW) → fstatSync → readFileSync(fd)` pattern
(per CodeRabbit review), so there is no TOCTOU window between the stat
and the read and the kernel rejects symbolic links at open time.

## Related Issue

Closes #2521

## Changes

- Add `MAX_PRESET_FILE_BYTES = 10_000_000` and reject files larger than
the limit before reading their contents.
- Replace the prior `existsSync` + `statSync` pair with an atomic
`openSync(O_RDONLY | O_NOFOLLOW)` followed by `fstatSync` on the
descriptor and `readFileSync(fd, "utf-8")`, with a `try/finally`
`closeSync` on every early-return path. Symbolic links are rejected via
`ELOOP` from `openSync` and surfaced with a clear error pointing the
user at `realpath`.
- Track and clean up `mkdtempSync` directories created by the `writeTmp`
helper inside the `loadPresetFromFile` describe block via `afterEach`.
- Add tests covering the new oversize-file (>10 MB) and
symlink-rejection paths.

## Type of Change

- [x] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Verification

- [x] `npx prek run --all-files` passes
- [x] `npm test` passes
- [x] Tests added or updated for new or changed behavior
- [x] No secrets, API keys, or credentials committed
- [ ] Docs updated for user-facing behavior changes
- [ ] `make docs` builds without warnings (doc changes only)
- [ ] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Enhanced preset file validation with strict extension checking and a
10MB file size limit.
* Improved security by rejecting symbolic links and enforcing stricter
file access and permission checks, with clearer error messages for
invalid files.

* **Tests**
* Added tests covering preset file size rejection and symbolic link
rejection scenarios.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>

Author: laitingsheng

src/lib/policies.ts

@@ -16,6 +16,8 @@ const { loadAgent } = require("./agent-defs");
 
 const PRESETS_DIR = path.join(ROOT, "nemoclaw-blueprint", "policies", "presets");
 
+const MAX_PRESET_FILE_BYTES = 10_000_000;
+
 type PresetInfo = {
   file: string;
   name: string;
@@ -617,29 +619,63 @@ function applyPreset(
  */
 function loadPresetFromFile(filePath: string): { presetName: string; content: string } | null {
   const abs = path.resolve(filePath);
-  if (!fs.existsSync(abs) || !fs.statSync(abs).isFile()) {
-    console.error(`  Preset file not found: ${filePath}`);
-    return null;
-  }
   if (!/\.ya?ml$/i.test(abs)) {
     console.error(`  Preset file must be .yaml or .yml: ${filePath}`);
     return null;
   }
-  let content: string;
-  let parsed: PolicyValue;
+  const NOFOLLOW = fs.constants.O_NOFOLLOW ?? 0;
+  let fd: number;
   try {
-    content = fs.readFileSync(abs, "utf-8");
-    parsed = YAML.parse(content);
+    fd = fs.openSync(abs, fs.constants.O_RDONLY | NOFOLLOW);
   } catch (err: unknown) {
     const code = (err as NodeJS.ErrnoException)?.code;
-    const message = err instanceof Error ? err.message : String(err);
-    const msg =
-      code === "ENOENT" || code === "EACCES"
-        ? `Cannot read ${filePath}: ${message}`
-        : `Invalid YAML in ${filePath}: ${message}`;
-    console.error(`  ${msg}`);
+    if (code === "ELOOP" || code === "EMLINK") {
+      console.error(
+        `  Preset file must not be a symbolic link: ${filePath} (resolve with 'realpath' and pass the target path).`,
+      );
+    } else if (code === "ENOENT" || code === "ENOTDIR") {
+      console.error(`  Preset file not found: ${filePath}`);
+    } else if (code === "EACCES" || code === "EPERM") {
+      const message = err instanceof Error ? err.message : String(err);
+      console.error(`  Cannot read ${filePath}: ${message}`);
+    } else {
+      const message = err instanceof Error ? err.message : String(err);
+      console.error(`  Cannot read ${filePath}: ${message}`);
+    }
     return null;
   }
+  let content: string;
+  let parsed: PolicyValue;
+  try {
+    const stat = fs.fstatSync(fd);
+    if (!stat.isFile()) {
+      console.error(`  Preset file not found: ${filePath}`);
+      return null;
+    }
+    if (stat.size > MAX_PRESET_FILE_BYTES) {
+      console.error(
+        `  Preset file too large: ${filePath} (${stat.size} bytes; max ${MAX_PRESET_FILE_BYTES} bytes).`,
+      );
+      return null;
+    }
+    try {
+      const buffer = Buffer.allocUnsafe(stat.size);
+      let offset = 0;
+      while (offset < buffer.length) {
+        const bytesRead = fs.readSync(fd, buffer, offset, buffer.length - offset, null);
+        if (bytesRead === 0) break;
+        offset += bytesRead;
+      }
+      content = buffer.toString("utf-8", 0, offset);
+      parsed = YAML.parse(content);
+    } catch (err: unknown) {
+      const message = err instanceof Error ? err.message : String(err);
+      console.error(`  Invalid YAML in ${filePath}: ${message}`);
+      return null;
+    }
+  } finally {
+    fs.closeSync(fd);
+  }
   if (!isPolicyDocument(parsed)) {
     console.error(`  Preset must be a YAML mapping: ${filePath}`);
     return null;

test/policies.test.ts

@@ -7,7 +7,7 @@ import os from "node:os";
 import path from "node:path";
 import { createRequire } from "node:module";
 import type { Interface as ReadlineInterface } from "node:readline";
-import { describe, it, expect, vi } from "vitest";
+import { afterEach, describe, it, expect, vi } from "vitest";
 import { spawnSync } from "node:child_process";
 import policies from "../dist/lib/policies";
 import { execTimeout } from "./helpers/timeouts";
@@ -1278,8 +1278,17 @@ Promise.resolve(require(${CLI_PATH}).mainPromise).finally(() => {
   });
 
   describe("loadPresetFromFile", () => {
+    const tmpDirs: string[] = [];
+    afterEach(() => {
+      while (tmpDirs.length > 0) {
+        const dir = tmpDirs.pop();
+        if (dir) fs.rmSync(dir, { recursive: true, force: true });
+      }
+    });
+
     function writeTmp(body: string, ext = "yaml") {
       const dir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-custom-preset-"));
+      tmpDirs.push(dir);
       const file = path.join(dir, `custom.${ext}`);
       fs.writeFileSync(file, body);
       return { dir, file };
@@ -1399,6 +1408,39 @@ Promise.resolve(require(${CLI_PATH}).mainPromise).finally(() => {
         errSpy.mockRestore();
       }
     });
+
+    it("rejects files exceeding the size limit before reading", () => {
+      const dir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-custom-preset-"));
+      tmpDirs.push(dir);
+      const file = path.join(dir, "huge.yaml");
+      const padding = "# ".repeat(5_500_000);
+      fs.writeFileSync(file, `preset:\n  name: huge\nnetwork_policies:\n  r:\n    name: r\n${padding}`);
+      const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+      try {
+        expect(policies.loadPresetFromFile(file)).toBe(null);
+        const msgs = errSpy.mock.calls.map((c) => c[0]);
+        expect(msgs.some((m) => typeof m === "string" && m.includes("too large"))).toBe(true);
+      } finally {
+        errSpy.mockRestore();
+      }
+    });
+
+    it("rejects symbolic links to a preset file", () => {
+      const body = "preset:\n  name: link-target\nnetwork_policies:\n  r:\n    name: r\n";
+      const { dir, file } = writeTmp(body);
+      const linkPath = path.join(dir, "link.yaml");
+      fs.symlinkSync(file, linkPath);
+      const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+      try {
+        expect(policies.loadPresetFromFile(linkPath)).toBe(null);
+        const msgs = errSpy.mock.calls.map((c) => c[0]);
+        expect(msgs.some((m) => typeof m === "string" && m.includes("must not be a symbolic link"))).toBe(
+          true,
+        );
+      } finally {
+        errSpy.mockRestore();
+      }
+    });
   });
 
   describe("policy-add --from-file / --from-dir", () => {