opencode icon indicating copy to clipboard operation
opencode copied to clipboard

feat: glob pattern-based external_directory permission

Open fpdy opened this issue 4 weeks ago • 4 comments

Summary

Fixes #5395 - Extends the external_directory permission with:

  1. Read/write split: Separate read and write permissions for external directory access
  2. Glob pattern-based rules: Fine-grained path-based permission control using glob patterns

Configuration Syntax

Simple permission (backward compatible)

{
  "permission": {
    "external_directory": "allow"
  }
}

Read/write split

{
  "permission": {
    "external_directory": {
      "read": "allow",
      "write": "deny"
    }
  }
}

Glob pattern-based

{
  "permission": {
    "external_directory": {
      "read": {
        "~/reference/**": "allow",
        "~/.ssh/**": "deny",
        "*": "ask"
      },
      "write": {
        "/tmp/**": "allow",
        "*": "deny"
      }
    }
  }
}

Note on pattern evaluation:

  • Patterns are evaluated in insertion order; first match wins
  • * serves as catch-all default (evaluated last)
  • ~ expands to home directory

Tool Behavior

Tool Permission Used
read read
write write
edit write
patch write
bash write

Note on bash tool

Uses write permission conservatively since bash commands can potentially modify files.

Tests

  • Added config schema tests for glob pattern permissions
  • Added tests for split permission in bash.test.ts
  • Created read.test.ts with tests for external directory read permissions

Appendix: Mermaid Flowchart on pattern evaluation

flowchart TD
    A[Receive file path] --> B{Is permission a string?}
    B -->|Yes| C[Return that value]
    B -->|No| D{Is read/write a string?}
    D -->|Yes| E[Return that value]
    D -->|No| F[Iterate through pattern map]
    F --> G{Is current pattern '*'?}
    G -->|Yes| H[Skip and continue to next]
    G -->|No| I{Does pattern match?}
    I -->|Yes| J[Return that value]
    I -->|No| H
    H --> K{More patterns remaining?}
    K -->|Yes| G
    K -->|No| L{Does map have '*' key?}
    L -->|Yes| M["Return map['*']"]
    L -->|No| N[Return undefined = evaluated as allowed]

fpdy avatar Dec 20 '25 14:12 fpdy

Great to see PR #5841 addressing the core read/write split! We've been working on a related design (not yet posted) that extends this further with pattern-based path permissions (like "~/reference/**": "allow") and TUI folder-level approvals. Happy to share proposed design doc if useful as a follow-on enhancement.

Feature PR #5841 Our Pending Proposal
Read/write split
Pattern-based paths
Per-folder config
TUI folder approval
Discovery tools (list/glob/grep)
Session approval leakage fix
{
  "external_directory": {
    "read": {
      "~/reference/**": "allow",
      "~/.ssh/**": "deny",
      "*": "ask"
    },
    "write": {
      "/tmp/**": "allow",
      "*": "deny"
    }
  }
}

3leapsdave avatar Dec 20 '25 20:12 3leapsdave

@3leapsdave Great to see too ;) Thanks for sharing diagram too, I'll try to implement with given (glob-based)syntax for even further granularity

fpdy avatar Dec 20 '25 20:12 fpdy

@fpdy (and maintainers) — I’ve posted the full design write-up as a gist for easier reference:

https://gist.github.com/3leapsdave/5b7cf0980af70b8b07af968bac4b4371

It consolidates a few follow-up hardening items that seem relevant to the current implementation:

  • Fail-secure default (ask vs implicit allow) when no pattern matches
  • Split permission prompt types (at least read vs write) to avoid session “Always” leakage
  • Ensure coverage includes discovery tools (list/glob/grep)
  • Path normalization / best-effort realpath to reduce traversal/symlink edge cases
  • OpenAPI/SDK schema includes the pattern-map union (so plugins don’t need type assertions)

Happy to help with follow-on commits or a follow-up PR for any of the above — just point me at what you’d like tackled first.

@fpdy (and maintainers) — I’ve posted the full design write-up as a gist for easier reference: https://gist.github.com/3leapsdave/5b7cf0980af70b8b07af968bac4b4371 It consolidates a few follow-up hardening items that seem relevant to the current implementation.

Caveat: I’m not fully sure what conventions/precedence rules the maintainers ultimately want here (first-match vs most-specific, etc), so take these as “things that felt important from a safety/UX perspective” rather than prescriptions.

In particular, one thing that stood out to me: when a pattern-map is used and there’s no match (and no * catch-all), the helper can return undefined, which the tools appear to treat as implicitly allowed. My bias would be to make that fail-secure (ask by default), but totally defer to maintainers on the exact semantics.

Other follow-ups captured in the gist:

  • Split permission prompt types (at least read vs write) to avoid session “Always” leakage
  • Ensure coverage includes discovery tools (list/glob/grep)
  • Path normalization / best-effort realpath to reduce traversal/symlink edge cases
  • OpenAPI/SDK schema includes the pattern-map union (so plugins don’t need type assertions)

Happy to help with follow-on commits or a follow-up PR for any of the above — just point me at what you’d like tackled first.

3leapsdave avatar Dec 23 '25 13:12 3leapsdave

@3leapsdave Thanks again for yr kindness, your point (about the implicit behavior when there’s no catch-all) is exactly what I was concerned about too. Personally, I also think returning ask would be better and makes more sense.

That said, the current behavior (allow when no permission is specified on config) was chosen mainly for backward compatibility with existing setups. But as you said, I’m totally fine leaving the final decision to the maintainers too(cf. #5395 ).

fpdy avatar Dec 23 '25 19:12 fpdy