fix(backup): reject pre-backup audit rows that a tab desynchronised

The pre-backup state-dir audit parses `find -printf "%y\t%p\t%l\n"` rows by
splitting on tab and reading the third field as the link target. A tab is a
legal byte in a Linux filename, so a path containing one splits the row into
extra fields: the real link target moves past the third field and is
discarded, and a fragment of the row is checked against the symlink whitelist
in its place. A symlink the audit is meant to reject is then recorded as
allowed.

Extra fields can only come from such a tab, so classify those rows as
violations. Shorter rows stay on the existing paths: the audit output is
trimmed as a whole, which removes the trailing empty `%l` of a final
non-symlink row.

Row classification moves into `classifyPreBackupAuditRow` so the field-count
check does not push `backupSandboxState` past its cognitive-complexity limit;
measured 148 before, 135 after.

Signed-off-by: Udaya Tejas <udayatejas2004@gmail.com>

Author: udsy19

src/lib/state/sandbox.ts

@@ -1307,6 +1307,38 @@ function resolveOpenClawBackupMetadata(
   };
 }
 
+/**
+ * Classify one pre-backup audit row emitted by
+ * `find -printf "%y\t%p\t%l\n"`, where `dirPrefix` is the state-dir root the
+ * row's absolute path is reported under.
+ *
+ * A tab is a legal byte in a Linux filename, so a crafted path splits the row
+ * into extra fields: the real link target moves past the third field and is
+ * discarded, and a fragment of the row is checked against the symlink
+ * whitelist in its place, so an entry the audit is meant to reject can be
+ * recorded as allowed. Extra fields can only come from such a tab, so a row
+ * that carries them is a violation rather than something to audit further.
+ * Shorter rows are ordinary: the audit output is trimmed as a whole, which
+ * removes the trailing empty `%l` of a final non-symlink row.
+ */
+function classifyPreBackupAuditRow(
+  entry: string,
+  dirPrefix: string,
+): "whitelisted" | "hardLinked" | "violation" {
+  const parts = entry.split("\t");
+  if (parts.length > 3) return "violation";
+  const type = parts[0] || "";
+  const absPath = parts[1] || entry;
+  const linkTarget = parts[2] || "";
+  const relPath = absPath.startsWith(dirPrefix) ? absPath.slice(dirPrefix.length) : absPath;
+  if (type === "l" && isAllowedStateSymlink(relPath, linkTarget)) return "whitelisted";
+  // The audit's `find` only emits regular files through its `-links +1`
+  // branch, so a reported `f` row is a hard link. Recorded, not rejected —
+  // see the rationale at the audit command (#9314).
+  if (type === "f") return "hardLinked";
+  return "violation";
+}
+
 export function backupSandboxState(sandboxName: string, options: BackupOptions = {}): BackupResult {
   const sb = registry.getSandbox(sandboxName);
   const agentName = sb?.agent || "openclaw";
@@ -1565,9 +1597,10 @@ export function backupSandboxState(sandboxName: string, options: BackupOptions =
         // dependency failed its pre-upgrade backup.
         //
         // The printf format emits "<type>\t<absPath>\t<linkTarget>" — %l is
-        // empty for non-symlinks but always present, so the field count is
-        // stable. Tab separator assumes state-dir paths don't contain tabs,
-        // matching the wider convention in this file.
+        // empty for non-symlinks but always present, so a well-formed row has
+        // three fields. A tab is a legal byte in a Linux filename, so
+        // `classifyPreBackupAuditRow` enforces that count instead of assuming
+        // it.
         // Per-dir `find` invocations are joined with `;` (not `&&`) and each
         // is tolerant of its own exit code via `|| true`. The base image bakes
         // a few state subdirs as root-owned (e.g. `extensions/<plugin>`,
@@ -1611,26 +1644,11 @@ export function backupSandboxState(sandboxName: string, options: BackupOptions =
           const hardLinked: string[] = [];
           const violations: string[] = [];
           const dirPrefix = `${dir}/`;
+          const rows = { whitelisted, hardLinked, violation: violations };
           for (const entry of allEntries) {
             // find -printf "%y\t%p\t%l\n" → "<type>\t<absPath>\t<linkTarget>"
             // (linkTarget is empty for non-symlinks).
-            const parts = entry.split("\t");
-            const type = parts[0] || "";
-            const absPath = parts[1] || entry;
-            const linkTarget = parts[2] || "";
-            const relPath = absPath.startsWith(dirPrefix)
-              ? absPath.slice(dirPrefix.length)
-              : absPath;
-            if (type === "l" && isAllowedStateSymlink(relPath, linkTarget)) {
-              whitelisted.push(entry);
-            } else if (type === "f") {
-              // The audit's `find` only emits regular files through its
-              // `-links +1` branch, so a reported `f` row is a hard link.
-              // Recorded, not rejected — see the rationale above (#9314).
-              hardLinked.push(entry);
-            } else {
-              violations.push(entry);
-            }
+            rows[classifyPreBackupAuditRow(entry, dirPrefix)].push(entry);
           }
           if (whitelisted.length > 0) {
             _log(

test/snapshot-backup-audit-hardlinks.test.ts

@@ -9,6 +9,10 @@
  * `lazy-packages`. Rejecting those aborted the whole pre-upgrade backup
  * (#9314). They are now recorded and archived; symlink and special-file
  * rejection is unchanged.
+ *
+ * The same harness covers how a row is parsed. Rows are tab-delimited, and a
+ * tab is a legal byte in a Linux filename, so a crafted path must not be able
+ * to desynchronise the split.
  */
 
 import fs from "node:fs";
@@ -200,3 +204,32 @@ describe("pre-backup audit — multiply-linked regular files (#9314)", () => {
     expect(backup.error).toContain("agent.sock");
   });
 });
+
+describe("pre-backup audit — tab-delimited row parsing", () => {
+  it("rejects a symlink row carrying a tab in its path", () => {
+    // `x<TAB>../qq/evil` is one filename an agent can create inside its own
+    // state dir. `find` emits its bytes verbatim, so the row splits into four
+    // fields: the audit reads `../qq/evil` as the link target — a shape the
+    // npm-bin whitelist accepts — and discards the real target in the fourth
+    // field, whitelisting a symlink it is meant to reject.
+    const backup = backupWithAuditOutput(
+      "l\t/sandbox/.openclaw/extensions/example-not-a-real-value-1/node_modules/.bin/x\t../qq/evil\t/etc/passwd",
+    );
+
+    expect(String(backup.error ?? "")).toMatch(/Pre-backup audit rejected/);
+    expect(String(backup.error ?? "")).toContain("node_modules/.bin/x");
+    expect(backup.success).toBe(false);
+  });
+
+  it("keeps accepting a hard-link row whose empty link target was trimmed away", () => {
+    // The audit output is trimmed as a whole, so the trailing empty %l of the
+    // last row is removed and that row carries two fields. This is the normal
+    // shape of every audit that ends on a non-symlink; it must not be read as
+    // a malformed row.
+    const backup = backupWithAuditOutput(
+      "f\t/sandbox/.openclaw/workspace/lazy-packages/edge_tts/__init__.py\t",
+    );
+
+    expect(backup.error ?? "").not.toMatch(/Pre-backup audit rejected/);
+  });
+});