-
Notifications
You must be signed in to change notification settings - Fork 6
[Epic] Richer Permission Pre-Approval #163
Description
Overview
copilot-bridge enforces tool-use permissions through a 9-layer resolution chain anchored in matchesRule() in src/config.ts. The current engine supports six pattern forms (blanket category, first-word, git/gh two-word, prefix, MCP server, MCP tool), but has gaps: two-word matching is restricted to git/gh, there is no glob/wildcard support, permission rules are global-only with no per-workspace overrides, and there is no way to define reusable named policy groups. This epic closes those gaps across three functional phases (plus a doc-only Phase 0), all backward-compatible with every existing pattern form.
Spec Reference
Full plan, design rationale, and implementation detail:
https://github.com/raykao/dark-factory/tree/speckit/richer-permission-pre-approval/specs/richer-permission-pre-approval
Phases and Tasks
Phase 0: Documentation Fix -- Doc-Only (Closes FR-008 Immediately)
- [T001] Update
README.mdpermission docs to explicitly document first-word match behavior:shell(bd)pre-approves allbdsubcommands (bd prime,bd ready --json,bd backup export-git, etc.) via first-word match -- closes FR-008 - [T002] [P] Add clarifying JSDoc/inline comment in
matchesRule()near the first-word match block:// First-word match: shell(bd) matches "bd prime", "bd ready --json", etc. - [T003] [P] Update dark-factory
AGENTS.mdto note thatshell(bd)inconfig.jsonis sufficient for FR-008 today and that Phase 1 glob patterns are an enhancement, not a prerequisite
Phase 1: Foundational -- Glob Engine Infrastructure
- [T004] Add module-level
globCacheconstant (const globCache = new Map<string, RegExp>();) abovematchesRule()insrc/config.ts - [T005] Implement
compileGlob(pattern: string): RegExpinsrc/config.ts-- escape regex metacharacters, replace*with.*, anchor with^...$, populate and return fromglobCache - [T006] [P] Export
invalidateGlobCache(): voidfunction that callsglobCache.clear()fromsrc/config.ts - [T007] Wire
invalidateGlobCache()into every config reload code path:ConfigWatcherreload handler and whereverreadConfig()orapplyConfigDiff()is invoked
Phase 2: User Stories 1 & 2 -- Glob Wildcard Pattern Matching (MVP)
- [T008] [P] Write parametric backward-compatibility test suite in
tests/config.test.tscovering all 6 existingmatchesRule()pattern forms (11 cases: blanket, first-word, git/gh two-word, prefix, MCP server, MCP tool) -- must pass without code change - [T009] [P] Write parametric new-behavior test suite in
tests/config.test.tsfor glob wildcard cases:shell(bd *)matchesbd prime/bd ready --json, does NOT matchbdc prime;shell(git log *)matchesgit log --oneline, NOTgit diff HEAD; deny overrides allow; cache hit returns same result - [T010] Add glob match as final check in shell branch of
matchesRule():if (parsed.tool.includes('*')) { return compileGlob(parsed.tool).test((commandText ?? '').trim()); } - [T011] Run T008 and T009 test suites and confirm all backward-compat cases pass and all new glob cases pass; fix any failures before proceeding
Phase 3: User Story 3 -- Generalized Two-Word Matching
- [T012] [P] Write generalized two-word test cases in
tests/config.test.ts:shell(npm run)matchesnpm run test/npm run build --prod, NOTnpm install;shell(docker compose)matchesdocker compose up -d; existingshell(git log)andshell(gh pr)still match - [T013] Remove git/gh restriction from
shellCmdFullextraction inmatchesRule(): replaceif ((parts[0] === 'git' || parts[0] === 'gh') && parts.length > 1)withif (parts.length > 1) - [T014] Update JSDoc for
matchesRule()to document all four shell matching forms in priority order: (1) exact first-word, (2) generalized two-word, (3) prefix match, (4) glob wildcard - [T015] Run T012 test suite; confirm all generalized two-word cases pass and full backward-compat suite (T008) shows no regressions
Phase 4: User Story 4 -- Workspace-Scoped Permission Files
- [T016] [P] Write workspace permission tests in
tests/config.test.tscovering 8 scenarios: feature gate disabled; gate enabled + workspace allow; global deny wins over workspace allow; workspace deny; malformed JSON (warning + continue); missing file (no error);/rulesshows[workspace]label;/reload configre-reads file - [T017] Add
allowWorkspacePermissions?: boolean(default:false) todefaultstype definition and JSON schema validation insrc/config.ts - [T018] Implement
loadWorkspacePermissions(workingDirectory: string): PermissionsConfig | nullinsrc/config.ts: check feature gate, resolve<workingDirectory>/.github/permissions.json, parse JSON, returnnullwithlogger.warnon any error - [T019] Modify
evaluateConfigPermissions()signature to accept optionalworkingDirectory?: stringparameter - [T020] Implement union deny-wins merge semantics in
evaluateConfigPermissions():effectiveDeny = [...globalPerms.deny, ...wsPerms?.deny],effectiveAllow = [...globalPerms.allow, ...wsPerms?.allow] - [T021] [P] Extend
evaluateConfigPermissions()to unionallowPathsandallowUrlsfrom workspace file with global config - [T022] Update
handlePermissionRequest()insrc/core/session-manager.tsto pass channel'sworkingDirectorytoevaluateConfigPermissions() - [T023] Add
[workspace]source label to/rulesoutput for rules sourced from.github/permissions.json - [T024] Wire workspace permission file re-read into
/reload configcode path - [T025] Run T016 test suite; confirm all 8 workspace scenarios pass and Phase 1 backward-compat suite shows no regressions
Phase 5: User Story 5 -- Named Policy Sets
- [T026] [P] Write policy expansion tests in
tests/config.test.tscovering 9 scenarios: built-in resolution; built-in non-match; user-defined policy; policy composition; cycle A->B; self-reference; built-in override warning;/rulesexpansion; unknown policy - [T027] Define
BUILT_IN_POLICIESconstant insrc/config.tswith 5 sets:safe-git-reads,safe-reads,bd-all,gh-pr-reads,gh-issue-reads - [T028] Add
policies?: Record<string, { allow?: string[]; deny?: string[] }>field topermissionsschema type definition insrc/config.ts - [T029] Extend
parsePermissionSpec()to handlepolicy:prefix: return parsed spec withkind: 'policy'and extracted name - [T030] Implement
expandPolicies(rules, allPolicies, visited?)insrc/config.tswith DAG traversal and cycle detection; throw with cycle path message on cycle; throw on unknown policy name - [T031] Add built-in override warning at config load when user-defined policy name collides with a
BUILT_IN_POLICIESkey - [T032] Integrate
expandPolicies()call intoevaluateConfigPermissions()to expand both effective allow and deny arrays at evaluation time - [T033] Update
/ruleshandler to callexpandPolicies()and display expanded members when a rule is apolicy:reference - [T034] Run T026 test suite; confirm all 9 policy scenarios pass and Phases 1-3 backward-compat suites show no regressions
Phase 6: Polish & Cross-Cutting Concerns
- [T035] [P] Add broad-glob warning at config load in
src/config.ts: emitlogger.warnfor dangerously broad patterns such asshell(*)orshell(* *) - [T036] [P] Backward-compatibility verification: run complete T008 parametric suite against final implementation; assert zero behavior changes; document results in
tests/config.test.ts - [T037] [P] Performance smoke test: assert
matchesRule()evaluates 100 glob rules in < 10 ms total; per-rule match with cached regex < 0.1 ms
Acceptance Criteria
Phase 0:
README.mddocuments first-word match behavior clearly- JSDoc in
matchesRule()notes thatshell(bd)matches allbdsubcommands - No functional changes; no test changes required
Phase 1 (Glob Engine - MVP):
matchesRule()accepts glob*wildcard in shell patterns- Two-word matching works for any command prefix (not just git/gh)
- Glob regex is compiled once and cached per pattern string; cache invalidates on config reload
- All 6 existing pattern forms pass backward-compat test suite (11 cases, zero regression)
shell(bd *),shell(git log *),shell(npm run *),shell(docker compose *)patterns work- No measurable latency regression (each match < 0.1 ms for cached regex)
Phase 2 (Workspace Scoping):
loadWorkspacePermissions()reads.github/permissions.jsonwhenallowWorkspacePermissions: true- Workspace deny rules union with global deny rules; global denies cannot be overridden by workspace allows
- Malformed or missing workspace file logs warning and falls back gracefully
allowWorkspacePermissionsdefaults tofalse/ruleslabels workspace-sourced rules as[workspace]/reload configre-reads workspace permission files
Phase 3 (Named Policy Sets):
permissions.policiesinconfig.jsondefines named policy sets- Built-in sets (
safe-git-reads,safe-reads,bd-all,gh-pr-reads,gh-issue-reads) ship with copilot-bridge policy:namereferences are valid in any allow/deny list in config or workspace files- Policy expansion happens at evaluation time (supports hot-reload without restart)
- Circular references produce a fatal config error identifying the full cycle path
/rulesshows expanded members of referenced policy sets
Notes
Key design decisions:
-
shell(bd)already works (FR-008 is a doc gap, not a code gap): The existing first-word match already pre-approves allbdsubcommands. Phase 0 is a doc-only fix; Phase 1 addsshell(bd *)as a more explicit/expressive form. -
Hot-path performance constraint:
matchesRule()is called synchronously for every tool invocation. All new matching logic uses a compiled regex cache (Map<string, RegExp>) keyed by pattern string, cleared on config reload. No recompilation per call. -
Workspace file location:
.github/permissions.jsonaligns with.github/hooks/hooks.jsonalready in use; consistent with existing workspace conventions. -
Security gate default:
allowWorkspacePermissionsdefaults tofalse-- matching theallowWorkspaceHooksprecedent -- because a malicious repo could grantshell(*)via a workspace file. Operator must explicitly opt in. -
Deny-wins merge semantics: Global denies are never overridden by workspace allows.
effectiveDeny = globalDeny + workspaceDeny; any deny from any source wins over any allow from any source. -
Policy expansion at evaluation time: Policies are expanded in
evaluateConfigPermissions()(not at config-load time) to support hot-reload without restart. -
?wildcard: Reserved for future use; currently treated as a literal character.
Risks:
- Two-word generalization (
if (parts.length > 1)) is additive but could theoretically match patterns that authors previously assumed would NOT match. Backward-compat test suite (T008/T036) is the guard. - Policy cycle detection must throw a fatal config error (not a warning) -- silent cycles would silently drop rules.
- Broad-glob patterns like
shell(*)are valid syntax but operationally dangerous; T035 adds a load-time warning.
Suggested delivery order:
- Phase 0 - standalone doc PR (ship immediately, zero risk)
- Phases 1-3 (T001-T015) as MVP -- ~30 LOC net new in
src/config.ts - Phase 4 (workspace scoping) after MVP merges
- Phase 5 (named policy sets) after multi-workspace adoption justifies it
- Phase 6 (polish) tasks are independent and can accompany any phase PR