Greptile Summary

This PR improves the session-store cache invalidation by adding file size as a secondary signal alongside mtimeMs, fixing a real flakiness issue in CI where writes within the same mtime granularity window returned stale cached data.

• src/config/cache-utils.ts: New getFileSizeBytes helper added correctly, mirroring getFileMtimeMs.
• src/config/sessions/store.ts: sizeBytes field added to SessionStoreCacheEntry and checked in both the cache-hit path and both write-through cache update sites — logic is correct.
• src/config/sessions.cache.test.ts: The new regression test has a subtle issue — fs.statSync is called after fs.writeFileSync, so utimesSync resets mtime to the newly-written file's mtime rather than the originally-cached mtime. On sub-second-precision filesystems (APFS, modern ext4, etc.) this means the mtime will already differ between the two writes, the cache invalidates on mtime alone, and the sizeBytes code path is never actually exercised by the test. The stat capture should happen before writing store2 to reliably reproduce the same-mtime scenario.

Confidence Score: 4/5

• The production fix in store.ts and cache-utils.ts is correct and safe to merge; the regression test needs a small fix to reliably exercise the new code path.
• The core logic change is straightforward and low-risk — adding a secondary cache invalidation signal. The only concern is the regression test, which may not reliably cover the intended scenario on high-precision filesystems due to reading stat after rather than before the file write. This doesn't affect production behaviour, only the quality of the test as a regression guard.
• src/config/sessions.cache.test.ts — the utimesSync call should use a stat captured before writing store2 to reliably force same-mtime.

_{Last reviewed commit: e6ac71c}

Landed via temp rebase onto main.

• Gate: pnpm test src/config/sessions.cache.test.ts
• Land commit: 3b7411c
• Merge commit: 11dcf96

Thanks @jalehman!

🔒 Aisle Security Analysis

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

1. 🔵 Static code scan cache can be bypassed by rewriting a file while preserving mtime and size

Description

The skill-scanner uses an in-memory cache to avoid re-scanning files. Cache validity is based only on (mtimeMs, size, maxFileBytes).

This allows stale scan results to be reused if an attacker (or fast CI/filesystems with coarse timestamp granularity) rewrites a file such that:

• The file content changes (e.g., introduces eval() / child_process.exec()), but
• The file size stays identical, and
• mtimeMs is unchanged (e.g., rewrite occurs within the same mtime granularity tick, or the attacker resets mtime using utimes).

In that case, scanFileWithCache() will return cached findings without reading the updated file, letting dangerous patterns evade detection in subsequent scans within the same long-running process.

Vulnerable code:

if (
  cached.size !== params.size ||
  cached.mtimeMs !== params.mtimeMs ||
  cached.maxFileBytes !== params.maxFileBytes
) {
  FILE_SCAN_CACHE.delete(params.filePath);
  return undefined;
}
return cached;

Recommendation

Strengthen cache invalidation so content changes cannot be hidden behind identical (mtime,size).

Recommended options (pick one):

1. Include ctimeMs in the cache key (helps against mtime spoofing via utimes on many platforms):

​// when stat-ing
const st = await fs.stat(filePath);
const cached = getCachedFileScanResult({
  filePath,
  size: st.size,
  mtimeMs: st.mtimeMs,
  ctimeMs: st.ctimeMs,
  maxFileBytes,
});

2. Content hashing for high-assurance scans (most robust):

• Store a fast hash (e.g., SHA-256/xxhash) of the last scanned content and compare it before reusing cached findings.

3. Disable caching for security/audit runs, or provide an option like { disableCache: true } for callers that need correctness over performance.

Also update tests to cover the same-size rewrite case (i.e., content change with identical size) to prevent regressions.

Analyzed PR: #32191 at commit 3b7411c

_{Last updated on: 2026-03-02T23:13:24Z}