Fix/issues from minax#82
Merged
Merged
Conversation
- Add --target <path> for explicit install directory override - Add --scope user|project|agent <name> for per-agent skill pools - Validate directory writability during auto-detection (skip read-only dirs) - Prompt user to select when multiple writable ~/.openclaw*/ dirs are found - Parse all flags upfront; uninstall now uses --uninstall flag (positional still works) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The script was cd-ing into skills/agentguard/scripts/ before running npm install, but package.json lives in skills/agentguard/. npm silently walked up and found the parent package.json, installing node_modules there instead of in scripts/ — causing runtime failures when scripts tried to resolve their dependencies. Fixes: - Step 2: cd into $SKILL_SRC (skills/agentguard/) where package.json lives - Step 4: copy package.json from $SKILL_SRC into $SKILLS_DIR/scripts/ before running npm install there, so node_modules land next to the scripts - Remove stale copy of package.json/package-lock.json from scripts/ loop (those files don't exist in scripts/ and the copy was a no-op) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
package.json lives in skills/agentguard/, not scripts/. The old code cd'd into scripts/ before running npm install, so npm silently walked up to the parent and installed node_modules there instead of where scripts expect them. Fix: copy package.json to $SKILLS_DIR (the skill root) and run a single npm install there. Scripts are always invoked as: cd $SKILLS_DIR && node scripts/<script> so Node's module resolution finds node_modules at $SKILLS_DIR without needing a second install in scripts/. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…(issue #50) Auto-detection priority: 1. $OPENCLAW_STATE_DIR (per-agent runtime env var) — covers multi-agent platforms 2. ~/.openclaw (global, writable only) — existing single-agent installs 3. ~/.claude — Claude Code If nothing is detected, exit with an actionable error pointing to --target. --target <path> installs to <path>/agentguard, supporting arbitrary directory layouts like ~/minax/agents/cto-owen/skills. Tilde expansion is handled safely without eval. Also fixes npm install running in the wrong directory (issue #49): dependencies now install at $SKILLS_DIR root so Node can resolve node_modules when scripts run as `cd $SKILLS_DIR && node scripts/...`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…#51) An LLM can read SKILL.md to learn the report format and fabricate findings without running any actual checks. Fix by adding three evidence gates to the checkup flow: 1. EVIDENCE RULE at Step 1 start: every finding must quote actual tool output; inference and fabrication are explicitly forbidden. 2. Per-check evidence requirements: each [REQUIRED] item now specifies exactly what artifact to record (permission string, grep match line, command output, etc.). 3. EVIDENCE GATE at Step 3: before writing the analysis, verify every finding traces back to a real tool output — drop any that don't. 4. Pre-Step-4 validation: evidence check before JSON assembly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
#52) Multi-agent platforms set $OPENCLAW_STATE_DIR per-agent at runtime. By deriving agentguard/ paths from it, each agent automatically gets its own isolated audit log and trust registry with no manual config. Path resolution priority (registry + audit log): 1. $OPENCLAW_STATE_DIR/agentguard/ — per-agent auto-isolation 2. $AGENTGUARD_HOME/ — explicit override (unchanged) 3. ~/.agentguard/ — fallback for single-agent envs Files changed: - src/registry/storage.ts: registry.json path respects OPENCLAW_STATE_DIR - src/adapters/common.ts: audit log dir respects OPENCLAW_STATE_DIR - src/adapters/openclaw-plugin.ts: same - skills/agentguard/scripts/auto-scan.js: same - skills/agentguard/SKILL.md: replace `find` binary calls in patrol checks with Glob + stat to work in hardened environments where external binaries are restricted Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…TTERN (issue #53) Rules 7 and 8 previously reported CRITICAL for all plaintext key matches regardless of git context, causing alert fatigue when keys are properly gitignored and never committed. Severity is now determined by a four-step git context check after the initial pattern match: 1. Not in a git repo → CRITICAL (unknown exposure) 2. File ever in git history → CRITICAL (already leaked) 3. Tracked but not gitignored → HIGH (could be committed) 4. Gitignored → MEDIUM (local only) git context result is recorded in the finding's Evidence column. Falls back to CRITICAL if git is not available in PATH. Changes: - skills/agentguard/SKILL.md: add Git Context Check section; mark Rule 7/8 as CRITICAL* in the detection rules table - skills/agentguard/scan-rules.md: add git-aware severity table to Rule 7 and Rule 8 definitions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ssue #54) Node.js 18/20 LTS cannot execute TypeScript files natively. The .ts scripts were being invoked directly with `node`, which fails on any LTS release below v22.6 without tsx/ts-node installed. Fix: rewrite both CLIs as plain ESM .js files with all TypeScript annotations removed. Functionality is identical — only type syntax is dropped. The .ts source files are removed to avoid confusion. Updated references: - skills/agentguard/SKILL.md: all 12 occurrences of trust-cli.ts / action-cli.ts → .js; allowed-tools permissions updated to match - setup.sh: file copy list updated to .js Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… (issue #55) patrol setup and patrol status were hardcoded to use `openclaw cron`, making them non-functional on Claude Code and standard Linux environments. Introduce three-path platform detection for patrol setup/status: Path A — OpenClaw CLI available: use `openclaw cron add` (existing behavior) Path B — system crontab available: write entry via `crontab -l | crontab -`, invoking auto-scan.js with AGENTGUARD_AUTO_SCAN=1 Path C — neither available: print the crontab entry for manual installation patrol status now queries whichever scheduler is present: - openclaw cron list (Path A) - crontab -l | grep agentguard (Path B) patrol run is unaffected — it requires no scheduling platform. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add checkup-score.js which accepts raw check facts as JSON and computes all dimension scores, composite score, and tier assignment without any LLM arithmetic. Refactor SKILL.md checkup flow into 7 steps so the LLM only collects raw facts and writes analysis — all numerical calculations are delegated to the script. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add scan-to-sarif.js which converts scan findings to valid SARIF 2.1.0 with full rule metadata and CVSS-like security-severity scores. Update SKILL.md to detect --format sarif for scan (LLM assembles findings JSON, script outputs SARIF) and --format json --output <file> for checkup (writes raw JSON, skips HTML generation). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ssion Add suppress.example.yaml with documented schema and common patterns (test fixtures, internal webhooks, dev tooling). Update SKILL.md scan flow to read .agentguard-suppress.yaml from the scan target root before detection, apply rule/path/domain matching to filter findings, and record a suppression summary line in the report. Suppressed findings are excluded from the findings table and risk level calculation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add trust seed subcommand to SKILL.md with --auto-attest-low-risk, --auto-attest-medium-risk, and --dry-run flags. Flow: discover all installed skills, filter unregistered ones, batch scan, show plan table with proposed actions, require explicit user confirmation, then sequentially attest qualifying skills via trust-cli. HIGH/CRITICAL risk skills are never auto-attested and are listed for manual review. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…into fix/issues-from-minax
…to fix/issues-from-minax
…' into fix/issues-from-minax # Conflicts: # setup.sh
…rts' into fix/issues-from-minax
…at' into fix/issues-from-minax
… into fix/issues-from-minax
…nto fix/issues-from-minax # Conflicts: # setup.sh
… into fix/issues-from-minax
…iability' into fix/issues-from-minax # Conflicts: # setup.sh # skills/agentguard/SKILL.md
…fix/issues-from-minax # Conflicts: # setup.sh # skills/agentguard/SKILL.md
…o fix/issues-from-minax # Conflicts: # skills/agentguard/SKILL.md
…x/issues-from-minax
AgentGuard PR ReviewI found a few concrete regressions and security issues introduced by the patch.
|
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
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.
Summary
This PR batches fixes and user-requested improvements collected from Minax feedback around AgentGuard setup, skill runtime compatibility, checkup reliability, scan output, patrol scheduling, and trust baseline workflows.
Fixed Issues
Fixes #48
--target,--scope user|project|agent, writable directory validation, and improved install target selection.Fixes #49
setup.shnpm install directory handling so dependencies are installed where AgentGuard skill scripts can resolve them correctly.Fixes #50
$OPENCLAW_STATE_DIR,~/.openclaw, and~/.claude.--target <path>fallback for custom agent skill layouts.Fixes #51
Fixes #52
$OPENCLAW_STATE_DIR.Fixes #53
Fixes #54
trust-cli.tsandaction-cli.tswith plain Node-compatible.jsscripts.Fixes #55
patrol setupandpatrol statuswhen OpenClaw cron is unavailable.Fixes #56
checkup-score.js.Fixes #57
scan-to-sarif.js.--format json --output <file>.Fixes #58
.agentguard-suppress.yamlsupport for false-positive suppression.Fixes #62
trust seedworkflow for batch trust baseline setup.Merge Notes
Merged latest
maininto this branch and resolved theskills/agentguard/SKILL.mdconflict by preserving main’s new CLI/subscribe/Hermes/checkup advisory behavior while keeping this branch’s skill bug fixes and new trust/scan/checkup options.Verification
npm run buildnpm testResult: 246 tests passed, 0 failed.
Type
Testing
npm run buildpassesnpm testpasses (32 tests)Related Issues
Closes #