Repository navigation
feat(kiro): add upgrade-guardian and generic rev-eng agents - #17
Conversation
Add two self-contextualizing Kiro agents to the distributed payload, both read-only and following the bepower-setup pattern (resources read the target project's README/AGENTS.md/package.json/steering at boot). upgrade-guardian: assesses whether a dependency/framework upgrade is safe, in an isolated worktree, and returns a reasoned LOW/MEDIUM/HIGH verdict with evidence. The substance lives in a reusable skill (skills/upgrade-guardian/ SKILL.md) — semver-scaled depth, 6-step playbook (isolate -> breaking-change analysis -> project impact -> verify -> cross-repo blast radius -> verdict) — so any agent can load it via skill_search; the agent is a thin wrapper. It never commits, pushes, publishes, opens a PR, or --saves into the real tree. rev-eng: generic API reverse-engineering via Playwright MCP, generalized from the Ream-specific original — the target app, auth variables and functional areas are deduced from the project, not hardcoded. Writes only to docs/api/**, read-only otherwise, with a clean-up guardrail that leaves the target platform in the state it was found.
Revise the hooks for the two-runtime world (kiro-cli/IDE + autonomous KiroCrew) and wire the new agents into distribution. Hooks now carry a `runtime: ide|any` front-matter tag instead of being split into folders: safety-gate is `any` (applies everywhere, defense-in-depth with each agent's deniedCommands), barrel-export is `ide` (fires on editor events, no-op headless). context-injection and post-task-summary are marked `deprecated: true` (superseded by Crew 0.7 'Skills Discovered by Search' and the native end-of-task summary respectively) but kept, not removed, pending an archival decision. A new hooks/README.md documents the runtime field and the proposed init-kiro filtering. init-kiro now installs upgrade-guardian and rev-eng (agents + prompts) for everyone, consistent with copying the whole payload; the test asserts they land.
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
This PR successfully adds two autonomous, self-contextualizing Kiro agents (upgrade-guardian and rev-eng) with proper read-only guarantees and security controls. The architecture is well-designed with defense-in-depth security through allowedTools, deniedCommands, and inline prompt constraints.
Critical Issue (Must Fix)
Security: One regex pattern in upgrade-guardian.json has insufficient coverage for blocking npm install variants that could bypass the read-only guarantee. See inline comment with fix.
Overall Assessment
Once the security regex is fixed, this PR is ready to merge. The implementation demonstrates strong security awareness with multiple layers of protection, comprehensive test coverage, and clear documentation for both agents and their integration patterns.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| ".*npm publish.*", | ||
| ".*gh pr create.*", | ||
| ".*gh pr merge.*", | ||
| ".*npm install.*--save.*", |
There was a problem hiding this comment.
🛑 Security Vulnerability: The regex pattern ".*npm install.*--save.*" will fail to block command variants with different flag formats. An agent could bypass this with npm install pkg -S, npm install --save-dev, or npm i --save, circumventing the read-only guarantee.1
| ".*npm install.*--save.*", | |
| .*npm (install|i).*(-S|--save|-D|--save-dev|--save-optional|--save-exact).* |
Footnotes
-
CWE-185: Incorrect Regular Expression - https://cwe.mitre.org/data/definitions/185.html ↩
The deniedCommand '.*npm install.*--save.*' only blocked the literal '--save' flag on 'npm install', leaving the read-only guarantee bypassable via 'npm i -S', 'npm install -D', 'npm add --save-dev', etc. Broaden it to 'npm (install|i|add)' with any save flag (-S/-D/-O/-E/--save*), so the agent cannot mutate the real dependency tree through any install alias. Addresses the Amazon Q Developer review finding on PR #17 (CWE-185).
|
Fixed in c71886b: broadened the npm-save deny pattern to |
|
/q review |
There was a problem hiding this comment.
Summary
This PR introduces two new self-contextualizing Kiro agents (upgrade-guardian and rev-eng) for autonomous operations. The implementation is well-structured with clear separation of concerns, but there are critical security issues that must be addressed before merge.
Critical Issues (Must Fix)
Security vulnerabilities in agent configurations: Both new agents have ambiguous write access configurations where write appears in tools but not in allowedTools, yet toolsSettings.write defines allowed paths. This creates unclear access control that could lead to unintended file modifications.
Missing error handling: The init-kiro command lacks error handling for all file operations, which will cause crashes on common failure scenarios (disk full, permissions) and leave partial installations.
Files Reviewed
- ✅ Agent configurations (
upgrade-guardian.json,rev-eng.json) - ✅ Agent prompts and skills
- ✅ Installation command updates
- ✅ Test coverage
The agents themselves are well-designed with proper read-only constraints in their prompts. Once the access control ambiguities and error handling are resolved, this will be a solid addition.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| "tools": [ | ||
| "read", | ||
| "write", | ||
| "shell", | ||
| "grep", | ||
| "glob", | ||
| "web_search", | ||
| "web_fetch", | ||
| "introspect", | ||
| "thinking", | ||
| "code" | ||
| ], | ||
| "allowedTools": [ | ||
| "read", | ||
| "shell", | ||
| "grep", | ||
| "glob", | ||
| "web_search", | ||
| "web_fetch", | ||
| "introspect", | ||
| "thinking", | ||
| "code" | ||
| ], | ||
| "toolsSettings": { | ||
| "write": { | ||
| "allowedPaths": [".kiro/upgrade-reports/**", "docs/upgrades/**"] | ||
| }, |
There was a problem hiding this comment.
🛑 Security Vulnerability: The write tool is listed in the tools array but not in allowedTools, yet toolsSettings.write configures allowed write paths. This creates ambiguity about whether write access is permitted. For a read-only agent, remove the write tool from the tools array entirely and remove the toolsSettings.write configuration to prevent any write capability.1
| "tools": [ | |
| "read", | |
| "write", | |
| "shell", | |
| "grep", | |
| "glob", | |
| "web_search", | |
| "web_fetch", | |
| "introspect", | |
| "thinking", | |
| "code" | |
| ], | |
| "allowedTools": [ | |
| "read", | |
| "shell", | |
| "grep", | |
| "glob", | |
| "web_search", | |
| "web_fetch", | |
| "introspect", | |
| "thinking", | |
| "code" | |
| ], | |
| "toolsSettings": { | |
| "write": { | |
| "allowedPaths": [".kiro/upgrade-reports/**", "docs/upgrades/**"] | |
| }, | |
| { | |
| "tools": [ | |
| "read", | |
| "shell", | |
| "grep", | |
| "glob", | |
| "web_search", | |
| "web_fetch", | |
| "introspect", | |
| "thinking", | |
| "code" | |
| ], | |
| "allowedTools": [ | |
| "read", | |
| "shell", | |
| "grep", | |
| "glob", | |
| "web_search", | |
| "web_fetch", | |
| "introspect", | |
| "thinking", | |
| "code" | |
| ], | |
| "toolsSettings": { | |
| "shell": { | |
| "autoAllowReadonly": true, | |
| "deniedCommands": [ | |
| ".*git commit.*", | |
| ".*git push.*", | |
| ".*git tag .*", | |
| ".*npm publish.*", | |
| ".*gh pr create.*", | |
| ".*gh pr merge.*", | |
| ".*npm (install|i|add).*(-S|-D|-O|-E|--save).*", | |
| ".*rm -rf.*" | |
| ] | |
| } | |
| } |
Footnotes
-
CWE-284: Improper Access Control - https://cwe.mitre.org/data/definitions/284.html ↩
There was a problem hiding this comment.
Rebutting — this is the intentional BePower "ask-before-write" pattern, not an ambiguity. write in tools but omitted from allowedTools means the tool exists but every write needs explicit approval, and toolsSettings.write.allowedPaths narrows where it may write. It is the exact shape used by the existing kiro/agents/functional-analyst.json (write allowed only under docs/** + .kiro/steering/**) and documented in kiro/skills/agent-template/SKILL.md (rule 4: "allowedTools should include everything EXCEPT write; write needs approval").
The write capability is required and deliberate: upgrade-guardian writes its report only to .kiro/upgrade-reports/** / docs/upgrades/**, gated by approval. Removing write entirely would break the agent’s one legitimate output. Keeping as-is.
| "tools": [ | ||
| "read", | ||
| "write", | ||
| "shell", | ||
| "grep", | ||
| "glob", | ||
| "web_search", | ||
| "web_fetch", | ||
| "introspect", | ||
| "thinking", | ||
| "@playwright" | ||
| ], | ||
| "allowedTools": [ | ||
| "read", | ||
| "grep", | ||
| "glob", | ||
| "web_search", | ||
| "web_fetch", | ||
| "introspect", | ||
| "thinking", | ||
| "@playwright" | ||
| ], | ||
| "toolsSettings": { | ||
| "write": { | ||
| "allowedPaths": ["docs/api/**"] | ||
| }, |
There was a problem hiding this comment.
🛑 Security Vulnerability: The write tool appears in tools array but not in allowedTools, yet toolsSettings.write configures allowed write paths. This creates ambiguity - either the agent can write (and should have write in allowedTools) or it's read-only (and should remove write from tools and toolsSettings). The description claims "never mutates project source outside the docs API folder", so if write access to docs/api/** is intentional, add write to allowedTools. Otherwise, remove write capability entirely.1
Footnotes
-
CWE-284: Improper Access Control - https://cwe.mitre.org/data/definitions/284.html ↩
There was a problem hiding this comment.
Rebutting — same intentional "ask-before-write" pattern as functional-analyst.json. write in tools + omitted from allowedTools + toolsSettings.write.allowedPaths: ["docs/api/**"] means the agent CAN write, but only to docs/api/** and only with per-write approval. The description ("never mutates project source outside the docs API folder") is exactly this: writing the discovered API docs there is the intended, scoped output. Adding write to allowedTools would REMOVE the approval gate — the opposite of what we want. Keeping as-is (approval-gated, path-scoped).
| await cp( | ||
| join(paths.kiro, 'agents/upgrade-guardian.json'), | ||
| join(kiroDir, 'agents/upgrade-guardian.json'), | ||
| ); | ||
| await cp(join(paths.kiro, 'agents/rev-eng.json'), join(kiroDir, 'agents/rev-eng.json')); |
There was a problem hiding this comment.
🛑 Crash Risk: All file copy operations including these new agent copies lack error handling. If any cp or mkdir operation fails (disk full, permission denied, source file missing), the command will crash without cleaning up partial copies, leaving the user with an incomplete installation. Wrap the entire .action() callback in try-catch and exit gracefully with a clear error message.1
Footnotes
-
CWE-755: Improper Handling of Exceptional Conditions - https://cwe.mitre.org/data/definitions/755.html ↩
There was a problem hiding this comment.
Fixed in 5542cec: wrapped the entire init-kiro .action() body in try-catch — on any mkdir/cp failure it now prints a clear ✗ Failed to install... message and sets process.exitCode = 1 instead of crashing with a raw stack trace. Typecheck + tests green.
Wrap the init-kiro action body in try-catch: if any mkdir/cp fails (disk full, permission denied, missing source), print a clear error and set a non-zero exit code instead of crashing with a raw stack trace on a partial install. Addresses the Amazon Q Developer review finding on PR #17 (CWE-755).
|
/q review |
There was a problem hiding this comment.
Review Summary
This PR adds two self-contextualizing Kiro agents (upgrade-guardian and rev-eng) with comprehensive documentation and testing. The implementation follows BePower conventions with proper error handling and security constraints.
Critical Issues Found (2)
Security vulnerabilities in both agent configurations: The "write" tool is declared in the "tools" array but not in "allowedTools", creating a security bypass. Agents could potentially use shell commands to write files outside allowed paths, violating the read-only guarantees that are central to both agents' design.
Recommendation
Address the security vulnerabilities before merging. Once fixed, the implementation is solid with good test coverage and well-structured prompts.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| "tools": [ | ||
| "read", | ||
| "write", | ||
| "shell", | ||
| "grep", | ||
| "glob", | ||
| "web_search", | ||
| "web_fetch", | ||
| "introspect", | ||
| "thinking", | ||
| "code" | ||
| ], |
There was a problem hiding this comment.
🛑 Security Vulnerability: The "write" tool is declared in the "tools" array but not in "allowedTools", creating the same security bypass as in rev-eng.json. An agent could use shell commands to write files outside the allowed paths, violating the read-only guarantees.
Remove "write" from the "tools" array.1
| "tools": [ | |
| "read", | |
| "write", | |
| "shell", | |
| "grep", | |
| "glob", | |
| "web_search", | |
| "web_fetch", | |
| "introspect", | |
| "thinking", | |
| "code" | |
| ], | |
| "tools": [ | |
| "read", | |
| "shell", | |
| "grep", | |
| "glob", | |
| "web_search", | |
| "web_fetch", | |
| "introspect", | |
| "thinking", | |
| "code" | |
| ], |
Footnotes
-
CWE-732: Incorrect Permission Assignment for Critical Resource - https://cwe.mitre.org/data/definitions/732.html ↩
| "tools": [ | ||
| "read", | ||
| "write", | ||
| "shell", | ||
| "grep", | ||
| "glob", | ||
| "web_search", | ||
| "web_fetch", | ||
| "introspect", | ||
| "thinking", | ||
| "@playwright" | ||
| ], |
There was a problem hiding this comment.
🛑 Security Vulnerability: The "write" tool is declared in the "tools" array but not in "allowedTools", which creates a security bypass. An agent could potentially use the "shell" tool with commands like echo "content" > file.txt to write to files outside the allowed paths, circumventing the write restrictions.
Remove "write" from the "tools" array to enforce the read-only guarantee.1
| "tools": [ | |
| "read", | |
| "write", | |
| "shell", | |
| "grep", | |
| "glob", | |
| "web_search", | |
| "web_fetch", | |
| "introspect", | |
| "thinking", | |
| "@playwright" | |
| ], | |
| "tools": [ | |
| "read", | |
| "shell", | |
| "grep", | |
| "glob", | |
| "web_search", | |
| "web_fetch", | |
| "introspect", | |
| "thinking", | |
| "@playwright" | |
| ], |
Footnotes
-
CWE-732: Incorrect Permission Assignment for Critical Resource - https://cwe.mitre.org/data/definitions/732.html ↩
1 participant
What
Add two self-contextualizing Kiro agents to the distributed payload and revise the
hooks for the two-runtime world (kiro-cli/IDE + autonomous KiroCrew).
upgrade-guardian— assesses whether a dependency/framework upgrade is safe,in an isolated worktree, returning a reasoned LOW/MEDIUM/HIGH verdict with evidence.
The substance lives in a reusable skill (
skills/upgrade-guardian/SKILL.md):semver-scaled depth, a 6-step playbook (isolate → breaking-change analysis →
project impact → verify → cross-repo blast radius → verdict), and the report format.
The agent is a thin read-only wrapper around it. Read-only: never commits, pushes,
publishes, opens a PR, or
--saves into the real tree.rev-eng— generic API reverse-engineering via Playwright MCP, generalized fromthe Ream-specific original. Target app, auth variables and functional areas are
deduced from the project, not hardcoded. Writes only to
docs/api/**.runtime: ide|anyfront-matter tag instead of folder splits;context-injectionandpost-task-summaryare markeddeprecated(superseded byCrew 0.7 features) but kept pending an archival decision. New
hooks/README.md.init-kirodistributes both new agents to everyone (consistent with copying thewhole payload); the test asserts they land.
Why
BePower's ICT Development is adopting KiroCrew alongside kiro-cli/IDE. The config repo
was designed for the IDE-human world; these agents are the first autonomous,
self-contextualizing additions (they read the target project's README/AGENTS.md/
package.json/steering at boot, following the
bepower-setuppattern).upgrade-guardiantargets the top pain point — regressions from library/framework upgrades — read-only,
so it proposes and proves rather than acting. Part of the KiroCrew PoC for 3 tech leads.
Closes #
How
bepower-setupshape:resourcesas a startup readinglist, read-only
allowedTools, scopedwrite.allowedPaths, inlinedeniedCommands.skill_search) with a thin agentwrapper — read-only rules are kept inline in the prompt so they bind before the skill
loads. Each prompt's first operative step is reading the project context.
runtimetag (lower churn than moving files); deprecated hookskept, not deleted, so archival is a human decision.
rev-engde-Ream-ified: inline Playwright MCP retained, target/auth/areas deduced.Checklist
npm run lint)npm test)npm run build)