fix(guardrails): address review follow-up findings (#104-107) by terisuke · Pull Request #109 · Cor-Incorporated/opencode

terisuke · 2026-04-06T08:45:03Z

Summary

Fix push block regex bypassed by flags between push and remote (fix(guardrails): push block regex bypassed by flags between push and remote #104)
Restrict security-engineer grep to deny sensitive files (fix(guardrails): security-engineer grep:allow bypasses read deny for .env files #105)
Reorder incident-responder curl rules for last-match-wins correctness (fix(guardrails): incident-responder curl rule ordering allows pipe-to-shell #106)
Gate auto-format reminder with code(file) check (fix(guardrails): auto-format reminder fires on non-source files #107)

Closes #104, Closes #105, Closes #106, Closes #107

Type of change

Bug fix (non-breaking change that fixes an issue)

Changes

4 atomic commits, each fixing one review follow-up finding:

fix(guardrails): push block regex bypassed by flags between push and remote #104: Add optional flag group (?:(?:-\w+|--[\w-]+)\s+)* to push block regex so git push -u origin main is correctly blocked
fix(guardrails): security-engineer grep:allow bypasses read deny for .env files #105: Replace blanket grep: allow with pattern-based deny list mirroring read deny rules (.env, credentials, pem, key, secret)
fix(guardrails): incident-responder curl rule ordering allows pipe-to-shell #106: Move curl *: ask before curl * | sh*: deny so pipe-to-shell is denied under last-match-wins semantics
fix(guardrails): auto-format reminder fires on non-source files #107: Add code(file) guard to auto-format reminder so it only fires for source code edits, not docs/config

Verification

bun turbo typecheck — 13/13 pass
Deploy fire test: git push -u origin main blocked
Deploy fire test: security-engineer grep .env denied
Deploy fire test: incident-responder curl X | sh denied
Deploy fire test: editing .md 3x does not trigger format reminder

Checklist

Code follows project conventions
Each fix references its follow-up origin PR (feat(guardrails): add CI gate, push block, and auto-format hooks #102, feat(guardrails): add CC parity commands and specialist agents #100)
No breaking changes

github-actions · 2026-04-06T08:45:12Z

This PR doesn't fully meet our contributing guidelines and PR template.

What needs to be fixed:

PR description is missing required template sections. Please use the PR template.

Please edit this PR description to address the above within 2 hours, or it will be automatically closed.

If you believe this was flagged incorrectly, please let a maintainer know.

Copilot

Pull request overview

This PR applies four follow-up fixes to the Guardrails profile to close identified bypasses and correctness issues in command blocking, agent permissions, and edit-triggered reminders.

Changes:

Update the protected-branch git push detection regex to account for flags between push and the remote.
Tighten security-engineer permissions by replacing blanket grep: allow with a structured rule block.
Reorder incident-responder curl rules to align with last-match-wins permission evaluation.
Gate the auto-format reminder behind a code(file) check so it only triggers on source edits.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
packages/guardrails/profile/plugins/guardrail.ts	Adjusts protected-branch push detection and gates the auto-format reminder on `code(file)`.
packages/guardrails/profile/agents/security-engineer.md	Replaces broad grep permission with allow + deny patterns intended to block sensitive files.
packages/guardrails/profile/agents/incident-responder.md	Reorders curl/pipe-to-shell rules to ensure deny wins under last-match-wins semantics.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-06T08:49:12Z

        const protectedBranch = /^(main|master|develop|dev)$/
        if (/\bgit\s+push\b/i.test(cmd)) {
          // Check explicit branch target
-          const explicitMatch = cmd.match(/\bgit\s+push\s+\S+\s+(?:HEAD:)?(\S+)/i)
+          const explicitMatch = cmd.match(/\bgit\s+push\s+(?:(?:-\w+|--[\w-]+)\s+)*\S+\s+(?:HEAD:)?(\S+)/i)
          if (explicitMatch && protectedBranch.test(explicitMatch[1])) {


The new git push regex still treats many valid flag forms as the remote name, so protected-branch pushes can still bypass the block (e.g., git push --push-option=ci.skip origin main where --push-option=... doesn’t match --[\w-]+, or flags that take a separate argument like -o ci.skip). Consider tokenizing the command (shellwords) and skipping all leading options (handling --opt=value and --opt value / -o value) before extracting <remote> <refspec> and checking the refspec/branch.

Copilot · 2026-04-06T08:49:12Z

          // Plain `git push` with no branch — check current branch
-          if (!/\bgit\s+push\s+\S+\s+\S+/i.test(cmd)) {
+          if (!/\bgit\s+push\s+(?:(?:-\w+|--[\w-]+)\s+)*\S+\s+\S+/i.test(cmd)) {
            try {


Same issue for the “plain git push” detection regex: options with attached or separate values (e.g. -o ci.skip, --push-option=...) won’t be consumed by the flag group and can cause the pattern to mis-detect whether a branch was provided, which may skip the intended protected-branch checks. Using the same tokenization-based approach here would avoid regex edge cases.

Copilot · 2026-04-06T08:49:13Z

+    "*": allow
+    "*.env*": deny
+    "*credentials*": deny
+    "*.pem": deny
+    "*.key": deny
+    "*secret*": deny


These grep deny patterns won’t actually prevent reading .env/key material via the grep tool, because GrepTool asks permission against params.pattern (the regex searched for), not the file paths being scanned. As a result, grep "DATABASE_URL" over a repo will still return matches from .env files. To fix #105 you likely need to (a) remove/disable grep for this agent, or (b) update the grep tool/permission model to authorize based on searched file paths (or at least exclude sensitive globs in the tool itself before returning matches).

Suggested change

"*": allow

"*.env*": deny

"*credentials*": deny

"*.pem": deny

"*.key": deny

"*secret*": deny

"*": deny

The push block regex now skips optional flags (-u, --force, --set-upstream) between `push` and the remote/branch arguments. Fixes #104 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…les (#105) Replace blanket `grep: allow` with pattern-based permissions that mirror the read deny list (.env, credentials, pem, key, secret). Fixes #105 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…-wins (#106) Move `curl * | sh*: deny` and `wget * | sh*: deny` after `curl *: ask` so the deny rule wins for pipe-to-shell commands under last-match-wins semantics. Fixes #106 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The formatter reminder now only fires for source code edits, not docs/config/markdown. Consistent with the doc-update reminder above it. Fixes #107 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings April 6, 2026 08:45

github-actions Bot added the needs:compliance label Apr 6, 2026

Copilot started reviewing on behalf of terisuke April 6, 2026 08:45 View session

terisuke mentioned this pull request Apr 6, 2026

[Master Plan] OpenCode Claude Code Parity — 実装ロードマップ #51

Closed

4 tasks

Copilot AI reviewed Apr 6, 2026

View reviewed changes

terisuke and others added 4 commits April 6, 2026 17:50

fix(guardrails): handle git push flags in protected branch regex (#104)

6eb3c44

The push block regex now skips optional flags (-u, --force, --set-upstream) between `push` and the remote/branch arguments. Fixes #104 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

terisuke force-pushed the fix/guardrails-follow-up-review-fixes branch from 2563c31 to be3a4e4 Compare April 6, 2026 08:50

terisuke merged commit 8c15b36 into dev Apr 6, 2026
4 of 5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(guardrails): address review follow-up findings (#104-107)#109

fix(guardrails): address review follow-up findings (#104-107)#109
terisuke merged 4 commits into
devfrom
fix/guardrails-follow-up-review-fixes

terisuke commented Apr 6, 2026

Uh oh!

github-actions Bot commented Apr 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

terisuke commented Apr 6, 2026

Summary

Type of change

Changes

Verification

Checklist

Uh oh!

github-actions Bot commented Apr 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants