feat(security): add approval gate for /copilot commands#168
Open
feat(security): add approval gate for /copilot commands#168
Conversation
Implements GitHub Issue #156: Add review/approval gate for /copilot commands. When COPILOT_REQUIRE_APPROVAL=true, /copilot commands require explicit approval before execution. User posts /copilot <prompt>, agent posts confirmation comment, user approves with 👍 or /copilot approve, then agent executes. Key changes: - New approval_store.py: Protocol + Memory + Redis implementations - Config: copilot_require_approval (default: false), copilot_approval_timeout - Models: PendingApproval for storing pending commands - mr_comment_handler: Refactored to handle approval flow, extracted _execute_copilot_task - webhook: Pass approval_store to handler, recognize /copilot approve - main: Wire approval_store into lifespan Security: - Only the original requester can approve (prevents hijacking) - Silent timeout expiry (no spam on timeout) - Backward compatible: disabled by default Testing: - 7 new tests for approval_store (basic flow, TTL, isolation) - 5 new tests for approval flow (require, approve, wrong user, no pending) - All 351 tests pass Note: 519-line diff (225 prod, 294 tests). Exceeds 200-line guideline for feat commits, but justified: 1. Feature is cohesive and atomic 2. Majority is test coverage (required for 90% target) 3. Proper Protocol abstraction (reuses state_backend pattern) 4. Zero impact when disabled (backward compat) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sed gate - Pass approval_store to GitLabPoller and handle_copilot_comment from poller path - Recognize /copilot approve notes in poller (not just webhook) - Make approval gate fail-closed: refuse execution when store unavailable - Fixes HIGH: poller path bypassed approval gate entirely Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
peteroden
force-pushed
the
feat/156-approval-gate
branch
from
1c0e37d to
83ca2a2
Compare
Replace get()+delete() with atomic pop() to prevent duplicate execution when concurrent workers process the same /copilot approve command. Memory store uses dict.pop(); Redis store uses GETDEL. Wrong-user rejections re-store the approval after pop. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Implements opt-in approval gate for
/copilotcommands (Issue #156). WhenCOPILOT_REQUIRE_APPROVAL=true, users must approve commands before execution.Closes #156
How it works
/copilot fix the bugon an MR/copilot approveto proceed."Configuration
COPILOT_REQUIRE_APPROVALfalseCOPILOT_APPROVAL_TIMEOUT3600Security
Files changed
New
src/gitlab_copilot_agent/approval_store.py— Protocol + Memory + Redis implementationstests/test_approval_store.py— 7 tests for store behaviorModified
config.py— 2 new settingsmodels.py—PendingApprovalPydantic modelmr_comment_handler.py— approval flow + extracted_execute_copilot_task()webhook.py— route/copilot approvemain.py— wire approval_store in lifespangitlab_poller.py— thread approval_store (fixes HIGH: poller bypass)tests/conftest.py— approval_store fixturetests/test_mr_comment_handler.py— 5 new approval flow testsCode Review
GPT-5.3-Codex review found HIGH: GitLab poller bypassed approval gate — fixed in second commit (fail-closed + threaded store through poller).
Testing
Diff size
599 lines (534 insertions in new files/tests). Exceeds 200-line guideline but feature is atomic — splitting would break functionality.