Skip to content

feat: add allowRead support and document sandbox precedence#34

Merged
isanchez31 merged 1 commit intoisanchez31:mainfrom
clinta:allowRead
Mar 30, 2026
Merged

feat: add allowRead support and document sandbox precedence#34
isanchez31 merged 1 commit intoisanchez31:mainfrom
clinta:allowRead

Conversation

@clinta
Copy link
Copy Markdown
Contributor

@clinta clinta commented Mar 20, 2026

Summary

  • Add filesystem.allowRead passthrough in plugin config so users can re-allow specific read paths while keeping broad deny rules.
  • Extend config tests to validate default allowRead behavior and user-provided passthrough values.
  • Document read/write precedence and add examples for SSH public-key access used in signed git commit workflows, including env-var usage.

Testing

  • bun test test/config.test.ts

Summary by CodeRabbit

  • New Features

    • Added support for explicitly allowing specific filesystem read paths to override default restrictions.
  • Documentation

    • Expanded configuration documentation with new examples and path precedence rules explaining how allow and deny permissions interact for filesystem read and write operations.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 20, 2026

📝 Walkthrough

Walkthrough

The changes introduce filesystem allowRead configuration support to the sandbox. Documentation is updated to explain the new feature and path precedence rules, the config interface is extended with an optional allowRead field that defaults to an empty array, and tests verify the configuration loading behavior across JSON config and environment variable sources.

Changes

Cohort / File(s) Summary
Documentation
README.md
Added filesystem allowRead section explaining default behavior, path precedence rules stating allowRead overrides denyRead for reads, and examples for both JSON and environment-variable configuration formats.
Configuration
src/config.ts
Extended SandboxPluginConfig.filesystem interface with optional allowRead?: string[] field and updated resolveConfig to populate it from user config or default to empty array.
Tests
test/config.test.ts
Added assertions for filesystem.allowRead field across "no user config," "user filesystem config overrides," "partial user config," and environment-variable loading test cases.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A hoppy new field has come to play,
allowRead hops in to save the day!
Precedence rules, clear as morning dew,
Defaults and configs—all shiny and new! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: adding allowRead support to the filesystem configuration and documenting sandbox precedence rules.
Description check ✅ Passed The description covers the main changes and testing but omits the 'Changes' and 'Related Issues' sections from the template, though core information is present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

Tip

You can disable the changed files summary in the walkthrough.

Disable the reviews.changed_files_summary setting to disable the changed files summary in the walkthrough.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@README.md`:
- Around line 182-202: The README example currently hard-codes the full default
denyRead array which duplicates logic defined in src/config.ts (defaults applied
when denyRead is omitted); edit the README snippet to remove the denyRead key
and show only the allowRead entry so the example relies on the code's default
behavior (refer to the default handling in src/config.ts) and add a short note
that defaults are applied when denyRead is omitted to avoid drift.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 262a5721-01e2-48a4-bb7f-25950a09abdb

📥 Commits

Reviewing files that changed from the base of the PR and between 3bc2084 and 473af91.

📒 Files selected for processing (3)
  • README.md
  • src/config.ts
  • test/config.test.ts

Comment thread README.md
Comment on lines +182 to +202
```json
// ~/.config/opencode-sandbox/config.json
{
"filesystem": {
"denyRead": [
"~/.ssh",
"~/.gnupg",
"~/.aws/credentials",
"~/.azure",
"~/.config/gcloud",
"~/.config/gh",
"~/.kube",
"~/.docker/config.json",
"~/.npmrc",
"~/.netrc",
"~/.env"
],
"allowRead": ["~/.ssh/id_ed25519.pub"]
}
}
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Avoid hard-coding the full default denyRead list in examples.

This list is duplicated from code and can drift. Since defaults are applied when denyRead is omitted (src/config.ts, Line 93), the example can show only allowRead to stay durable.

Proposed README simplification
 {
   "filesystem": {
-    "denyRead": [
-      "~/.ssh",
-      "~/.gnupg",
-      "~/.aws/credentials",
-      "~/.azure",
-      "~/.config/gcloud",
-      "~/.config/gh",
-      "~/.kube",
-      "~/.docker/config.json",
-      "~/.npmrc",
-      "~/.netrc",
-      "~/.env"
-    ],
     "allowRead": ["~/.ssh/id_ed25519.pub"]
   }
 }
-OPENCODE_SANDBOX_CONFIG='{"filesystem":{"denyRead":["~/.ssh","~/.gnupg","~/.aws/credentials","~/.azure","~/.config/gcloud","~/.config/gh","~/.kube","~/.docker/config.json","~/.npmrc","~/.netrc","~/.env"],"allowRead":["~/.ssh/id_ed25519.pub"]}}' opencode
+OPENCODE_SANDBOX_CONFIG='{"filesystem":{"allowRead":["~/.ssh/id_ed25519.pub"]}}' opencode

Also applies to: 224-225

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 182 - 202, The README example currently hard-codes
the full default denyRead array which duplicates logic defined in src/config.ts
(defaults applied when denyRead is omitted); edit the README snippet to remove
the denyRead key and show only the allowRead entry so the example relies on the
code's default behavior (refer to the default handling in src/config.ts) and add
a short note that defaults are applied when denyRead is omitted to avoid drift.

@isanchez31 isanchez31 merged commit fd9c0b8 into isanchez31:main Mar 30, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants