diff --git a/ISSUE.md b/ISSUE.md new file mode 100644 index 00000000000..5b49efcb0cf --- /dev/null +++ b/ISSUE.md @@ -0,0 +1,72 @@ +# `external_directory` permission patterns don't support relative paths + +## Description + +The `external_directory` permission does not work with relative path patterns, unlike other permissions like `edit`. This is inconsistent with the documented behavior and prevents users from configuring granular permissions for sibling directories. + +## Steps to Reproduce + +1. Create an `opencode.json` config with a relative path pattern for `external_directory`: + +```json +{ + "$schema": "https://opencode.ai/config.json", + "permission": { + "external_directory": { + "*": "ask", + "../sibling-project-*": "allow" + } + } +} +``` + +2. Attempt to access a file in a sibling directory matching the pattern (e.g., `../sibling-project-foo/file.txt`) + +3. OpenCode still prompts for permission instead of allowing access + +## Expected Behavior + +Based on the [permissions documentation](https://opencode.ai/docs/permissions/#granular-rules-object-syntax), relative path patterns should work for `external_directory` the same way they work for `edit`: + +```json +{ + "permission": { + "edit": { + "*": "deny", + "packages/web/src/content/docs/*.mdx": "allow" + } + } +} +``` + +Users would reasonably expect that `../sibling-project-*` would match sibling directories. + +## Actual Behavior + +The `external_directory` permission uses absolute paths internally (e.g., `/Users/foo/sibling-project-foo/*`), so a relative pattern like `../sibling-project-*` never matches. + +## Root Cause + +In `src/tool/external-directory.ts`, the `assertExternalDirectory` function constructs permission patterns using the absolute path: + +```typescript +const parentDir = kind === "directory" ? target : path.dirname(target) +const glob = path.join(parentDir, "*") // This is an absolute path +``` + +The `edit` permission, by contrast, uses relative paths: + +```typescript +patterns: [path.relative(Instance.worktree, filePath)] +``` + +## Impact + +- Users cannot pre-configure access to external directories using intuitive relative paths +- The behavior is inconsistent with other permission types +- Workaround requires knowing and specifying full absolute paths, which are machine-specific + +## Environment + +- OpenCode version: 1.x +- OS: All platforms diff --git a/PR.md b/PR.md new file mode 100644 index 00000000000..5fc6b6a2f03 --- /dev/null +++ b/PR.md @@ -0,0 +1,67 @@ +# fix: use relative paths for external_directory permission patterns + +## Summary + +This PR fixes the `external_directory` permission to use relative paths instead of absolute paths, making it consistent with other permissions like `edit` and allowing users to configure patterns like `../sibling-project-*`. + +## Problem + +The `external_directory` permission was using absolute paths internally, which meant user-configured relative path patterns would never match. For example: + +```json +{ + "permission": { + "external_directory": { + "*": "ask", + "../sibling-project-*": "allow" + } + } +} +``` + +This config would not work because when accessing `/Users/foo/sibling-project-bar/file.txt` from `/Users/foo/myproject`, the internal pattern would be `/Users/foo/sibling-project-bar/*`, which doesn't match `../sibling-project-*`. + +## Solution + +Changed `assertExternalDirectory` to compute relative paths using `path.relative(Instance.directory, parentDir)`: + +```typescript +// Before +const parentDir = kind === "directory" ? target : path.dirname(target) +const glob = path.join(parentDir, "*") + +// After +const parentDir = kind === "directory" ? target : path.dirname(target) +const relativeParentDir = path.relative(Instance.directory, parentDir) +const glob = path.join(relativeParentDir, "*") +``` + +Now when accessing `/Users/foo/sibling-project-bar/file.txt` from `/Users/foo/myproject`, the pattern becomes `../sibling-project-bar/*`, which correctly matches `../sibling-project-*`. + +## Rationale + +1. **Consistency**: The `edit` permission already uses relative paths (`path.relative(Instance.worktree, filePath)`), so `external_directory` should behave the same way + +2. **Principle of least surprise**: The documentation shows relative path examples, and users reasonably expect them to work + +3. **Portability**: Relative paths work across machines, while absolute paths are machine-specific + +4. **Minimal change**: The fix is a single-line addition that converts the path before building the glob pattern + +## Changes + +- `src/tool/external-directory.ts`: Use `path.relative()` to compute relative path patterns +- `test/tool/external-directory.test.ts`: Updated test expectations for relative paths +- `test/tool/read.test.ts`: Updated test to check for relative path format + +## Testing + +All existing tests pass with updated expectations: +- `test/tool/external-directory.test.ts` - 5 tests +- `test/tool/read.test.ts` - 26 tests +- `test/tool/bash.test.ts` - 12 tests (unchanged) +- `test/agent/agent.test.ts` - 34 tests + +## Note + +The `bash` tool has separate `external_directory` logic that still uses absolute paths. This can be addressed in a follow-up PR to keep this change tightly scoped. diff --git a/packages/opencode/src/tool/external-directory.ts b/packages/opencode/src/tool/external-directory.ts index 1d3958fc464..74c1fe67904 100644 --- a/packages/opencode/src/tool/external-directory.ts +++ b/packages/opencode/src/tool/external-directory.ts @@ -18,7 +18,8 @@ export async function assertExternalDirectory(ctx: Tool.Context, target?: string const kind = options?.kind ?? "file" const parentDir = kind === "directory" ? target : path.dirname(target) - const glob = path.join(parentDir, "*") + const relativeParentDir = path.relative(Instance.directory, parentDir) + const glob = path.join(relativeParentDir, "*") await ctx.ask({ permission: "external_directory", @@ -26,7 +27,7 @@ export async function assertExternalDirectory(ctx: Tool.Context, target?: string always: [glob], metadata: { filepath: target, - parentDir, + parentDir: relativeParentDir, }, }) } diff --git a/packages/opencode/test/tool/external-directory.test.ts b/packages/opencode/test/tool/external-directory.test.ts index b21f6a9715c..0e87bf54ea4 100644 --- a/packages/opencode/test/tool/external-directory.test.ts +++ b/packages/opencode/test/tool/external-directory.test.ts @@ -64,7 +64,7 @@ describe("tool.assertExternalDirectory", () => { const directory = "/tmp/project" const target = "/tmp/outside/file.txt" - const expected = path.join(path.dirname(target), "*") + const expected = path.join(path.relative(directory, path.dirname(target)), "*") await Instance.provide({ directory, @@ -90,7 +90,7 @@ describe("tool.assertExternalDirectory", () => { const directory = "/tmp/project" const target = "/tmp/outside" - const expected = path.join(target, "*") + const expected = path.join(path.relative(directory, target), "*") await Instance.provide({ directory, diff --git a/packages/opencode/test/tool/read.test.ts b/packages/opencode/test/tool/read.test.ts index 7250bd2fd1e..26900861030 100644 --- a/packages/opencode/test/tool/read.test.ts +++ b/packages/opencode/test/tool/read.test.ts @@ -72,7 +72,8 @@ describe("tool.read external_directory permission", () => { await read.execute({ filePath: path.join(outerTmp.path, "secret.txt") }, testCtx) const extDirReq = requests.find((r) => r.permission === "external_directory") expect(extDirReq).toBeDefined() - expect(extDirReq!.patterns.some((p) => p.includes(outerTmp.path))).toBe(true) + // Pattern should be a relative path like "../tmpXXX/*" + expect(extDirReq!.patterns.some((p) => p.startsWith("..") && p.endsWith("*"))).toBe(true) }, }) })