Skip to content

docs: correct tool permission examples and read-only guidance (refs #212) - #215

Merged
ben-vargas merged 1 commit into
mainfrom
docs/tool-permissions-followup
Oct 25, 2026
Merged

ben-vargas merged 1 commit into
mainfrom
docs/tool-permissions-followup

Conversation

@ben-vargas

Copy link
Copy Markdown
Owner

Refs #212

Follow-up to #213/#214. Changes docs and examples only; nothing in src/ changes.

What changed

  • examples/mcp-filesystem.ts is now actually read-only. It previously used bypassPermissions, which approves every tool, so the allowedTools "restriction" did nothing and write_file succeeded. It now uses permissionMode: 'dontAsk', with only the three MCP read tools pre-approved. The prompt ends with a write_file probe aimed at a file inside a temporary directory created for that run.
    • The example prints "Write blocked" only when the provider's permissionDenials metadata records that exact tool call ID, there was no stream error, and no probe file exists.
    • In every other case it prints INCONCLUSIVE or FAIL and exits 1.
    • A finally block removes only the run-owned directory. v4 also adds the missing tool-error case, so denied calls show up in the output.
  • Fake tool names removed: Delete was never a Claude Code tool, and current CLIs no longer have NotebookRead. Both are gone from both guides and tool-management.ts. LS (also not in the current CLI) is replaced in the current-version docs and examples, and kept in the 0.x guide, where it was a real tool.
  • "Read-only mode" corrected: denying only Write/Edit is not read-only, because Bash can still write.
    • Current guidance is tools: ['Read', 'Grep', 'Glob'], with MCP tools restricted separately.
    • The 0.x guide uses disallowedTools: ['Write', 'Edit', 'MultiEdit', 'NotebookEdit', 'Bash'], checked against the tools in @anthropic-ai/claude-code 1.0.24.
    • In the v5 guide, tools is labeled "provider 2.3.0+ on 2.x, or 3.1.0+", and dontAsk "3.2.0+". The MCP pattern that works in every version is listing the server's write tools in disallowedTools.
  • examples/README.md: the "MCP tool allowlists" wording is corrected. CHANGELOG entry under Unreleased → Fixed. No version bump.

Example runs

Run from a scratch cwd with CLAUDE_CODE_DISABLE_AUTO_MEMORY=1 set through the provider env, using the /private/tmp realpath as the target.

  • Before: mcp__filesystem__write_file returned a result, and the probe file existed after the run.
  • After: list_allowed_directories, list_directory and read_text_file returned the file contents. write_file failed with "Permission to use mcp__filesystem__write_file has been denied because Claude Code is running in don't ask mode". The run ended with "Write blocked: the permission system denied write_file, and no probe file was created", exited 0, and left no directory behind.
  • tool-management.ts also ran cleanly.

Commands run

npm run lint, npm run typecheck, npm test (1390 tests passed), npm run format:check, npm run build and git diff --check all passed.

Independent review (Codex gpt-6-astra, xhigh)

  • Round 1, CHANGES REQUESTED: dontAsk was recommended in the 1.x–2.x guide without a version label; the probe passed on file absence alone; cleanup was skipped when the stream threw.
  • Round 2, CHANGES REQUESTED: cleanup could delete a pre-existing file; an undefined stream-error payload produced a false pass; "2.3.0+" wrongly included 3.0.x.
  • Round 3, SIGN-OFF on both lines. One non-blocking nit, left for later: document that the probe needs a writable target directory.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-25T07:33:47.887090Z 94de5f9 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Claude finished @ben-vargas's task in 15s —— View job


PR Review: #215 docs: correct tool permission examples and read-only guidance

Docs and examples only, with nothing in src/ changed. I reviewed the diff for examples/ and CHANGELOG.md. I did not run the code or re-read the two guide diffs, and I did not check the 0.x disallowedTools claim against @anthropic-ai/claude-code 1.0.24.

Overall: looks good. The bypassPermissions → dontAsk fix is correct, and the probe logic is stricter than the earlier absence-only check.

examples/mcp-filesystem.ts

  • Good: "Write blocked" is printed only when the provider's permissionDenials metadata records the exact probe call ID. A missing probe file is no longer treated as proof.
  • Good: The finally block removes only the mkdtemp directory this run created, so it can't delete a user's file. It runs even if the stream throws.
  • Good: The isWriteTo helper tolerates string and object inputs. deniedToolCallIds handles missing metadata defensively.
  • Nit: probeWritten = existsSync(probeFile) is checked in finally, but sawStreamError is only handled afterward. If the stream throws, the finally still runs, and main().catch then reports the error. That is fine. A comment saying the failure path is covered by main().catch would help readers.
  • Nit (already noted in the PR): mkdtempSync(join(targetDir, ...)) fails if targetDir isn't writable. Consider a clearer error message or a short note in the README that the example needs a writable target directory.
  • Nit: finish-step is the only place denials are collected. If the provider ever reports permissionDenials only on finish, the example would print INCONCLUSIVE. Reading both events would be more robust, if the metadata can appear on finish.
  • Nit: The tool-error case uses console.error, while tool-result errors use console.log. That is inconsistent but harmless.

examples/tool-management.ts

  • The renaming from readOnlyClaude to safeClaude, and the corrected log lines, match the new guidance.
  • Replacing LS with Glob and dropping Delete and NotebookRead is consistent with current CLI tools.

Docs and CHANGELOG

  • The CHANGELOG entry is accurate and sits under Unreleased → Fixed.
  • examples/README.md now describes the dontAsk plus allowedTools pairing correctly.
  • The version labels (tools on "2.3.0+ on 2.x, or 3.1.0+", dontAsk on "3.2.0+") are in the guide diffs I didn't read, so I can't confirm them here.

Tests and security

  • No src/ changes, so there are no unit tests to add. The example exits 1 on FAIL or INCONCLUSIVE, which makes it usable as a smoke test.
  • Security: the example narrows what the model can do and avoids bypassPermissions. Cleanup is scoped to a run-owned temporary directory.

No blocking issues from me.

@ben-vargas
ben-vargas merged commit e796690 into main Oct 25, 2026
3 checks passed
@ben-vargas
ben-vargas deleted the docs/tool-permissions-followup branch October 25, 2026 09:00
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.

1 participant