Synchronous existsSync inside async filesystem walker

isInsideGitRepo uses the synchronous existsSync from node:fs and can traverse many directory levels before reaching the filesystem root. This blocks the Node.js event loop for the entire traversal. Every other FS operation in collectFilesystemFindings uses the async fsp variants. Consider using fsPromises.access in an async version of this helper so it stays consistent with the rest of the function:

import { access as fspAccess } from "node:fs/promises";

async function isInsideGitRepo(startDir: string): Promise<string | null> {
  let dir = path.resolve(startDir);
  while (true) {
    try {
      await fspAccess(path.resolve(dir, ".git"));
      return dir;
    } catch {
      // not found at this level
    }
    const parent = path.dirname(dir);
    if (parent === dir) break;
    dir = parent;
  }
  return null;
}

collectFilesystemFindings is already async, so this is a non-breaking change.

This is a comment left during a code review.
Path: src/security/audit.ts
Line: 207-220

Comment:
**Synchronous `existsSync` inside async filesystem walker**

`isInsideGitRepo` uses the synchronous `existsSync` from `node:fs` and can traverse many directory levels before reaching the filesystem root. This blocks the Node.js event loop for the entire traversal. Every other FS operation in `collectFilesystemFindings` uses the async `fsp` variants. Consider using `fsPromises.access` in an async version of this helper so it stays consistent with the rest of the function:

```ts
import { access as fspAccess } from "node:fs/promises";

async function isInsideGitRepo(startDir: string): Promise<string | null> {
  let dir = path.resolve(startDir);
  while (true) {
    try {
      await fspAccess(path.resolve(dir, ".git"));
      return dir;
    } catch {
      // not found at this level
    }
    const parent = path.dirname(dir);
    if (parent === dir) break;
    dir = parent;
  }
  return null;
}
```

`collectFilesystemFindings` is already `async`, so this is a non-breaking change.

How can I resolve this? If you propose a fix, please make it concise.

Read web_fetch settings from canonical tools.web.fetch path

This check reads cfg.tools.webFetch/cfg.tools.web_fetch and allowedUrls, but the validated config only exposes tools.web.fetch and is strict (src/config/zod-schema.agent-runtime.ts), so tools.web.fetch.enabled=false is ignored and users still get tools.web_fetch.no_url_allowlist. In non-loopback deployments this produces a persistent false warning with remediation pointing to unsupported keys, so the audit cannot be satisfied through valid configuration.

Useful? React with 👍 / 👎.

Avoid referencing unsupported token expiry config fields

The token-expiry warning is gated on gateway.auth.tokenExpiry/gateway.auth.tokenRotation, but gateway.auth is strict and only accepts mode, token, password, allowTailscale, rateLimit, and trustedProxy (src/config/zod-schema.ts:613-645). Any token-auth deployment will therefore always trigger this finding and the suggested remediation cannot be applied in a valid config.

Useful? React with 👍 / 👎.

Stop checking nonexistent gateway message rate-limit keys

This condition looks for gateway.rateLimit.enabled or gateway.messageRateLimit, but neither key exists in the strict gateway schema (src/config/zod-schema.ts:587-760), so non-loopback gateways will always be flagged regardless of real settings. Because the remediation tells operators to set unsupported keys, this creates an unsatisfiable audit warning.

Useful? React with 👍 / 👎.

PYTHONPATH, RUBYOPT, PERL5OPT, JAVA_TOOL_OPTIONS, and JDK_JAVA_OPTIONS are routinely injected into the process environment by ordinary language tooling — Python virtualenvs (virtualenv, conda, Poetry), JetBrains/Eclipse IDEs, Ruby bundler wrappers, etc. Treating these identically to LD_PRELOAD, LD_AUDIT, and DYLD_INSERT_LIBRARIES (which are near-universal native code-injection vectors) will produce persistent false positives.

In practice, a Python developer who opens a terminal with an active virtualenv will always get this warning for PYTHONPATH, regardless of threat exposure. The remediation text "Unset these variables unless intentionally configured" compounds the issue — following that advice for a virtualenv-managed PYTHONPATH would actively break the user's Python development environment.

Consider splitting the list into tiers and only surfacing the high-risk tier as "warn":

// Always high-risk: native shared library / linker injection
const HIGH_RISK_ENV_VARS = [
  "LD_AUDIT",
  "LD_PRELOAD",
  "LD_LIBRARY_PATH",
  "DYLD_INSERT_LIBRARIES",
  "DYLD_LIBRARY_PATH",
  "SSLKEYLOGFILE",
];

// Moderately risky but routinely set by language tooling
const MEDIUM_RISK_ENV_VARS = [
  "NODE_OPTIONS",
  "GLIBC_TUNABLES",
  "JAVA_TOOL_OPTIONS",
  "JDK_JAVA_OPTIONS",
  "PYTHONPATH",
  "PYTHONSTARTUP",
  "RUBYOPT",
  "PERL5OPT",
  "BASH_ENV",
];

Emit "warn" only for HIGH_RISK_ENV_VARS and "info" (or suppress) for MEDIUM_RISK_ENV_VARS to preserve the T-DISC-004 signal without alert-fatiguing developers on every audit run.

This is a comment left during a code review.
Path: src/security/audit.ts
Line: 1299-1315

Comment:
`PYTHONPATH`, `RUBYOPT`, `PERL5OPT`, `JAVA_TOOL_OPTIONS`, and `JDK_JAVA_OPTIONS` are routinely injected into the process environment by ordinary language tooling — Python virtualenvs (virtualenv, conda, Poetry), JetBrains/Eclipse IDEs, Ruby bundler wrappers, etc. Treating these identically to `LD_PRELOAD`, `LD_AUDIT`, and `DYLD_INSERT_LIBRARIES` (which are near-universal native code-injection vectors) will produce persistent false positives.

In practice, a Python developer who opens a terminal with an active virtualenv will **always** get this warning for `PYTHONPATH`, regardless of threat exposure. The remediation text *"Unset these variables unless intentionally configured"* compounds the issue — following that advice for a virtualenv-managed `PYTHONPATH` would actively break the user's Python development environment.

Consider splitting the list into tiers and only surfacing the high-risk tier as `"warn"`:

```ts
// Always high-risk: native shared library / linker injection
const HIGH_RISK_ENV_VARS = [
  "LD_AUDIT",
  "LD_PRELOAD",
  "LD_LIBRARY_PATH",
  "DYLD_INSERT_LIBRARIES",
  "DYLD_LIBRARY_PATH",
  "SSLKEYLOGFILE",
];

// Moderately risky but routinely set by language tooling
const MEDIUM_RISK_ENV_VARS = [
  "NODE_OPTIONS",
  "GLIBC_TUNABLES",
  "JAVA_TOOL_OPTIONS",
  "JDK_JAVA_OPTIONS",
  "PYTHONPATH",
  "PYTHONSTARTUP",
  "RUBYOPT",
  "PERL5OPT",
  "BASH_ENV",
];
```

Emit `"warn"` only for `HIGH_RISK_ENV_VARS` and `"info"` (or suppress) for `MEDIUM_RISK_ENV_VARS` to preserve the T-DISC-004 signal without alert-fatiguing developers on every audit run.

How can I resolve this? If you propose a fix, please make it concise.

NODE_OPTIONS causes false positives for Node.js developers

NODE_OPTIONS is placed in the HIGH_RISK_ENV_VARS tier (always "warn"), but it is routinely set by normal Node.js development tooling — e.g. ts-node sets NODE_OPTIONS='--loader ts-node/esm', monorepo runners set memory limits like NODE_OPTIONS='--max-old-space-size=4096', and various test harnesses use it for experimental features. The test at audit-gaps.test.ts:137 even demonstrates this exact false-positive scenario: NODE_OPTIONS: "--max-old-space-size=4096" is explicitly expected to trigger a "warn" finding.

Unlike LD_PRELOAD or BASH_ENV, whose presence alone is a strong signal, the danger from NODE_OPTIONS is value-dependent (--require/-r loads arbitrary code; --max-old-space-size does not). JAVA_TOOL_OPTIONS — which similarly enables agent injection via -javaagent: — was moved to the lower-risk LANG_TOOLING_ENV_VARS tier, so the asymmetry here seems unintentional.

Consider either moving NODE_OPTIONS to LANG_TOOLING_ENV_VARS, or applying value-based detection:

// In HIGH_RISK_ENV_VARS check, also scan NODE_OPTIONS value for injection patterns
const nodeOpts = env["NODE_OPTIONS"] ?? "";
if (/(?:^|\s)(?:-r|--require|--loader|--import)\s/i.test(nodeOpts)) {
  highRiskSet.push("NODE_OPTIONS");
} else if (nodeOpts.trim().length > 0) {
  langToolingSet.push("NODE_OPTIONS");
}

This is a comment left during a code review.
Path: src/security/audit.ts
Line: 1301

Comment:
**`NODE_OPTIONS` causes false positives for Node.js developers**

`NODE_OPTIONS` is placed in the `HIGH_RISK_ENV_VARS` tier (always `"warn"`), but it is routinely set by normal Node.js development tooling — e.g. `ts-node` sets `NODE_OPTIONS='--loader ts-node/esm'`, monorepo runners set memory limits like `NODE_OPTIONS='--max-old-space-size=4096'`, and various test harnesses use it for experimental features. The test at `audit-gaps.test.ts:137` even demonstrates this exact false-positive scenario: `NODE_OPTIONS: "--max-old-space-size=4096"` is explicitly expected to trigger a `"warn"` finding.

Unlike `LD_PRELOAD` or `BASH_ENV`, whose presence alone is a strong signal, the danger from `NODE_OPTIONS` is value-dependent (`--require`/`-r` loads arbitrary code; `--max-old-space-size` does not). `JAVA_TOOL_OPTIONS` — which similarly enables agent injection via `-javaagent:` — was moved to the lower-risk `LANG_TOOLING_ENV_VARS` tier, so the asymmetry here seems unintentional.

Consider either moving `NODE_OPTIONS` to `LANG_TOOLING_ENV_VARS`, or applying value-based detection:

```ts
// In HIGH_RISK_ENV_VARS check, also scan NODE_OPTIONS value for injection patterns
const nodeOpts = env["NODE_OPTIONS"] ?? "";
if (/(?:^|\s)(?:-r|--require|--loader|--import)\s/i.test(nodeOpts)) {
  highRiskSet.push("NODE_OPTIONS");
} else if (nodeOpts.trim().length > 0) {
  langToolingSet.push("NODE_OPTIONS");
}
```

How can I resolve this? If you propose a fix, please make it concise.

Honor tool deny policy in web_fetch allowlist audit

This check treats web_fetch as enabled whenever tools.webFetch/web_fetch is not explicitly disabled, but it never consults runtime tool policy (tools.deny, agent-specific deny lists, or group policy), even though execution gating enforces deny-first matching via isToolAllowedByPolicies/makeToolPolicyMatcher in src/agents/pi-tools.policy.ts. In a hardened config that denies web_fetch globally (for example tools.deny: ["web_fetch"] or group:web), this still emits tools.web_fetch.no_url_allowlist, creating a false positive that can’t be cleared by the actual mitigation (disabling the tool through policy).

Useful? React with 👍 / 👎.