doctor: warn about orphaned agent dirs#65113
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5e0644fe4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Greptile SummaryAdds orphaned-agent-directory detection to Confidence Score: 5/5Safe to merge; the change is read-only, well-tested, and correctly scoped to the doctor state-integrity pass. No P0 or P1 issues found. The only finding is a P2 style note about using the normalized ID for both the configuredIds lookup and the existsDir path check — a false-negative edge case that cannot occur with real OpenClaw data because agent directories are always created with already-normalized names. No files require special attention. Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/commands/doctor-state-integrity.ts
Line: 75-80
Comment:
**Normalized name used for path lookup**
`normalizeAgentId(entry.name)` can transform the raw directory name (e.g. lowercasing or replacing `_` with `-`), and then `existsDir` is called with the transformed ID as a path segment. On a case-sensitive filesystem this means a directory named `BigBrain` would be mapped to `bigbrain`, and the `agent/` check at `agentsRoot/bigbrain/agent` would silently fail even though `agentsRoot/BigBrain/agent` exists — producing a false negative.
Safer to preserve the original name for path construction and normalize only for the `configuredIds` comparison:
```suggestion
.map((entry) => ({ raw: entry.name, normalized: normalizeAgentId(entry.name) }))
.filter(({ raw, normalized: agentId }) => {
if (!agentId || configuredIds.has(agentId)) {
return false;
}
return existsDir(path.join(agentsRoot, raw, "agent"));
})
.map(({ normalized }) => normalized)
.toSorted((left, right) => left.localeCompare(right));
```
In practice OpenClaw creates all agent directories with already-normalized (lowercase) IDs, so this is a false-negative edge case rather than a present bug — but it's worth making the intent explicit.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "docs(changelog): note orphaned agent war..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Addressed the review feedback about path probing for orphaned agent dirs. What changed:
Why this changed:
Verification:
The follow-up is in commit |
🔒 Aisle Security AnalysisWe found 2 potential security issue(s) in this PR:
1. 🟡 Symlink-following realpath/stat on agent directories can cause local filesystem probing and DoS
DescriptionThe new orphan-agent-directory detection follows symlinks inside the state directory when checking
then running Vulnerable code: function existsDir(dir: string): boolean {
return fs.existsSync(dir) && fs.statSync(dir).isDirectory();
}
...
const rawRealPath = fs.realpathSync.native(rawDir);RecommendationAvoid following symlinks when scanning the state directory, or strictly confine resolution to inside Option A (simplest): skip symlinks function existsRealDirNoSymlink(p: string): boolean {
try {
const st = fs.lstatSync(p);
return st.isDirectory(); // symlink -> false
} catch {
return false;
}
}
// use existsRealDirNoSymlink(path.join(agentsRoot, dirName, "agent"))Option B: allow symlinks but confine const agentsRootReal = fs.realpathSync.native(agentsRoot);
const candidateReal = fs.realpathSync.native(candidate);
if (!candidateReal.startsWith(agentsRootReal + path.sep)) {
return false; // points outside state dir
}Also consider using async filesystem APIs to reduce CLI blocking and adding defensive limits (max entries processed) to prevent excessive work on attacker-controlled directory structures. 2. 🟡 Terminal/log injection via unsanitized agent directory names in `doctor` warning output
Description
If an attacker can create a directory under Vulnerable code: function formatOrphanAgentDirLabel(entry: OrphanAgentDir): string {
return entry.dirName === entry.agentId ? entry.agentId : `${entry.dirName} (id ${entry.agentId})`;
}RecommendationSanitize untrusted filesystem-derived strings before writing them to the terminal. Options (pick one consistent approach used across the CLI):
function sanitizeForTerminal(value: string): string {
// Remove C0 controls + DEL (incl. \n, \r, \t, ESC)
return value.replace(/[\u0000-\u001F\u007F]/g, "?");
}
function formatOrphanAgentDirLabel(entry: OrphanAgentDir): string {
const safeDir = sanitizeForTerminal(entry.dirName);
return safeDir === entry.agentId ? entry.agentId : `${safeDir} (id ${entry.agentId})`;
}
Also consider using Analyzed PR: #65113 at commit Last updated on: 2026-04-12T04:04:15Z |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c5e3ff01e0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Addressed the latest review comment in . The orphan-agent check now treats a case- or normalization-mismatched on-disk folder as configured only when the normalized configured path resolves to the same real directory. That means we now warn for truly unreachable folders like on case-sensitive filesystems, while avoiding false positives on case-insensitive filesystems where and resolve to the same directory. I also updated the examples text to show the on-disk folder name when it differs from the normalized agent id, and added focused tests for both the unreachable and same-realpath cases. Verification:
RUN v4.1.2 /Users/neeravmakwana/Desktop/github_repos/openclaw_repo_prrefresh Test Files 1 passed (1) |
obviyus
force-pushed
the
fix-doctor-orphan-agent-dirs
branch
from
26e94d0 to
58a0fdb
Compare
obviyus
left a comment
obviyus
left a comment
There was a problem hiding this comment.
Verified the doctor gap where on-disk ~/.openclaw/agents/<id>/agent directories could survive config loss while noteStateIntegrity() stayed silent, and confirmed this change adds the warning on the direct state-integrity path.
Maintainer follow-up: rebased onto latest main, fixed the changelog entry into the current Unreleased Fixes block with (#65113) Thanks @neeravmakwana, and polished the warning text/pluralization.
Local gate: pnpm test src/commands/doctor-state-integrity.test.ts.
|
Landed on main. Thanks @neeravmakwana. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58a0fdb998
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!configuredIds.has(agentId)) { | ||
| return true; |
There was a problem hiding this comment.
The orphan-dir filter currently treats any agents/<folder>/agent directory as orphan when <folder> is not a configured agent ID, but runtime agent storage can be redirected with agents.list[].agentDir (via resolveAgentDir). In that common override case (for example, agent id main using ~/.openclaw/agents/work/agent), work is a live configured directory yet this branch always flags it as orphan, leading to a misleading doctor warning and incorrect cleanup guidance.
Useful? React with 👍 / 👎.
…akwana) * doctor: warn about orphaned agent dirs * docs(changelog): note orphaned agent warning * doctor: preserve orphan agent dir casing * doctor: flag unreachable agent dirs * fix: polish orphan agent dir warning --------- Co-authored-by: Ayaan Zaidi <hi@obviy.us>
…akwana) * doctor: warn about orphaned agent dirs * docs(changelog): note orphaned agent warning * doctor: preserve orphan agent dir casing * doctor: flag unreachable agent dirs * fix: polish orphan agent dir warning --------- Co-authored-by: Ayaan Zaidi <hi@obviy.us>
2 participants
Summary
openclaw doctordid not warn when~/.openclaw/agents/<id>/agentdirectories still existed on disk but the matchingagents.list[]entries were missing from config.agents.list[]entries.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
agents.list[]entries.Regression Test Plan (if applicable)
src/commands/doctor-state-integrity.test.ts~/.openclaw/agents/<id>/agentdirectories, but ignores configured agents and incomplete folders.User-visible / Behavior Changes
openclaw doctornow warns when agent state directories exist on disk without matchingagents.list[]entries in config.Diagram (if applicable)
Security Impact (required)
Yes, explain risk + mitigation:Repro + Verification
Environment
agents.list: [{ id: "main", default: true }]plus extra on-disk agent dirs under~/.openclaw/agents/Steps
~/.openclaw/agents/big-brain/agentand~/.openclaw/agents/cerebro/agentin a temp state dir.noteStateIntegrity()with config that only definesmaininagents.list.Expected
agents.list[]entries.Actual
Evidence
Human Verification (required)
pnpm check.~/.openclaw/agents/<id>folders without nestedagent/do not warn.pnpm buildis currently failing in the existing tree due to an unrelatedextensions/acpx/src/runtime.tsexport-resolution error.Review Conversations
Compatibility / Migration
Risks and Mitigations
~/.openclaw/agents/could be mistaken for orphaned agents.agent/runtime dir, and tests cover the incomplete-folder false-positive case.AI assistance: prepared with an agent, with the code path, tests, and verification reviewed before submission.
Made with Cursor