feat(opencode): add command and URL allowlists - #3789
feat(opencode): add command and URL allowlists#3789Sylvester Kaczmarek (sylvesterkaczmarek) wants to merge 9 commits into
Conversation
Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com>
PR Review Summary
Verdict: AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims. |
|
🟡 Contributor Check: MEDIUM
Automated check by AGT Contributor Check. |
Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com>
Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com>
Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com>
Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com>
Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com>
Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com>
|
MohammadHaroonAbuomar Thanks for the detailed review. I pushed fixes covering all six findings:
The OpenCode package build/tests and repository CI checks pass on the updated head. Please re-review. |
MohammadHaroonAbuomar
left a comment
There was a problem hiding this comment.
- Last item from the earlier review: please add a CHANGELOG.md [Unreleased] entry covering (a) slashless 'https:host' forms now canonicalizing into existing urlRules and (b) the new unconditional backslash-authority deny (it fires even with urlDefaultEffect: allow), and correct the PR body's 'existing policies keep their current behavior' claim, which both changes now overstate. Everything else from the review is verified fixed — this is the only remaining item.
Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com>
a81d4f7 to
148f4b0
Compare
MohammadHaroonAbuomar
left a comment
There was a problem hiding this comment.
- One sentence left: the PR description's 'so existing policies keep their current behavior' (Security and compatibility section) is still unqualified — the backslash-authority deny fires even with urlDefaultEffect: allow, as your new CHANGELOG entry correctly states. Please qualify that sentence (e.g. '...keep their current behavior, except the two hardening changes noted in the changelog'). Everything else is verified and this is ready to go once edited.
|
MohammadHaroonAbuomar Updated the PR description to qualify the compatibility statement as requested. The changelog entry is also in place, all earlier review items are addressed, and the review threads are resolved. Ready for re-review when convenient. |
Summary
Add opt-in positive command and URL/domain allowlists to the OpenCode governance policy surface, closing #3666.
Problem
OpenCode currently supports negative controls through
blockedToolCalls[].commandPatternsanddirectResourcePolicies.urlRules, but cannot express policies such as “only these commands” or “only these network destinations” with an explicit default-deny posture.Changes
toolPolicies.allowedCommandPatternsandcommandDefaultEffectdirectResourcePolicies.allowedDomains,allowedUrlPatterns, andurlDefaultEffect*.example.comsubdomains, and optional explicit portsg/yregex flagsdenyOnPolicyErrorbehavior./policyexport, and bundled MCP server through the OpenCode-specific allowlist layerSecurity and compatibility
The positive allowlist controls are opt-in: both new default effects remain
allowunless explicitly changed todeny. Separately, this PR tightens existing URL-rule handling: surfaced HTTP(S) forms such ashttps:hostare canonicalized before existingurlRulesare evaluated, and ambiguous HTTP(S) authorities containing backslashes are denied even whenurlDefaultEffectisallow.The positive layer can only add a deny.
PolicyEnginealready resolves backend decisions withdenytaking precedence overreviewandallow.For URLs, every HTTP(S) value surfaced as a tool argument is checked, so an allowed primary URL does not authorize a separate unapproved redirect-target argument. Redirects hidden inside an HTTP client remain outside the plugin hook boundary.
No runtime dependency is added.
Testing
Added
agent-governance-opencode/test/allowlist-policy.test.mjsand wired it into the package's existingnpm run check/npm testglob.Closes #3666