🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

1. 🟡 DoS via blocking read of attacker-controlled .git/HEAD or ref file (FIFO/device)

Description

resolveCommitHash() reads .git/HEAD (and potentially the referenced refs/... file) using synchronous fs.openSync()/fs.readSync() without validating that the target is a regular file.

In an untrusted repository/worktree, an attacker can create .git/HEAD or .git/refs/... as a FIFO (named pipe) or a symlink to a blocking device (e.g., a FIFO with no writer, /dev/random). On POSIX systems:

• opening a FIFO for reading can block indefinitely until a writer connects
• reading from certain device files can also block
• because this uses *Sync APIs, the entire Node.js event loop is blocked, resulting in a denial-of-service/hang

Vulnerable code:

const safeReadFilePrefix = (filePath: string, limit = 256) => {
  const fd = fs.openSync(filePath, "r");
  try {
    const buf = Buffer.alloc(limit);
    const bytesRead = fs.readSync(fd, buf, 0, limit, 0);
    return buf.subarray(0, bytesRead).toString("utf-8");
  } finally {
    fs.closeSync(fd);
  }
};

And it is invoked on paths derived from the working directory:

• headPath = resolveGitHeadPath(options.cwd ?? process.cwd())
• safeReadFilePrefix(headPath)
• if HEAD points to a ref, safeReadFilePrefix(refPath)

The surrounding try/catch in resolveCommitHash() does not mitigate hangs because a blocked openSync()/readSync() never throws.

Recommendation

Reject non-regular files (and optionally symlinks) before opening, and/or open in non-blocking mode.

Option A: lstat and only allow regular files (and disallow symlinks):

const safeReadFilePrefix = (filePath: string, limit = 256) => {
  const st = fs.lstatSync(filePath);
  ​// Avoid symlinks, FIFOs, devices, sockets, etc.
  if (!st.isFile() || st.isSymbolicLink()) {
    throw new Error(`Refusing to read non-regular file: ${filePath}`);
  }

  const fd = fs.openSync(filePath, fs.constants.O_RDONLY);
  try {
    const buf = Buffer.alloc(limit);
    const bytesRead = fs.readSync(fd, buf, 0, limit, 0);
    return buf.subarray(0, bytesRead).toString("utf-8");
  } finally {
    fs.closeSync(fd);
  }
};

Option B: non-blocking open (still consider lstat as defense-in-depth):

const fd = fs.openSync(filePath, fs.constants.O_RDONLY | fs.constants.O_NONBLOCK);

Handle EAGAIN/empty reads by returning null/"" and treating commit as unavailable.

Also consider using async I/O to avoid blocking the event loop if commit resolution occurs on latency-sensitive paths.

Analyzed PR: #35713 at commit 7ebb3f3

_{Last updated on: 2026-03-06T03:39:55Z}

Greptile Summary

This PR appends the short commit SHA to the --version / -v / -V output by calling the existing resolveCommitHash() helper and wrapping it in a ternary. The change is well-scoped, but there are two findings:

Inconsistent fallback format — The ternary at line 114 produces OpenClaw 9.9.9-test (abc1234) when commit metadata is available, but falls back to bare 9.9.9-test (without the OpenClaw prefix) when unavailable. This inconsistency contradicts the PR's stated goal of "keep existing behavior" — the format shape differs depending on environment. Both branches should include the OpenClaw label for consistency.

Missing test coverage — Only the happy path (with commit hash) is tested. A second test case with resolveCommitHashMock.mockReturnValue(null) would document and guard the fallback behaviour, ensuring both branches of the ternary are covered.

Confidence Score: 3/5

• Safe to merge after fixing the inconsistent fallback output format and adding test coverage for the null branch.
• The logic bug in the ternary fallback means the --version output format differs depending on whether a commit hash is resolvable, which is likely unintentional and could surprise users or break tooling that parses the version string. The fix is straightforward (one-line change to include OpenClaw in the fallback), but it should be resolved before merging. Additionally, the incomplete test coverage leaves the fallback path untested.
• src/cli/program/help.ts — the fallback branch of the version format ternary (line 114). src/cli/program/help.test.ts — add a test case for the null commit hash scenario.

_{Last reviewed commit: 8c80fa3}

hey @echoVic can you add to the discussion on #34972? the codex comments are correct

Thanks! I’ll follow up in #34972 to keep the discussion consolidated — agree the Codex comments there are pointing at the right shape. I’ll align this PR accordingly (and add the missing fallback test + harden commit formatting per the review notes).

Pushed an update to address the review notes:

• Keep output shape consistent (always includes prefix; only append when available).
• Add test coverage for the no-commit fallback branch.
• Harden against crafted markers/HEAD refs (only allow , prevent path escape, bounded reads) and validate commit output as hex-only to avoid control-char/log injection.

Latest commit: b91602c.

@echoVic
can you fix up these two? no point in making it a separate PR imo

scripts/docker/install-sh-common/cli-verify.sh:30-37
scripts/docker/install-sh-e2e/run.sh:71-77

or did I miss something? #35713 (comment)

Following up here.

The review points about keeping the fallback output shape consistent, adding the no-commit test, and hardening commit-hash resolution were folded into this PR in the later update I pushed. If there are still two concrete snippets from the linked discussion that you want adjusted further, point me at the exact lines and I will patch them on this branch instead of splitting anything out.

Follow-up pushed on this branch.

This folds in the two installer/verifier call sites called out in the review so they stay semver-compatible with openclaw --version now returning OpenClaw <version> (<sha>) when commit metadata is present.

Updated:

• scripts/docker/install-sh-common/cli-verify.sh
• scripts/docker/install-sh-e2e/run.sh

Both now normalize the CLI --version output back to the semver string before comparing against npm view ... version.

Local verification:

• pnpm test src/cli/program/help.test.ts
• bash -n scripts/docker/install-sh-common/cli-verify.sh
• bash -n scripts/docker/install-sh-e2e/run.sh

🚢