Skip to content

fix(capability-index): a non-file <name>.md is one unknown capability, not a dead index (PHARN-08) - #202

Merged
PrzemekGalarowicz merged 2 commits into
mainfrom
claude/bold-archimedes-5czyyn
Sep 24, 2026
Merged

PrzemekGalarowicz merged 2 commits into
mainfrom
claude/bold-archimedes-5czyyn

Conversation

@PrzemekGalarowicz

Copy link
Copy Markdown
Contributor

What this changes

Each capability is expected to have a markdown file at <subtree>/<name>/<name>.md. The index parser only checked that something existed at that path with existsSync, which is also true for a directory. readFileSync then threw EISDIR. That error is not a ManifestValidationError, so the per-capability loop re-threw it instead of skipping the capability. The result: one oddly-shaped capability in upstream pharn-oss made init, add, update and status exit 1 for every released CLI at once, with an error that did not name the path. Reproduced on the upstream tree 85bdaa37 plus a newcap/newcap.md/ directory. The shape has never occurred in upstream history, so this is protection against a future break.

The parser now uses lstat and requires a regular file. A directory, a symlink or a FIFO at that path is reported like any other malformed capability: it is added to unknown with the reason "…/.md is not a regular file", and the other capabilities still parse.

  • Symlinks are never followed. A symlink could point outside the downloaded upstream copy.
  • FIFOs are never read. Reading one would block forever.
  • Real I/O errors still stop the command, as before (for example a permission error, or the downloaded copy disappearing).

Built with /pharn-dev-ship; stage artifacts are in .dev/features/capability-index-nonfile-md/. Results:

  • validate: exit 0
  • regress: no-regressions
  • verify: PASS

The first verify run failed because the new symlink test wrote a file outside its own temp directory. I fixed the test before pushing.

Type of change

  • feat — new stack option, wizard step, or command capability
  • fix — bug fix
  • docs — docs-only change
  • chore / refactor — tooling or internal restructure, no behavior change

Area(s) touched

lib/capability-index

Checklist

  • Read the existing file(s) before editing; followed the ESM .js-extension import convention.
  • Added tests for a directory, a symlink and a FIFO at the markdown path. The directory and symlink tests fail on the old code with EISDIR. The FIFO test was not run against the old code, because the read would hang.
  • Docs: none needed; LIMITS.md §3e already promises this behavior.
  • Preserved the security invariants.

Quality gates

  • npm run check passes locally (1338/1338; non-root user, node 22).
  • npm run build / npm run test:coverage (left to CI).

🤖 Generated with Claude Code

https://claude.ai/code/session_01TvcuVhk8hTeDskp5pAJhnc


Generated by Claude Code

…, not a dead index (PHARN-08)

`existsSync` is true for a DIRECTORY at `<subtree>/<name>/<name>.md`, and
`readFileSync` then threw EISDIR — not a ManifestValidationError, so the
per-capability tolerance re-threw it and one oddly-shaped upstream capability
would abort init/add/update/status in every deployed CLI at once.

The markdown path is now lstat'ed and must be a regular file: a directory, a
symlink (never followed — it could point outside the clone) or a FIFO (a read
would block forever) is reported as `unknown` like any other per-capability
shape problem. Genuine I/O errors still propagate.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvcuVhk8hTeDskp5pAJhnc
@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: f707d3ae-e147-4759-92f7-054b65f8bd75


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…ptor

CodeQL flagged the lstat-then-readFileSync pair as a file-system race. The
markdown is now opened once (O_NOFOLLOW: a symlink is ELOOP; O_NONBLOCK: a
FIFO cannot block the open), fstat'ed on that descriptor, and read from the
same descriptor — no window between the type check and the read. Same
refusals and messages as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvcuVhk8hTeDskp5pAJhnc
@PrzemekGalarowicz
PrzemekGalarowicz merged commit dd165c8 into main Sep 24, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants