Skip to content

fix(audit): repair kiro-cli audit integration#1215

Merged
stack72 merged 1 commit intomainfrom
fix-kiro-cli-audit
Apr 23, 2026
Merged

fix(audit): repair kiro-cli audit integration#1215
stack72 merged 1 commit intomainfrom
fix-kiro-cli-audit

Conversation

@stack72
Copy link
Copy Markdown
Contributor

@stack72 stack72 commented Apr 23, 2026

Summary

Three independent bugs prevented swamp init --tool kiro from ever recording a kiro-cli shell command to the audit log. End-to-end verified against a throwaway repo with kiro-cli chatswamp audit.

Bug 1 — normalizer dropped every real event
src/domain/audit/hook_input.ts hard-coded tool_name !== "execute_bash", but kiro-cli emits tool_name: "shell" at runtime (it's Amazon Q Developer CLI under the hood; execute_bash / execute_cmd / shell are all aliases its agent schema accepts, and shell is the canonical runtime name). Accept all three.

Bug 2 — agent config used invalid tools: ["*"]
The .kiro/agents/swamp.json template used "tools": ["*"], which fails kiro-cli schema parsing and causes silent fallback to the built-in kiro_default agent — so the swamp postToolUse audit hook never fires. Replaced with an explicit list: ["read", "write", "shell", "grep", "glob", "thinking", "todo"].

Bug 3 — no workspace default agent
swamp init --tool kiro created the hook + agent but never wrote .kiro/settings/cli.json. Without it, plain kiro-cli chat (no --agent flag) loads the default agent and skips swamp entirely. Now created with { "chat.defaultAgent": "swamp" }. If the file already exists, merge the key while preserving unrelated settings; if a non-swamp chat.defaultAgent is already set, leave it alone and log a warning.

Test Plan

  • Unit tests: added shell and execute_cmd cases to hook_input_test.ts; existing execute_bash / Kiro-IDE camelCase cases still pass
  • Integration tests: added three repo_service_test.ts cases covering all-three-files output, cli.json merge with unrelated keys preserved, and non-swamp defaultAgent preserved
  • deno check / deno lint / deno fmt --check clean
  • Full deno run test: 4709 pass (one unrelated parallel-env-var flake in src/libswamp/auth/logout_test.ts that passes in isolation; not touched here)
  • deno run compile + manual swamp init --tool kiro ./tmp — confirmed .kiro/settings/cli.json contains {"chat.defaultAgent":"swamp"} and .kiro/agents/swamp.json has the explicit tools list

Three independent bugs prevented `swamp init --tool kiro` from recording
any kiro-cli shell commands to the audit log:

1. Normalizer only accepted `tool_name: "execute_bash"`, but kiro-cli
   emits `"shell"` at runtime. Every real event was dropped. Accept
   `execute_bash`, `shell`, and `execute_cmd` — the aliases the
   kiro-cli agent schema recognizes.

2. `.kiro/agents/swamp.json` shipped with `"tools": ["*"]`, which fails
   kiro-cli schema parsing and causes silent fallback to the default
   agent — so the swamp postToolUse hook never ran. Replace with an
   explicit list: read, write, shell, grep, glob, thinking, todo.

3. `swamp init --tool kiro` did not write the workspace-scoped
   `.kiro/settings/cli.json`. Without it, plain `kiro-cli chat` (no
   --agent flag) loads the built-in agent instead of swamp. Now
   created with `{ "chat.defaultAgent": "swamp" }`, merging into any
   existing file and leaving a user's non-swamp preference untouched
   (with a warning).
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Clean, well-scoped fix for three independent bugs preventing swamp init --tool kiro from recording shell commands to the audit log. All three bugs are real, the root causes are well-understood, and the fixes are minimal and correct.

Blocking Issues

None.

Suggestions

  1. Malformed cli.json handling (repo_service.ts:1499): ensureKiroCliDefaultAgent catches Deno.errors.NotFound but will throw a raw SyntaxError if the file exists with invalid JSON. A graceful fallback (log a warning, overwrite) might be friendlier, though this is a narrow edge case.

  2. Kiro IDE camelCase test for "shell" alias (hook_input_test.ts): The new snake_case tests cover "shell" and "execute_cmd", but the Kiro IDE camelCase tests only exercise "execute_bash" as the toolName. Adding one camelCase test with toolName: "shell" would confirm the alias works through the IDE path too. The code handles it correctly — this is just test completeness.

Both are minor — neither blocks merge. The PR has good test coverage for all three bugs, correct error handling for the non-swamp-defaultAgent case, proper use of atomicWriteTextFile, and appropriate comments explaining why (the silent "*" failure, the runtime alias mismatch). DDD-wise, ensureKiroCliDefaultAgent fits naturally as a private method on RepoService alongside the existing kiro methods, and KIRO_SHELL_TOOL_NAMES is a clean domain constant.

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial Review

Critical / High

None.

Medium

  1. src/domain/repo/repo_service.ts:1498-1499 — malformed cli.json produces an unhelpful error.
    If .kiro/settings/cli.json exists but contains empty or invalid JSON (e.g. a 0-byte file left by an interrupted write, or a hand-edited file with a trailing comma), JSON.parse(content) throws SyntaxError. The catch block only handles Deno.errors.NotFound and re-throws everything else, so the user gets an opaque parse error from the guts of init/upgrade instead of a message like "cli.json is corrupt, overwriting." This is a realistic scenario for files users might hand-edit.

    Breaking example: echo "" > .kiro/settings/cli.json && swamp init --tool kiro .
    Suggested fix: Catch SyntaxError alongside NotFound, log a warning that the existing file is malformed, and fall through to re-create it (treating it the same as not-found).

Low

  1. src/domain/audit/hook_input.ts:143-144 — comment mentions executeBash (camelCase) as an alias but it's absent from KIRO_SHELL_TOOL_NAMES.
    The docstring says configs accept "executeBash" / "execute_bash" / "execute_cmd", but the Set only contains the latter two plus "shell". This is correct — executeBash is a config-level alias that doesn't appear in hook payloads — but a future contributor reading the comment might add it to the Set unnecessarily, or worse, might remove shell thinking the comment is authoritative. Consider rephrasing the comment to make the distinction between config aliases and runtime names explicit.

  2. src/domain/repo/repo_service.ts:1497-1502 — non-object JSON in cli.json silently corrupts settings.
    If cli.json contains valid JSON that isn't an object (e.g. "hello", 42, [1,2]), the spread ...existingSettings produces numeric keys from array indices or string characters. The write succeeds but the result is garbage. Extremely unlikely in practice.

  3. src/domain/repo/repo_service.ts:1497-1531 — TOCTOU between read and atomic write.
    The read-modify-write cycle on cli.json is not itself atomic. A concurrent swamp init (or user edit) between the readTextFile and atomicWriteTextFile could lose one writer's changes. Theoretical for a local dev config file that's written once during init.

Verdict

PASS — The three bug fixes are correct, well-tested, and well-scoped. The normalizer fix properly expands the tool name set, the agent config fix avoids kiro-cli's silent fallback, and the cli.json creation addresses a genuine missing integration step. The medium finding (malformed JSON) is worth hardening but doesn't block merge.

@stack72 stack72 merged commit 3db9257 into main Apr 23, 2026
10 checks passed
@stack72 stack72 deleted the fix-kiro-cli-audit branch April 23, 2026 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant