Open
Conversation
Review Summary by Qodo
WalkthroughsDescription• Add guard command for managing Agent Guard hooks • Support install/uninstall/status for Claude Code and Cursor • Extract push key minting/revocation into reusable module • Refactor EVO command to use new push key utilities Diagramflowchart LR
CLI["CLI guard command"]
Guard["guard.py module"]
PushKeys["pushkeys.py module"]
Claude["Claude Code config"]
Cursor["Cursor config"]
Script["snyk-agent-guard.sh script"]
CLI -- "install/uninstall/status" --> Guard
Guard -- "mint/revoke keys" --> PushKeys
Guard -- "manage hooks" --> Claude
Guard -- "manage hooks" --> Cursor
Guard -- "copy/execute" --> Script
File Changes1. src/agent_scan/cli.py
|
Code Review by Qodo
1. _run_install() too long
|
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
![qodo-merge-etso[bot]](https://avatars.githubusercontent.com/u/12959162?s=60&v=4){kind=small}
Comment on lines
+98
to
+181
| def _run_install(args) -> None: | ||
| client: str = args.client | ||
| url: str = args.url | ||
| push_key = os.environ.get("PUSH_KEY", "") | ||
| headless = bool(push_key) | ||
| tenant_id: str = getattr(args, "tenant_id", None) or "" | ||
|
|
||
| label = _client_label(client) | ||
| snyk_token = "" | ||
|
|
||
| if not headless: | ||
| # Interactive flow — mint a push key | ||
| rich.print(f"Installing [bold magenta]Agent Guard[/bold magenta] hooks for [bold]{label}[/bold]") | ||
| rich.print() | ||
|
|
||
| snyk_token = os.environ.get("SNYK_TOKEN", "") | ||
| if not snyk_token: | ||
| rich.print("Paste your Snyk API token ( from https://app.snyk.io/account ):") | ||
| snyk_token = input().strip() | ||
| if not snyk_token: | ||
| rich.print("[bold red]Error:[/bold red] SNYK_TOKEN is required to mint a push key.") | ||
| sys.exit(1) | ||
|
|
||
| if not tenant_id: | ||
| tenant_id = os.environ.get("TENANT_ID", "") | ||
| if not tenant_id: | ||
| rich.print("Enter your Snyk Tenant ID ( from the URL at https://app.snyk.io ):") | ||
| tenant_id = input().strip() | ||
| if not tenant_id: | ||
| rich.print("[bold red]Error:[/bold red] Tenant ID is required to mint a push key.") | ||
| sys.exit(1) | ||
|
|
||
| description = _get_machine_description(client) | ||
| rich.print(f"[dim]Minting push key for {description}...[/dim]") | ||
| try: | ||
| push_key = mint_push_key(url, tenant_id, snyk_token, description=description) | ||
| except RuntimeError as e: | ||
| rich.print(f"[bold red]Error:[/bold red] {e}") | ||
| sys.exit(1) | ||
| rich.print(f"[green]\u2713[/green] Push key minted [yellow]{_mask_key(push_key)}[/yellow]") | ||
|
|
||
| hook_client = "claude-code" if client == "claude" else "cursor" | ||
| minted = not headless # True if we minted the key in this run | ||
| config_path = _config_path(client, getattr(args, "file", None)) | ||
| # Copy hook script first so we can use it for the test event | ||
| dest_path, script_existed, script_updated = _copy_hook_script(client) | ||
|
|
||
| first_install = not config_path.exists() or not script_existed | ||
| run_test = first_install or minted or getattr(args, "test", False) | ||
|
|
||
| # Verify connectivity by invoking the actual hook script | ||
| if run_test and not _send_test_event(push_key, url, hook_client, dest_path): | ||
| # Clean up copied script only if it didn't exist before | ||
| if not script_existed: | ||
| dest_path.unlink(missing_ok=True) | ||
| if minted: | ||
| rich.print("[dim]Revoking minted push key...[/dim]") | ||
| try: | ||
| revoke_push_key(url, tenant_id, snyk_token, push_key) | ||
| rich.print("[green]\u2713[/green] Push key revoked") | ||
| except RuntimeError as e: | ||
| rich.print(f"[yellow]Warning:[/yellow] Could not revoke push key: {e}") | ||
| rich.print("[bold red]Aborting install — test event failed.[/bold red]") | ||
| raise SystemExit(1) | ||
|
|
||
| # Build command string and edit client config | ||
| command = _build_hook_command(push_key, url, dest_path, hook_client, tenant_id=tenant_id) | ||
|
|
||
| if client == "claude": | ||
| config_changed = _install_claude(command, config_path) | ||
| elif client == "cursor": | ||
| config_changed = _install_cursor(command, config_path) | ||
|
|
||
| if script_updated or config_changed or minted: | ||
| rich.print(f"[green]\u2713[/green] Hooks installed for [bold]{label}[/bold]") | ||
| else: | ||
| rich.print(f"[green]\u2713[/green] {label} hook integration up to date") | ||
| rich.print(f" Config: [dim]{config_path}[/dim]") | ||
| rich.print(f" Script: [dim]{dest_path}[/dim]") | ||
| rich.print(f" Remote URL: [dim]{url}[/dim]") | ||
| rich.print(f" Push Key: [yellow]{_mask_key(push_key)}[/yellow]") | ||
| rich.print() | ||
|
|
||
|
|
There was a problem hiding this comment.
1. _run_install() too long 📘 Rule violation ⚙ Maintainability
The new _run_install() function is ~80+ lines long, exceeding the 40 SLOC limit and making the install flow harder to maintain and review. This should be split into smaller helpers (e.g., token/tenant collection, mint/revoke, test event, config write).
Agent Prompt
## Issue description
`_run_install()` exceeds the ≤40 line function-length compliance limit, making the installation flow difficult to maintain.
## Issue Context
The function currently mixes: interactive credential collection, push-key minting/revocation, test-event sending, config/script path resolution, and config file mutation.
## Fix Focus Areas
- src/agent_scan/guard.py[98-181]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+133
to
+146
| local -a curl_args | ||
| curl_args=( | ||
| -sS | ||
| -X POST | ||
| "$url" | ||
| -H "User-Agent: ${user_agent}" | ||
| -H "X-User: ${x_user}" | ||
| -H "Content-Type: text/plain" | ||
| -H "X-Client-Id: ${pushkey}" | ||
| --data-binary "${encoded_body}" | ||
| ) | ||
|
|
||
| resp="$(curl "${curl_args[@]}" -w $'\n'"${marker}%{http_code}")" || die "Request failed" | ||
| http_code="${resp##*$'\n'"${marker}"}" |
There was a problem hiding this comment.
2. Hook can hang indefinitely 🐞 Bug ☼ Reliability
The installed hook script calls curl without connect/overall timeouts, so network stalls can block Claude/Cursor while the hook is executing. Because hooks run frequently, this can freeze the client on transient network problems.
Agent Prompt
### Issue description
The hook script can block the IDE/client indefinitely because `curl` has no connection or total timeout.
### Issue Context
Hooks are executed inline by Claude Code/Cursor; a hung hook can stall the entire user workflow.
### Fix Focus Areas
- src/agent_scan/hooks/snyk-agent-guard.sh[133-146]
### Suggested change
Add sane defaults like `--connect-timeout 5` and `--max-time 15` (and optionally a small retry) to the curl invocation. Consider making these overridable via env vars (e.g., `SNYK_AGENT_GUARD_CONNECT_TIMEOUT`, `SNYK_AGENT_GUARD_MAX_TIME`) so enterprise environments can tune them.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Trailing \r\n from [Console]::In.ReadToEnd() was getting base64-encoded, causing the server to reject the payload as invalid JSON. Also send the HTTP body as explicit UTF-8 bytes to avoid PowerShell's default encoding. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mmilanta
approved these changes
Apr 2, 2026
Contributor
mmilanta
left a comment
mmilanta
left a comment
There was a problem hiding this comment.
Looks good. Installed hooks on claude code with it and it looks fine.
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
2 participants
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.
No description provided.