implementing-dapper-queries

refactor(bitwarden-delivery-tools): make labeling-changes the single owner of change types (#255)

feat(bitwarden-delivery-tools): extract PR conventions into their own skill (#235) * feat(bitwarden-delivery-tools): extract PR conventions into their own skill The title format, PR template body, and ai-review label were three steps inside `creating-pull-request`, which meant any other skill wanting them had to drive that workflow's numbered steps and inherit its review gate, preview, and `gh pr create`. `force-multiplier` did exactly that, walking the whole workflow at its pilot target to collect conventions it then replicated. `applying-pr-conventions` now owns those values and returns them. Both callers invoke it as a peer, so neither reaches into the other, and `creating-pull-request` loses its caller-detection protocol entirely. * refactor(bitwarden-delivery-tools): narrow the extraction to its objective Base-branch resolution and the PR title allowlist both rode along with the conventions extraction rather than belonging to it. - creating-pull-request: Step 1b, the --base guidance, and the preview's base row are out. Scoping a review to a release branch needs a base-ref parameter in bitwarden-code-reviewer first, which lives in another plugin. - references/pr-title-allowlist.md is deleted. --title "$(cat ...)" is what closes the injection, since command-substitution output is not re-parsed, and the pattern refused the conventional-commit ! breaking-change marker. The file-handoff rule survives inline at each gh pr create. - creating-pull-request: review findings are assessed with the user before any are acted on. - evals: the compound title-and-label query leaves the set, along with the description clause written to force it. Baseline re-recorded at 9/9 and 12/12.

feat(bitwarden-testing-tools): recommend test layer (#240) * feat(bitwarden-testing-tools): add recommending-test-layers skill Adds a forward-looking counterpart to assessing-test-coverage that recommends which tests a change needs and at which layer each belongs (static / unit / component / contract / integration / E2E), from a Jira key, Testmo CSV, an assessing-test-coverage report, a PR, or a feature description. It separates deterministic pre-merge gates from non-deterministic post-deploy tests and grades criticality against Bitwarden's Defect Severity Classification Guide. Tool scoping follows the repo's review conventions: gh api restricted to --method GET, no git clone, no AskUserQuestion, writes confined to ${CLAUDE_PLUGIN_DATA}. Ships an evals/ trigger harness (baseline recorded with Sonnet at 7 runs/query: 10/10 should-trigger, 9/9 should-not-trigger) so the three sibling skills do not cannibalize each other's triggers. Bumps bitwarden-testing-tools to 1.2.0 across plugin.json, marketplace.json, the root README catalog, and the plugin README, with a matching CHANGELOG entry. * test(bitwarden-testing-tools): re-baseline recommending-test-layers eval on claude-opus-4-8 Align the recommending-test-layers trigger eval with its sibling assessing-test-coverage eval, which baselines on claude-opus-4-8 (also the runner's default). Regenerate baseline.json on opus instead of the prior sonnet run and update the README's documented model. Verdicts are unchanged: 10/10 should-trigger, 9/9 should-not-trigger. * fix(bitwarden-testing-tools): address recommending-test-layers review findings Scope the skill description away from abstract testing-concept questions, trim redundant trigger phrasings, and drop the unused gh api tool grant. Reword step 2 so it marks unverifiable coverage rather than gating on it. Harden both eval runners' read-only carve-out (gh api repos/ only, treat > as a chain), record the first skill of any kind to expose sibling-first triggers, and persist timeouts plus model/runs/timestamp so baselines are self-describing. Re-baseline recommending-test-layers at 10/10 and 10/10, and narrow the changelog cannibalization claim to the two measured legs. * fix(bitwarden-testing-tools): correct recommending-test-layers doc mismatches Address code-review findings on the unreleased 1.2.0 skill: - evals/README.md: 9 -> 10 should-not-trigger queries (matches trigger-eval.json) - SKILL.md: add 'unverified' to the Existing coverage legend, matching step 2 * fix(bitwarden-testing-tools): close gh api write-bypass in eval read-only guard The `gh api repos/` read-only carve-out in both trigger-eval harnesses only bounded the target path, not the HTTP method. `gh` accepts the endpoint as the first positional with `-X`/`--method` after it, so `gh api repos/O/R/issues -X POST` matched the prefix, tripped no SHELL_CHAINS operator, and was waved through instead of bailed — letting an adversarial should-not-trigger run execute a live state-changing API call the guard was meant to prevent (CWE-183/CWE-77). Replace the inline prefix check with `is_read_only_bash()`, which also rejects a non-GET `-X`/`--method` or any request-body flag, and correct the comment that overstated the guarantee. Both harnesses stay byte-identical modulo the skill token. Re-record the assessing-test-coverage baseline against the updated harness output schema (model/runs_per_query/recorded_utc, per-result timeouts/first_skills) so its committed baseline matches the code it runs. 10/10 and 10/10, no regression. * fix(bitwarden-testing-tools): harden eval read-only guard against joined gh flags Address PR #240 review findings on both eval runners: - Normalize gh flag tokens so the read-only carve-out bails on write requests spelled with joined or =-attached forms (-XPOST, --method=POST, --field=k=v), not just the space-separated spelling. - Add "<" to SHELL_CHAINS so process substitution (gh pr view unit -> component -> contract) instead of being driven by the Defect Severity Guide. Criticality is demoted to a post-deploy-only input that decides which critical journeys earn E2E or smoke, resolving the review finding that made `integration` unreachable. Adopt minimumCD's full taxonomy (adds smoke, synthetic monitoring, exploratory; renames acceptance->E2E, E2E smoke->smoke), replace the Gates-deploy column with Pipeline role, and update the README row and 1.2.0 changelog text to match. * fix(bitwarden-testing-tools): resolve recommending-test-layers layer/gate contradictions Route integration, synthetic monitoring, and exploratory via explicit triggers so all advertised layers are reachable; scope the deterministic-gate rule to the pre-merge gate and carve out E2E promotion gating; reconcile smoke rollback wording; scope criticality to journeys with an n/a value for non-journey behaviors; map the assessing-test-coverage taxonomy onto this skill's layers; align README and CHANGELOG. * fix(bitwarden-testing-tools): resolve recommending-test-layers review contradictions Address code-review findings on recommending-test-layers/SKILL.md: - Step 4 no longer says recommend 'one' post-deploy layer; states a top-band journey can earn smoke + E2E (+ synthetic monitoring). - E2E now owns end-to-end proof of the deployed top-band journey, not 'load/resilience/compliance' that no trigger routes to it. - Acceptance criteria stay owned 1:1 by the component layer; E2E proves the assembled artifact, removing the component/E2E overlap. - Cross-reference the promotion-stage carve-out at the deterministic pre-merge-gate rule so E2E gating reads as deliberate, not contradictory. - Reword the deterministic ladder so component is the center of gravity and contract is a concern-specific choice, not a rung above component. * fix(bitwarden-testing-tools): resolve recommending-test-layers contradictions Align synthetic-monitoring trigger with the layer table, README, and CHANGELOG (any SLO-bound journey, not just top-band); add the missing unit->static/unit taxonomy mapping in step 2; state the component-vs-unit tie-breaker in step 3; and let the Recommendations template record a behavior earning multiple layers via one row per layer. * refactor(bitwarden-testing-tools): debloat recommending-test-layers skill Align verbosity and directiveness with the sibling assessing-test-coverage skill: cut concept explanations the model already knows, remove rules that were restated up to five times, and drop operational over-specification. The merge-gate-vs-promotion-gate distinction is now stated once, resolving the apparent self-contradiction. Objective, layer table, and output template are preserved. * refactor(bitwarden-testing-tools): tighten recommending-test-layers step 3 and gotchas Trim step 3 to a single decision rule and condense the component-scope and E2E-gating gotchas. * test(bitwarden-testing-tools): re-baseline recommending-test-layers eval after description change Re-recorded baseline.json on claude-opus-4-8 at 7 runs/query following the skill's description update (added smoke/synthetic monitoring/exploratory layers, revised trigger phrasings). Clean run: 10/10 should-trigger, 10/10 should-not-trigger, no timeouts, no PASS/FAIL regressions. Bumped the recorded date in the eval README to match. * test(bitwarden-testing-tools): re-baseline assessing-test-coverage eval after sibling description change Re-recorded baseline.json on claude-opus-4-8 at 7 runs/query following the recommending-test-layers description edits (widened layer list, narrowed "pyramid or trophy for this change" trigger), per the cross-skill re-run rule in evals/README.md. Clean run: 10/10 should-trigger, 10/10 should-not-trigger, no timeouts, no PASS/FAIL regressions. The 'testing trophy or pyramid' near-miss still routes 7/7 to recommending-test-layers with no cannibalization. Bumped the recorded date in the eval README to match. * fix(bitwarden-testing-tools): make eval shell-chain guard quote-aware Both trigger-eval harnesses detected shell chaining with a raw substring scan over SHELL_CHAINS, so the '' entries also matched inside quoted arguments: a read-only 'gh search code "Task"' was misclassified as a shell chain, the child was killed, and a legitimate mid-execution skill run was recorded as a real-work bail. Bails are indistinguishable from clean non-triggers in trigger_rate, the value the regression gate keys on. Replace the substring scan with a quote-aware shlex tokenizer (non-posix + punctuation_chars) so only bare unquoted operator tokens count; command substitution and newlines are still matched on the raw string, deliberately quote-blind since "$(...)" executes in a real shell. Track a per-query 'bails' count so a harness bail is greppable during triage rather than folded silently into the non-trigger count (it stays in the denominator: a genuine real-work bail is a legitimate non-trigger). Re-recorded both baseline.json on claude-opus-4-8 at 7 runs/query: 10/10 should-trigger, 10/10 should-not-trigger, no verdict changes. Residual bails on should-trigger queries now trace to genuine chains ('... | head') and the known gh-pr-diff / leading-slash allowlist gaps, not the '' false positive. Addresses the Major finding in the PR #240 validation review. Co-authored-by: Claude * fix(bitwarden-testing-tools): stop eval read-only guard over-bailing genuine reads Address three over-bail edge cases in both skills' run_real_eval.py harnesses, plus a cspell ordering nit: - Detect command substitution quote-aware: a single-quoted '$(...)' literal is left alone (a real shell suppresses it), while double-quoted "$(...)" and backticks still bail. - Match only a short flag's leading letter, so a benign cluster like -sf is no longer misread as the write flag -f (every write short flag takes a value and must lead its cluster). - Restore the broad `gh api` carve-out so gh api graphql, gh api /repos/..., and gh api user count as reads again; the method/body flag guard, not the prefix, enforces read-only. - Sort "shlex" into place in .cspell.json. Folded into the unreleased 1.2.0 changelog entry; baselines unaffected (changes only reduce spurious bails). * refactor(bitwarden-testing-tools): simplify eval harnesses, switch to sonnet Align both trigger-eval runners with the bitwarden-delivery-tools pattern: remove the ~95-line quote-aware read-only-bash guard and real-work bail path in favor of detecting the target skill token as it streams into a Skill/Read tool input. Contain the adversarial should-not-trigger queries with --allowedTools Skill Read so they cannot clone repos or run build/test toolchains, and bound anything that stalls with --timeout. Fix the memory leak: each claude -p is a ~1GB Node process with its own child tree, and process.kill() reaped only the parent, orphaning the rest until a full run exhausted memory. Each subprocess now runs in its own session and is reaped as a process group on trigger, completion, or timeout, and the default --num-workers drops from 5 to 3 to cap peak memory. Default both harnesses to --model claude-sonnet-5 and re-record baselines on that model (recommending 10/10 + 10/10, assessing 9/10 + 10/10). One assessing should-trigger phrasing sits below threshold on sonnet where it fired 7/7 on opus; recorded honestly in the baseline and documented as a known gap. * refactor(bitwarden-testing-tools): consolidate eval runners into shared engine The assessing-test-coverage and recommending-test-layers skills each shipped a byte-identical copy of run_real_eval.py differing only in a hardcoded skill token, so every engine fix had to be hand-applied to both. Move the runner to a single plugin-level evals/run_real_eval.py and resolve the target token from --skill, or infer it from the eval-set path when omitted. Each skill's evals/ now holds only its data (trigger-eval.json, baseline.json) and a README, so adding evals for a new skill no longer requires copying the engine. Runtime behavior and the recorded baselines are unchanged. Bump to 1.2.1 and remove stray review artifacts from the eval dirs. * test(bitwarden-testing-tools): re-baseline trigger evals on 1.2.1 skills Re-run both trigger-eval suites against the reinstalled 1.2.1 plugin (sonnet-5, 7 runs/query) after the eval-engine consolidation and description tightening on this branch. One dual-intent query per skill slips from 4/7 to 3/7, straddling the 0.5 threshold as it splits samples with its sibling skill; recorded in each baseline and documented as a known gap in the eval READMEs. * fix(bitwarden-testing-tools): address PR review findings - run_real_eval.py: parse the final stdout chunk on process exit so a trigger in the last read (or a last event without a trailing newline) is no longer dropped as a false negative - recommending-test-layers: enumerate severity bands inline (Critical/ High/Medium/Low) so the guide-unreachable fallback has a vocabulary - recommending-test-layers: drop "post-deploy" from the step-4 heading so it no longer contradicts E2E gating production promotion - recommending-test-layers: remove unused Grep/Glob from allowed-tools - README: note the step-4 Confluence severity-guide use for any input - .cspell.json: drop dead pflag/shlex entries * fix(bitwarden-testing-tools): restore SKILL.md trigger guard and untangle sibling triggers Address the major findings from the PR #240 validation review: - Restore the SKILL.md guard the per-skill runners had. The shared engine matched a bare skill-token substring, so an exploratory Read of the skill's own evals/ directory (the eval's CWD) counted as a trigger. Detection now requires a plugin-qualified Skill invocation (:) or a Read of the skill's own SKILL.md, on both the streaming and assistant paths. The plugin token is resolved from --plugin or inferred from the eval-set path. - Tighten the recommending-test-layers description so its exclusion clause disclaims layer-worded inventory phrasings, which were cannibalizing the sibling assessing-test-coverage skill's dual-intent triggers. - Re-record both baselines with the tightened runner and description. The sibling's "which layers do they land at" query recovers from below threshold to a pass; correct the 1.2.1 changelog entry that claimed baselines were unchanged. * docs(bitwarden-testing-tools): trim run_real_eval docstring to a README pointer The module docstring restated the conceptual overview already in evals/README.md (why a custom runner, what counts as a trigger, token inference). Collapse it to a short pointer so the README is the single source of truth and the two stop drifting apart. Inline comments left intact since they explain code-local decisions. * docs(bitwarden-testing-tools): consolidate eval READMEs and update changelog Fold the two per-skill eval READMEs into the shared evals/README.md, correct the trigger-detection wording to match the plugin-qualified matching, document the --plugin flag, and record the changelog entries for the consolidation and timeout-accounting fixes. * docs(bitwarden-testing-tools): record baseline model and align eval docs Add a top-level model field to both baseline.json files recording the model each was captured on, so a re-run can match it via --model. Update the shared evals/README.md to point runners at that field, drop the stale recommending-test-layers below-threshold entry now that it passes 10/10, and correct the runner line count in the changelog. * docs(bitwarden-testing-tools): collapse unreleased changelog to a single 1.2.0 entry 1.2.0 never shipped (main is 1.1.0), so shipping both 1.2.0 and 1.2.1 left the released history documenting intermediate states (bare-token detection, two per-skill runners) that 1.2.1 undid within the same PR. Collapse into one 1.2.0 entry describing only the net change from 1.1.0, and renumber 1.2.1 -> 1.2.0 across plugin.json, marketplace.json, and the README catalog. Also fix eval-doc drift and dead code flagged in review: - evals/README.md: correct the 'Why a custom runner' line that still described bare-token detection, and note a timed-out run records as a non-trigger with a stderr warning. - run_real_eval.py: drop the dead 'triggered' local (always False) in favor of the literal on the non-trigger return paths. * fix(bitwarden-testing-tools): record model in eval summary output The shared eval runner never emitted the model into its JSON summary, so a baseline updated by copying a run output dropped the top-level model field the Arguments section tells the next runner to match. Write model into the summary, add it to the regression-check projection so a diff against a baseline recorded on a different model fails, and drop the now-redundant manual model-recording step from the README. * docs(bitwarden-testing-tools): baseline trigger evals on claude-opus-4-8 Switch the eval runner default to --model claude-opus-4-8, the model most users run, and re-record both baseline.json files on it. Both skills pass every eval query at 10/10 should-trigger and 10/10 should-not-trigger. Update evals/README.md and the plugin README to match, and trim the 1.2.0 changelog eval entries to the end state (1.2.0 is unreleased, so the sonnet-to-opus switching history is not relevant to a reader). * docs(bitwarden-testing-tools): debloat 1.2.0 changelog entries * fix(bitwarden-testing-tools): address PR #240 review findings - Gate mis-placement flag (step 5) on inputs that reveal current test placement; skip inference for bare Jira keys and feature descriptions - Drop drifting layer enumeration from the skill description - Collapse the thrice-stated trigger-detection rule in evals/README to a single statement, restore the run-before-merge gate rule - Distinguish a crashed claude child (non-zero exit) from a genuine non-trigger in run_real_eval.py so partial error rates don't silently depress results - Correct changelog wording (shared harness, not a per-skill harness) * test(bitwarden-testing-tools): re-record eval baselines on head (a4d0b83) Re-ran both trigger evals at --runs-per-query 7 --model claude-opus-4-8 against the merged description, resolving the PR #240 finding that the baselines were recorded before the description trim in a4d0b83. - recommending-test-layers: 10/10 / 10/10, byte-identical to the prior baseline (the description trim was inert), so no file change - assessing-test-coverage: 10/10 / 10/10; one should-trigger query drifted 7/7 -> 6/7 (rate 0.857, verdict unchanged), recorded here Both diffed clean against the prior baselines (no verdict flips). * fix(bitwarden-testing-tools): address SaintPatrck's PR #240 review - Step 4: phrase the severity guide fetch as a directive and stop echoing its band names, so the skill cannot drift from the live page - Step 2: drop the assessing-test-coverage taxonomy mapping details - Step 1: describe what to read from a PR rather than which commands to run - Remove the redundant References section * fix(bitwarden-testing-tools): restore Glob/Grep so recommending-test-layers can locate inputs --------- Co-authored-by: Claude

feat(bitwarden-ai-telemetry): attach the signed-in user's email to hook records (#254)

build: pin pnpm through packageManager only (#253)

feat(bitwarden-ai-telemetry): warn when records are not reaching the collector (#250)

fix(bitwarden-ai-telemetry): derive git context from the edited file (#249)

[deps]: Update pnpm to v11.11.0 [SECURITY] (#224)

fix(bitwarden-atlassian-tools): declare the optional write token optional (#248)

fix(bitwarden-atlassian-tools): start the MCP server on Node releases without Corepack (#246)

feat(bitwarden-security-engineer): transition scanner triage from Checkmarx/SonarCloud/Grype to Aikido (#226) * feat(bitwarden-security-engineer): transition scanner triage from Checkmarx/SonarCloud/Grype to Aikido Bitwarden has moved SAST, IaC, SCA, secrets, cloud, container, malware, EOL, and license scanning onto Aikido as a single unified feed. Update the plugin's triage and dependency-review skills, the agent, and perform-security-review's evidence gathering to match, and bump to 1.4.0. - triaging-security-findings: query findings via the aikido:issues skill instead of the GHAS code-scanning API; triage now flows through Jira (see VULN-665) rather than tool-specific dismissal states; documents the Jira comment format, severity mapping, and the group-scoped-action verification trap - reviewing-dependencies: Aikido replaces Grype for container/SCA scanning - perform-security-review: scan-evidence step now sources SAST/IaC findings from Aikido instead of the now-empty GHAS code-scanning endpoint - agent + READMEs updated to reflect the new toolchain * fix(bitwarden-security-engineer): address pre-PR review findings on Aikido transition - Add the resolved Aikido MCP tool to perform-security-review's allowed-tools so the new scan-evidence step can actually reach it instead of silently returning nothing - Restore a False Positive Protocol section to triaging-security-findings (data-flow tracing, sanitizer-vs-validator, full lifecycle, documented rationale) dropped during the Aikido rewrite - Stop grouping the cloud_instance Aikido issue type under "container images" in reviewing-dependencies — it's a distinct cloud VM/instance category - Document the aikido plugin as a prerequisite in the plugin README - Fix grammar and add a read pointer for the VULN-665 reference * Update plugins/bitwarden-security-engineer/README.md Co-authored-by: claude[bot] * Update README.md Co-authored-by: claude[bot] * fix(bitwarden-security-engineer): address PR review findings on Aikido transition Addresses withinfocus's PR review: inlines the Jira triage rubric into triaging-security-findings instead of pointing to a mutable Jira ticket, corrects the severity mapping's Hotfix/Critical row, adds fallback guidance when the aikido:issues skill is unavailable, trims stale Grype-comparison language, and compresses/re-categorizes the CHANGELOG entry per Keep a Changelog conventions. * fix(bitwarden-security-engineer): address remaining PR review findings on Aikido transition Grants the agent the Aikido MCP tool it was missing so triaging-security-findings and reviewing-dependencies can reach the feed when routed through it, updates the README skills table off the retired Checkmarx/SonarCloud/Grype/GHAS descriptions, and gives perform-security-review's Aikido evidence the same outcome vocabulary as its other scanners so a missing session can't read as a clean scan. Bumps the plugin to 2.1.0 and moves the Aikido-transition changelog entries out of the already-released 1.4.0 section into their own. * docs(bitwarden-security-engineer): clarify usage examples and aikido install path The Usage blocks followed "Install the plugin and invoke the agent:" with no label, so they read as commands to run rather than example prompts. Reframe the lead-in the way the other plugin READMEs do. Prerequisites also pointed `aikido` installs at the `claude-plugins-official` marketplace. Per PR #228 the plugin is pinned as an external entry in this marketplace instead, so name the upstream author and give the actual install command. Depends on #228 landing for that command to resolve. Co-Authored-By: Claude Opus 5 (1M context) * fix(bitwarden-security-engineer): correct aikido plugin install source in README The aikido plugin ships from the claude-plugins-official marketplace, not as a pinned entry in bitwarden-marketplace. Corrects the install command and changelog entry accordingly. * fix(bitwarden-security-engineer): give Agent 2 pre-fetched Aikido SCA/container evidence perform-security-review barred Agent 2 from calling aikido:issues live while also telling it to invoke reviewing-dependencies, whose Aikido data source is that same skill. Step 1-C now pre-fetches open_source and docker_container findings alongside SAST/IaC so Agent 2 has the data through the SCAN_EVIDENCE block instead. * fix(bitwarden-security-engineer): resolve remaining Aikido PR review findings - Disambiguate empty response from None/Not available in perform-security-review scan-evidence rules; key Not available to failed/unauthenticated responses - Reflect Aikido SCA/container evidence (not just SAST/IaC) in README skills table and Prerequisites for perform-security-review * fix(bitwarden-security-engineer): address second-round Aikido PR review findings - Reword agent post-fix verification to report the Jira update for an engineer to apply, rather than confirm a ticket it has no tool to reach - Restore the released 2.0.0 CHANGELOG entry verbatim; keep the Aikido step-1-C wording in the 2.1.0 entry - Name the Aikido MCP tool alongside Skill(aikido:issues) in the perform-security-review sub-agent constraints and reviewing-dependencies, so a sub-agent holding the grant cannot re-fetch pre-provided evidence - Update tool-grants.md to match step 1-C: scope the prompt/deny wording to the two gh api scanners, and argue the Aikido MCP grant's safety from exact-name tool scoping (aikido_issues_list only, not the aikido_ignore_issue mutation) - Dedupe the CVSS convention: state it in triaging-security-findings, point there from reviewing-dependencies - Pin the API version header on triaging-security-findings gh api examples for stable --jq output - Condense the 2.1.0 CHANGELOG entry * Update plugins/bitwarden-security-engineer/skills/triaging-security-findings/SKILL.md Co-authored-by: claude[bot] * Update plugins/bitwarden-security-engineer/skills/reviewing-dependencies/SKILL.md Co-authored-by: Patrick Honkonen * fix(bitwarden-security-engineer): remove unactionable Aikido grant rationale section SaintPatrck flagged the "Why the Aikido MCP grant is pre-approved" section in tool-grants.md as reading like a defense of a review comment rather than actionable guidance. Removes it and the now-dangling cross-reference, and drops the matching CHANGELOG mention. --------- Co-authored-by: claude[bot] Co-authored-by: Graham Walker Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: Patrick Honkonen

[deps]: Update npm minor (#242)

[deps]: Lock file maintenance (#186)

[deps]: Update vitest to v4.1.11 [SECURITY] (#241) Co-authored-by: renovate[bot]

[AI-52] feat: Trigger external plugin pin audits in CI (#230)

feat(bitwarden-code-review): recognize confirmed stack layers when validating findings (#219) A stacked PR is reviewed against the layer below it, so a lower layer legitimately adds exports and types whose callers land above it. The rejection criteria assumed a self-contained diff, which turns that design into dead-code findings on every bottom layer. Detection is gated rather than inferred from the base ref alone: Bitwarden runs release branches with cherry-picks, so rc, hotfix-rc, release/*, and develop all have a non-default base while being ordinary self-contained changes. Confirming a layer needs a corroborating signal, and the relaxation never covers a layer that cannot build alone or a security or correctness defect in code the layer does contain.

fix: resolve the diff base from origin/HEAD instead of assuming main (#217) Local-mode code review and branch-comparison security review both diffed against a literal `main`, which is the wrong base in any repo whose default branch is named something else. Both now resolve `origin/HEAD` and ask when it cannot be resolved, and both gained the `git symbolic-ref` grant needed to run that resolution.

[AI-52] feat: Add auditing-external-claude-plugins skill (#227) * [AI-52] feat: Add auditing-external-claude-plugins skill Security audit of a third-party Claude Code plugin at a pinned commit, before it is vendored into this marketplace. Covers the plugin manifest, MCP server configuration, bundled or runtime-fetched dependencies, tool permission scope, failure-mode behavior of network-dependent checks, prompt-injection surface, secrets, and license. Runs as an isolated bitwarden-security-engineer subagent on the fable model, blocking, so it can be triggered manually or by a CI workflow that posts the report as a PR review comment. Ships a bundled gather-evidence.sh script so allowed-tools can pre-approve one script invocation instead of enumerating every git/npm/tar/hash command, keeping the skill runnable unattended. Bumps bitwarden-security-engineer 1.3.0 -> 1.4.0 (minor, additive skill). * Trim changelog entry * Trim README skill table entry * Revert .gitignore change * Address PR review: validate repo URL, guard tar/openssl failures, pin js-beautify - Reject any $repo_url that isn't https:// or git@ before cloning, and add -- separators on clone/checkout, closing an ext:: remote-helper RCE and option-injection path via a leading-dash argument. - Guard the openssl hash and tar extraction so a failure (including GNU tar refusing a path-traversal member) emits an explicit marker instead of aborting the script under set -e before the scratch path is printed. - Pin js-beautify to the exact version already used successfully in manual dry runs; the bare @* wildcard let npm's alias/tarball spec syntax resolve to an arbitrary package. * Address validate-ai findings: failure markers, arg guard, npm arg injection, output path, cleanup - gather-evidence.sh: guard against missing arguments; emit NPM_VIEW_FAILED, NPM_PACK_FAILED, ATTESTATIONS_UNAVAILABLE, and MULTIPLE_NPM_PACKAGES_DETECTED markers instead of failing silently with || true; add -- before the package spec on npm view/pack so a dash-leading spec can't be read as a flag (verified: without --, npm silently mis-parses it; with --, it's rejected as an invalid package name instead). npm flags must stay before -- or npm also swallows them as positional args, including --registry. - scratch dir now uses a fixed, predictable /tmp/plugin-audit.XXXXXXXX prefix instead of a bare mktemp -d, so cleanup can be scoped to it specifically. - SKILL.md: any stderr marker must be named in the report's Not done field, not read as a clean result; default OUTPUT_FILE moves to /plugin-audits/ instead of a repo-visible path; js-beautify's grant is now tied to an explicit step (deobfuscating a minified server entry point); added an unconditional final cleanup step removing the scratch directory, matching perform-security-review's precedent in this same plugin; README gets a usage example for the skill's positional-argument invocation. Tar/symlink extraction and the ext:: remote-helper claims from the same review round were independently tested against this environment's tar and git and did not reproduce; not changed. The npm-registry attestations @-encoding claim also tested identically either way; not changed. * [AI-52] fix: Scope allowed-tools to used commands, move js-beautify behind a script Removes the gh api --method GET allowed-tools grant: gh api is never invoked by this skill or gather-evidence.sh, only warned against as a mutation to avoid. Moves the js-beautify invocation into a new beautify.sh script, mirroring gather-evidence.sh. The version now lives in a scoped package.json/lockfile that Renovate's npm manager already tracks, instead of a string in SKILL.md's frontmatter that could only be bumped by hand. Uses npm rather than pnpm since npm is already a hard dependency of this skill (gather-evidence.sh shells out to it), while pnpm/corepack availability in the skill's actual CI execution environment (a reusable workflow in bitwarden/gh-actions) can't be verified from this repo. * [AI-52] fix: Scope scratch-directory deletion to a validating script Bash(rm -rf /tmp/plugin-audit.*) is a literal prefix match on the whole command string, so extra paths appended to the same invocation (e.g. rm -rf /tmp/plugin-audit.ab12cd34 ~/src) would also match and run unprompted. That matters here specifically: this skill treats the cloned repo as adversarial, and step 7 has the agent compose the rm invocation from a path it derives after running that untrusted content through several tools. Moves the deletion into cleanup.sh, mirroring the beautify.sh remedy shape. It accepts exactly one argument and refuses to delete anything that doesn't match gather-evidence.sh's exact mktemp pattern (/tmp/plugin-audit.XXXXXXXX), so an appended or malformed path is rejected by the script instead of pre-approved by the allowed-tools rule. * [AI-52] fix: Bump js-beautify to latest (2.0.3) Verified the beautify.sh CLI usage (js-beautify -o ) behaves identically on 2.0.3 against a sample minified file before bumping. * [AI-52] fix: Default beautify.sh output alongside its input, not the caller's cwd An unspecified output path previously resolved to ./pretty.js relative to whatever directory the agent happened to be in, so the de-minified copy of a third-party bundle landed in the working repo instead of the scratch directory step 7 cleans up. Defaulting to the input file's own directory keeps it inside that scratch directory. * [AI-52] refactor: Drop the scratch-directory cleanup step Deleting the clone buys nothing on an ephemeral CI runner, and locally it leaves only inert files in /tmp. That payoff doesn't justify a third script, a wildcard allowed-tools grant, and a path guard that has to hold against adversarial input. The case pattern it used matched separators anyway, so /tmp/plugin-audit.../../.. passed validation and resolved to /. * [AI-52] fix: Neutralize symlinks in the audited clone and tarball The audit tells the agent to read every file in an untrusted tree, report any secret it finds with the value, and the caller posts that report to a public PR comment. A committed symlink turns that into an exfiltration primitive with no prompt injection required: docs/notes.txt pointing at ../../../.git/config makes the agent read the runner's GITHUB_TOKEN and dutifully publish it, because every component behaved as designed. Cloning with core.symlinks=false writes each committed symlink as a plain file holding its target path, and the config persists into the repo so the detached checkout stays safe too. Tarball members get recorded and deleted, since tar refuses to write through an existing symlink but will happily create one pointing anywhere. Both inventories land in symlinks.txt, where a target outside the tree is itself a finding. * [AI-52] refactor: Stop enumerating gather-evidence.sh's stderr markers The script's own messages already say what evidence is missing, so listing the marker names here only couples SKILL.md to the script: a new marker means editing both files, and a forgotten one reads as an approved omission. NO_NPM_PACKAGE_DETECTED stays named because it calls for extra manual work rather than just a Not done entry. * [AI-52] refactor: Have gather-evidence.sh report its own inventory SKILL.md carried a prose copy of the script's output layout, which only the script knows at runtime and which drifts the moment either side changes. The script now ends with an inventory: SCRATCH_DIR as a named key rather than a positional last line, one row per artifact that exists, and a NOT COLLECTED list of the markers it emitted. SKILL.md keeps the judgment that does not belong in a shell script, namely that a NOT COLLECTED entry goes in Not done and an escaping symlink target is a finding. Every label in the inventory is a fixed string or a count. Nothing derived from the audited repo is echoed there, because script output reads as more trustworthy to the agent than the file contents it describes. A row is printed only for a non-empty file, and a failed npm view now has its error-object output removed, so a fetch that failed cannot leave behind something a later read mistakes for registry metadata. Also moves the URL validation above mktemp and adds an EXIT trap, so a rejected URL creates no scratch directory and a mid-script failure removes the partial clone instead of orphaning adversarial content that the caller never learned the path to. * [AI-52] fix: Fail the attestation fetch closed on a missing attestation A 404 from the attestations endpoint means the package is unsigned, which is a security-relevant answer rather than a transport error. Without -f curl exited 0 and saved {"error":"Not found"} to attestations.json, so no marker fired and the agent was handed a file that reads as attestation data. The inventory would then vouch for it as collected evidence. -f makes that a failure, the file is removed so nothing can be mistaken for provenance, and the marker says to treat the package as unsigned unless proven otherwise. --max-time bounds a stalled connection, which previously could hang the audit with no ceiling. * [AI-52] fix: Keep credential values out of the audit report The report is published verbatim to a public pull request comment, and the skill previously told the agent to find hardcoded secrets and describe each finding concretely, with nothing saying the value itself must stay out. A secret the audited plugin committed would be disclosed by us on discovery, and one belonging to the auditing machine would be exfiltrated by the same sentence that asked for it. States the publication boundary next to the existing untrusted-data boundary, since both govern everything the agent does rather than any one step, and restates it in the Where field where the instinct to paste a token actually lands. Location, kind, and a SHA-256 prefix keep a finding verifiable by a human without republishing the credential. * [AI-52] fix: Treat a failed npm audit as a missing report, not a clean one npm audit exits non-zero both when it finds vulnerabilities, which is a real report, and when it fails outright, in which case it writes an error object to stdout. The exit status cannot tell those apart, so a registry or proxy failure landed that error object in audit.json, where a non-empty file read as collected evidence and no marker fired. An audit that never ran was presented as an audit that found nothing. Keys off auditReportVersion, which only a real report carries, so a vulnerabilities-found report survives while a failure removes the file and raises the marker. That also subsumes the missing-lockfile case, where audit never ran and the file was never created, which is why there is now one message for both rather than two that ask for the same action.

Update release notes skill (#223) * Update release notes skill Improve the release notes writing skill to account for already shipped work that is having its feature flag enabled for the first time with this release. * fix for refactor flags * Address review feedback

Add jira write tool to write comments (#216) * AI-49 add jira write tool to write comments * fixing issues based on comments * updating based on comments * fixing addComment instances * fixing file names * adding end to end test

fix(bitwarden-code-review): grant git status where a local review is told to run it (#220) `code-review-local` instructs the reviewer to inspect the working tree with `git status`, and its README documents that behavior, but neither the command nor the agent listed the grant — so the call prompts or fails on the local path while the multi-agent path, which already grants it, works.

fix(claude-config-validator): close three false negatives in security-scan.sh (#203) * fix(claude-config-validator): close three false negatives in check 1 and the permission checks Rebased onto the current #201, which absorbed the secret-filter and verdict work this branch previously carried. #201 anchors placeholder patterns to value position and adds the INCOMPLETE verdict with exit 2, both better than what was here, so only the gaps it does not cover remain. Check 1 still piped git ls-files into grep -q. Under pipefail a match makes grep exit first, git dies of SIGPIPE with 141, and the pipeline reports failure, so a committed settings.local.json read as absent. Reproduced against #201 at 20k tracked files. It also ran without -C, so the check read whatever repository the shell was in and reported a pass for a target outside one. The two permission checks still printed "(OK)" when there was no settings.json to read, which #201's own skipped accounting then did not see. Add security-scan.test.sh. Eight of its fifteen assertions fail against #201 as it stands. Credential fixtures are assembled at run time so the file holds no string a secret scanner should flag, which keeps the scanner's exclusion list from having to grow to accommodate its own tests. * fix(claude-config-validator): answer the open review threads on the scanner An absent settings.json is a determinate answer rather than a check that could not run: no file means no rules are set in the scanned directory. Marking it skipped drove a common .claude shape to INCOMPLETE and exit 2, overstating the doubt in the direction opposite to the false negatives this branch closes. It reports the result plainly now, and the README states that rules reaching the session from elsewhere are outside what the scan sees. Correct the README sentence saying the settings.local.json check does not report itself as skipped, which check 1 now does outside a git repository. Make the clean-configuration case jq-aware. Check 3's rule tests need jq, so without it the run is legitimately incomplete and the old assertion failed on any machine lacking it, which contradicted the no-new-dependency claim. Document the suite and how to run it in the scanner section, including that nothing runs it automatically. * fix(claude-config-validator): make case 1 actually reproduce the pipeline failure The fillers sat in the repository root, outside the scanned directory, so with -C and a pathspec git wrote one line and the pipe never filled. The case caught the -C fix but not the no-pipe half: reintroducing `| grep -q` still passed. Moving the fillers under the scanned directory and matching the pathspec puts roughly 500 KB past the 64 KB buffer, and reintroducing the pipe now fails the case. Dropping xargs -n1 collapses 20k process spawns, so the suite runs in about six seconds. security-patterns.md documented check 1 as the pre-fix command, teaching the false pass this branch removes and contradicting the file's own note that only the check 3 and check 4 blocks mirror the script. The manual form now guards the repository first and asks git with a pathspec, and the duplicated script block points at the shipped script instead. * chore(claude-config-validator): renumber to 2.0.2 above the merged 2.0.1 marketplace.json, plugin.json, and the README catalog row, with the changelog section retitled so it sits above #208's entry rather than replacing it. * fix(claude-config-validator): scope check 1 to the repository, not the scanned subtree The -C fix targeted CLAUDE_DIR, which narrowed the check from the repository to the .claude subtree: a committed settings.local.json anywhere else reported as absent, and the paths listed no longer matched the remediation printed below them. Reproduced with apps/foo/.claude/settings.local.json in a monorepo. It resolves the scanned directory's worktree root first, which keeps the original coverage while still targeting the target's repository rather than the shell's. Route an ls-files failure to skipped rather than the OK branch, so a genuine error cannot read as a pass. Give case 5 a positive assertion. Built only from reject assertions, it passed whenever the scanner produced no output at all, including an early exit, which made it the one case where a broken scanner still showed green. Add a case for the repository-wide coverage; reverting to subtree scope fails it. * fix(claude-config-validator): widen the documented check to the worktree root 5f5d126 widened the script but left the reference's manual command asking git from .claude, so a human following it on a monorepo got the pass this branch removes. Same drift class as the README sentence and the Automated Detection block already corrected here, in a third place. Verified against a monorepo fixture: the old form returns nothing, the new form returns apps/foo/.claude/settings.local.json.

feat(bitwarden-code-review): route changed SKILL.md to plugin-dev:skill-reviewer (#206) * feat(bitwarden-code-review): route changed SKILL.md to plugin-dev:skill-reviewer claude-config-validator 2.0.0 declines SKILL.md, and neither review path invoked plugin-dev:skill-reviewer, so skill changes were left uncovered while both paths still advertised progressive-disclosure review. The multi-agent pipeline gains a conditional Agent 5 on the plugin-dev:skill-reviewer subagent type, emitting source_agent "skill" with id prefix skl. The single-agent reviewer gains Task, scoped by its definition to that one delegation. * fix(bitwarden-code-review): detect skill support files under references/ Nine of this repo's ten skill support directories are references/, so enumerating only reference/ dropped them from Claude-configuration detection. * fix(bitwarden-code-review): record a skipped skill review instead of omitting silently SKILL.md sits outside the Claude-configuration bucket, so a missing plugin-dev leaves no agent applying the skill lens. The report gains a Not covered line so the omission cannot read as a pass. * fix(bitwarden-code-review): reconcile Agent 5 with the skill-reviewer contract plugin-dev:skill-reviewer holds Read, Grep, Glob and emits a fixed prose report, so it can satisfy neither the Review Rules bundle nor the Finding Shape schema. The orchestrator now translates its report in Step 3 under a named carve-out. SKILL.md also stays in the scope handed to reviewing-claude-config: its credential scan runs over every file whatever the type, and only its routing declines the file, so dropping it from that bucket had removed the secret scan. Task cannot be scoped per subagent in frontmatter, so the limit is documented as a convention rather than a boundary. * fix(bitwarden-code-review): drop the Task grant and tighten Agent 5's contract A spawned subagent takes its tools from its own definition, so Task put unrestricted Bash one delegation from an agent that reads contributor diffs unattended in CI. The single-agent path now states that skill content review did not run and names the pipeline that covers it. Agent 5 gains a scope fence, keeps Tool Discipline minus its gh/git bullet, takes Untrusted Input Boundary widened from hunks to whole files, harvests the reviewer's section-level issue lists, and is classified against the pipeline's own severity definitions. * fix(bitwarden-code-review): give Agent 5 a verbatim input boundary and order the fallback The carve-out described Agent 5's untrusted-input boundary as a rewording rather than supplying it, leaving the pipeline's one whole-file reader with the only boundary composed fresh each run. It is now a verbatim block, and the three sections asserting propagation to every subagent name the exception. In the single-agent path, the general fallback to existing review knowledge trailed the skill bullet and reversed its instruction not to substitute an opinion for skill review. * fix(bitwarden-code-review): rest Agent 5's carve-out on instruction, not declaration The carve-out justified a reduced rule bundle from the callee's frontmatter, which this pipeline does not enforce and which harnesses have been seen ignoring. Agent 5's Tool Discipline variant now instructs the tool restriction directly, keeps the WebFetch/WebSearch ban that dropping the first bullet had removed, and carries an exception for the referenced-file check its brief requires. Its prompt blocks and translation procedure move to references/agent-5-skill-review.md. Support-file detection returns to open-ended, which had silently dropped evals/, contexts/, and templates/. * fix(bitwarden-code-review): let injection findings survive Agent 5's scope fence The widened boundary tells Agent 5 to report a file that tries to direct its review, then the fence dropped anything the diff did not touch, which is exactly where a planted injection sits. Nothing else covers it: reviewing-claude-config declines SKILL.md for content and Agent 4 gets those files for the credential scan only. The coverage note gains a real slot in posting-review-summary rather than relying on the reporting agent to improvise one, and its gate keys on whether Agent 5 returned a usable review rather than on why it did not. * fix(bitwarden-code-review): stop a clean skill review reporting as uncovered Gating the Not covered line on translated finding count made the common case a false positive: the scope fence and the severity bars are meant to empty the list, so a skill change whose issues are all pre-existing would report the lens as never run. The gate keys on whether Agent 5 returned anything reviewable, which the orchestrator can observe without counting findings. * fix(bitwarden-code-review): use a word cspell knows for a malformed return * fix(bitwarden-code-review): carve CWE-1427 out of Step 4's pre-existing test Letting an injection finding clear Agent 5's scope fence achieved nothing while Step 4 dismissed it on the same ground one layer down, so the observation reached the Dismissed block rather than the report. The README no longer promises the threat-model preamble to every subagent, which Agent 5 is now a full exception to, and the single-agent path drops a reviewing-claude-config finding that only restates the coverage gap its own summary line already owns. * fix(bitwarden-code-review): give the coverage note a reason its own path can state The worked example asserted plugin-dev was not installed, but the single agent path cannot reach skill-reviewer whatever is installed, and it then pointed at a remedy that same absence would block. As the section's only example it is what a real comment gets patterned on. * fix(bitwarden-code-review): launch one Agent 5 per changed SKILL.md plugin-dev:skill-reviewer's output contract is one skill's report, so a plural path list can blend two skills into a report whose description, length, and progressive-disclosure sections name no file — the properties the agent exists to cover. Number skl ids across instances, and let the coverage line name only the files that went unreviewed. * fix(bitwarden-code-review): carry the CWE-1427 exception into Do Not Flag Step 5 receives the Review Rules, whose unqualified pre-existing bullet told it to dismiss what Step 4's exception had just kept. Filter the second list to SKILL.md paths that survive the change, so a deleted skill does not launch an Agent 5 instance or report a gap for its own removal. * fix(bitwarden-code-review): close the validator findings on the Agent 5 change Operating Rules still promised Tool Discipline verbatim to every subagent, which would hand Agent 5 a Bash instruction contradicting the restriction its reduced rules bundle rests on. Context Partitioning's ONLY clause omitted the one path the agent needs, and its diff is now narrowed to the skill under review. Both coverage-note exemplars name their files, the support-file-only case is stated rather than left silent, and the summary skill's render condition no longer turns on a word it never defines. * fix(bitwarden-code-review): gate the single-agent config review on SKILL.md too Dropping SKILL.md from both enumerations left a SKILL.md-only pull request matching neither, so the clause handing those files to reviewing-claude-config sat inside a bullet that never fired and the credential scan was lost on that path. Gate on the disjunction the way the pipeline already does. Qualify the two places that promised Agent 5 the whole diff, and stop the coverage note firing for an optional skill that falls back silently. * fix(bitwarden-code-review): narrow the coverage note in the block agents copy The render condition was tightened in the prose and left broad in the summary template beside it, which is the copy an agent assembles output from. Scope both to what this review covered, so naming the path that does cover the gap no longer reads as a reason to suppress the line. Label the example by the reviewer whose tool grant the reason belongs to rather than by an output mode the file defines as something else, and settle the Agent 5 hunk scope on the skill directory in all three places that state it. * fix(bitwarden-code-review): filter deleted SKILL.md out of the single-agent gates The SKILL.md disjunct these gates gained admitted deleted paths, so a skill split or rename would draw a coverage note asserting the removed file went unreviewed. The pipeline path already filters on presence at the head of the change; carry the same condition through all three gates on this path, so a later edit to one cannot leave the others behind.

[BRE-1833] feat(bitwarden-devops-engineer): Add managing-workflow-secrets skill (#205)

feat: add creating-jira-tickets delivery skill (#170) * feat: add creating-jira-tickets delivery skill Turns a tech breakdown's tasks.md into Jira tickets — an epic plus one child story/task per task entry, with real ticket titles, a Gherkin Acceptance criteria ADF section, and wired Blocked-by/Depends-on links. Creates tickets one at a time behind a human-in-the-loop review gate and goes through the acli CLI. Includes an acli/ADF reference and a skill-creator eval set (7 behavior cases + a 10/10 trigger set). Bumps bitwarden-delivery-tools 2.1.0 -> 2.2.0 (marketplace.json, plugin.json, README, CHANGELOG) and adds acli/workitem to the cspell dictionary. * Fix inverted link-list read-back example in acli reference * Replace creating-jira-tickets with filing-breakdown-tasks skill * Removed text * Trim filing-breakdown-tasks docs and record an eval baseline Descriptions and changelog cut to the length the surrounding entries use. The eval runner is a copy of the creating-pull-request sibling with a repeatable --plugin-dir, which the baseline run needs: filing-jira-tickets is what the should-not-trigger near-misses are written against, and it is not in the installed cache. Its output now records model, plugin_dirs and runs_per_query, so a baseline carries the conditions that produced it. baseline.json: 20/20 on claude-opus-5 at 3 runs with both plugin roots loaded. * Drop vestigial Owner capture from filing-breakdown-tasks

[QA]Port over qa test writing skill from test repo (#200) * Port over qa test writing skill from test repo * Address PR Feedback, remove AskUserQuestion from allowed-tools, change where the files are written to * Port over qa test writing skill from test repo * Slight tweak around mapping priority * Address Claude review feedback - align canonical example with the underlying rules, add more specificity about escaping double quotes * Fix typo * Update plugins/bitwarden-testing-tools/skills/writing-manual-test-cases/SKILL.md Co-authored-by: Patrick Honkonen --------- Co-authored-by: Patrick Honkonen

fix(claude-config-validator): stop two skills failing to load on quoted shell syntax (#208) Claude Code expands a bash-execution block on the raw text of any file it loads, so a skill that quotes the syntax to teach it runs the quoted fragment as a shell command and fails. reviewing-claude-config carried one in an inline code span; reviewing-command-definitions carried four plus a fenced example. Fenced blocks are not a safe haven: plugin-dev:command-development fails to load for the same reason, from a fenced block.

feat(claude-config-validator)!: fence findings to the changeset, split the review skill, and stop guessing at commands (#201) * feat(claude-config-validator)!: fence findings to the changeset and split the review skill Report only what a changeset introduced or worsened, fail only on CRITICAL, and replace the checklist tree with four targeted review skills. BREAKING CHANGE: reviewing-claude-config is now a router. The checklists/ and examples/ directories are removed, and SKILL.md review belongs to plugin-dev:skill-reviewer. * fix(claude-config-validator): close review findings in the guardrail change Restore the hook credential-access check dropped in the checklist split, add a security floor to the verdict so a widened permission or tool grant fails the run regardless of tier, exempt CRITICAL and security findings from the filter, add a fallback routing row for skill support files, and carry the CWE-1427 boundary into each targeted skill. Also realign the READMEs and the marketplace escalation with the new structure. * fix(claude-config-validator): gate the frontmatter skip on actual coverage Skip agent frontmatter review only where plugin-dev:plugin-validator ran, rather than wherever the file sits inside a plugin, so an uninstalled plugin-dev cannot silence a CRITICAL band. Restore the permission-example wildcards, and align both READMEs with the security floor and the coverage the targeted skills actually have. * fix(claude-config-validator): correct the permission schema the skills teach Permission rules are Tool(specifier) under permissions.allow/deny; the skills and both references taught a top-level autoApprovedTools array with bare Tool:specifier rules, which Claude Code does not read, so a reviewer following them would grep for a key that never appears and report an over-broad settings file clean. Also qualify the routing table's four skill names, cover shell execution and argument interpolation in command review, gate the command frontmatter skip on actual coverage, and carry the verdict's security floor into the remaining statements of it. * fix(claude-config-validator): correct the permission rule grammar and the detectors A bare Tool rule is valid and is the broadest grant available, so instructing reviewers that every rule is Tool(specifier) filed a CRITICAL against a working deny: ["WebFetch"] while calibrating them past the real unbounded grant. Scope that finding to the two forms Claude Code genuinely does not read. The detection greps, detect-broad-permissions.sh, and the shipped security-scan.sh still matched the old notation, so all three reported a filesystem-wide grant as scoped. Also settle // as absolute against a single leading slash as settings-relative, give command review a real frontmatter pass, and gate the hook carve-out on actual coverage like its two siblings. * fix(claude-config-validator): use low-entropy placeholders for the secret examples The example credentials had to match the quantifiers in the regexes the skill points at, but a realistic-looking string of that length reads as a live secret to a scanner. Repeated EXAMPLE text satisfies the length the regexes require without the entropy that makes it look real. * fix(claude-config-validator): stop telling reviewers that quoting closes shell injection Pass 7 said quoting $ARGUMENTS inside a !`cmd` block closes the hole and is not a finding. Substitution is textual and happens before the shell parses the line, so $(rm -rf ~) still executes inside double quotes and 1" ; rm -rf ~ ; " closes the author's quote outright. Both verified. Interpolation is now a finding either way, with stdin or a validated allowlist as the remedy. Also correlate the bare-tool-rule detector against permissions.allow so a bare rule in deny no longer fails the scan, exclude placeholder text from secret detection now that the teaching examples live inline, and correct the command frontmatter severity: frontmatter is optional for commands, so a missing description does not stop loading. * docs(claude-config-validator): drop the superseded quoting claim from the changelog * fix(claude-config-validator): guard the documented detector and scope frontmatter severity The bare-rule detector in security-patterns.md had no jq guard, so with jq absent the branch was skipped, ISSUES stayed 0, and it printed "Permissions appropriately scoped" for a check that never ran. The shipped script already guarded this; both copies now agree. Frontmatter severity is stated per file type, since commands do not require it and an unqualified CRITICAL row failed a valid frontmatter-less command. The model-value correction now reaches all four statements in the reference, including the detection table a reviewer classifies from. * fix(claude-config-validator): extend the security floor to shell-execution findings The floor enumerated permissions, tool grants, and hook capabilities, so a changeset adding a quoted $ARGUMENTS interpolation inside a !`cmd` block produced one IMPORTANT finding, widened nothing enumerated, and reported Pass with a live injection vector listed underneath. It now covers any new path from contributor-controlled input to a shell, in all three copies of the clause. Also annotate the Grep-path bare-rule pattern as candidate-only, since it cannot tell allow from deny and the router has no Bash to reach the jq version, and carry the model-identifier correction to the last two of six statements. * fix(claude-config-validator): bring hook input guidance to parity with commands reviewing-command-definitions asserted that the sibling applied the same rule to hook input. It did not, and it should not: verified by execution, a quoted value in a shell variable used directly is safe, the same value handed to a nested shell such as bash -c is not, and slash-command substitution has no safe quoted form at all. Both skills now state the three cases rather than one claiming agreement. Also add a settings pass for the fields that execute or bypass, since defaultMode: bypassPermissions disables prompting wholesale and matched no rule-shaped check; give priority-framework hook and settings severity tables, which it had none of while a skill delegated hook classification to it; settle the command tier in one place; and write the report rather than asking which PR to validate when nobody can answer. * fix(claude-config-validator): stop the secret scanner dropping keys by file path The placeholder filter added in 47ea728 ran over the whole grep -r record, which begins with the path, so a real key in any file under an example-named path was discarded and the scan reported a pass. Verified by execution both ways: an identical 40-char sk- key in example-agent.md and prod-agent.md is now reported for both, and the plugin's own teaching examples still produce no finding. Also reconcile hook severity, which contradicted itself four lines apart: a quoted value consumed directly by the command is not a finding, unquoted is CRITICAL, and a nested shell is CRITICAL either way. Drop the blanket permission elevation in Security Context, which overrode the per-field tables and falsified the security floor's own rationale, and extend agent tool-access review to Skill alongside Task. * fix(claude-config-validator): anchor the third secret filter to the value e5d374f narrowed the OpenAI and GitHub token filters from the whole grep record to the value and did not bring the generic-credential check along, so it still discarded any match whose file path contained example or xxx. Reproduced the reported fixture: three identical credentials, one reported. All three are reported now, and the plugin's own teaching examples still produce no finding. Also take acceptEdits out of the block whose closing sentence makes every item CRITICAL, since the framework rates it IMPORTANT and asks why rather than blocking, and state in the verdict that a widening the changeset justifies is not a finding at all, which is what makes asking possible. Move the Migration heading below the five Fixed entries it had captured. * fix(claude-config-validator): close three false negatives in security-scan.sh Verified by execution before and after. A raw sk- key under any examples/ directory reported "All security checks passed", because the path exclusion on the two token filters dropped it and the generic filter cannot see a key with no apiKey: prefix. Both filters are anchored to the value now, as the generic one already was. The sensitive-path loop grepped the whole settings file, so a config whose only entries are hardening denies failed the scan, contradicting the framework row this PR adds saying such a rule is the control. It reads permissions.allow via jq and records itself skipped without jq. Also drop additionalDirectories from the block that tiers every item CRITICAL, hoist the agent scope exclusion ahead of Pass 1, and reword a CRITICAL checkbox that read as a platform guarantee rather than a condition to confirm. Per review: cut the 2.0.0 changelog entry from 78 bullets to 17, and remove the skill-level README, moving the scanner usage it uniquely held to the plugin root. * fix(claude-config-validator): make skipped checks reach the verdict and scope check 4 Regressions from 88a366c, all reproduced before and after. With jq absent, a settings file granting bare Bash and Read(//Users/x/.ssh/**) printed "All security checks passed" while the same commit added a README line promising skipped checks are never reported as passed. Skips are counted now and a clean-but-incomplete run exits 2. That rewrite also dropped /etc from the sensitive-path list, which two other statements still said was reported, and left the documented twin grepping the whole settings file so a hardening deny failed it. Check 4 had the same whole-file problem plus four patterns whose unescaped pipe made them ERE alternations, so any quoted string containing curl tripped them. Both fixed in the script and the documented copy, which now share one path list. * fix(claude-config-validator): anchor .config and finish the skipped-check sweep 484fe38 unified the two sensitive-path lists and moved .config from a whole-file grep into a contains() loop, where it matches vite.config.ts, jest.config.js, and a Bash(npx jest --config ...) rule. Reproduced: the scan exited 1 on a benign edit rule. Anchored to .config/, which still matches Read(//Users/x/.config/**). The same commit added a skip branch to detect-dangerous-commands.sh without the counter its sibling block already had, so it printed SKIPPED and then exited 0 while the changelog claimed both copies gate their verdict. Every skip branch in both files is now checked to increment: six of six. Also annotate the reviewer-facing dangerous-command grep as candidates-only and anchor dd as dd if=. That is the block two skills tell reviewers to reuse, and it read as allow-only while being file-wide. * fix(claude-config-validator): report a settings.json that does not parse jq exits non-zero on a parse error exactly as it does on no-match, and the jq conversion in this PR swallowed both. A malformed settings.json holding a bare Bash grant, an ssh read grant, and Bash(rm -rf:*) produced "All security checks passed" and exit 0. Before the conversion those three checks were greps that fired regardless of whether the file parsed, so this is a regression the PR introduced. Invalid JSON is now CRITICAL, matching what reviewing-runtime-configuration already rates it, in the script and both documented detectors. Verified that valid files still report all three grants and that the jq-absent path still exits 2. Also reconcile two pairs of contradicting instructions: whether the agent frontmatter pass runs by default, and whether a quoted $ARGUMENTS interpolation is CRITICAL when the same file demonstrates it is arbitrary command execution. * fix(claude-config-validator): stop checks that did not run reporting as passed * fix(claude-config-validator): detect the colon-form permission rule it rates CRITICAL * fix(claude-config-validator): list what a configuration runs instead of classifying it * docs(claude-config-validator): scope the scanner's coverage claim to the check that makes it

[AI-88] docs: Update reference to the retired Claude context guide (#204) The per-repo .claude/CONTRIBUTING.md convention is retired, so this points repo-specific guidance at the repository's .claude/ directory instead of a file that no longer exists.

fix(claude-config-validator): match the skill's documented behavior to its tool grants (#199) * fix(claude-config-validator): make the skill's documented behavior match its grants The skill's allowed-tools is Read, Grep, Glob, but its output section made inline pull request comments the required mode and exempted only the two commands, so the default path could not be carried out on a direct invocation. Findings are now per-issue text by default; comment posting and the single-document report contract are what an invoking context can supply. The cross-plugin secret-detection enrichment called Skill(detecting-secrets) without holding Skill. Conditioned on the grant rather than widening allowed-tools, since a validator that preaches least privilege should not grant itself the ability to invoke arbitrary skills to reach one optional enrichment. Narrow Bash(git fetch:*) to origin, which is all step 1 fetches, and record the shared-host caveat for the fixed /tmp report path. * fix(claude-config-validator): finish the README coherence pass and order the output cases The first pass left the same capability claim in five more places: the skill README's process step and Review Output Format section, and the plugin README's feedback heading and two output descriptions. Order the three output cases specific to general with a first-match rule. /validate-ai holds a comment-posting grant, so it matched two bullets in an unordered list. * fix(claude-config-validator): retire the inline-comment instruction on the routing path checklists/agents.md carried the retired instruction under its Process heading, and SKILL.md step 3 loads checklists into context, so a direct invocation still received it. The occurrences in checklists/prompts.md and under examples/ are sample content rather than reviewer instructions and are left alone. Finish the skill README: the always-first security list claimed the git-tracking check runs unconditionally and dropped the finish-remaining-checks half of the rule, the feature list and tree still advertised executable automation, and Requirements listed Git and Bash as the skill's rather than the helper's. Also fold in the duplicated priority-level block, which had drifted to contradict its own earlier copy, point Troubleshooting at /plugin list for plugin installs, drop an unsourced statistic, and repair a sentence that inverted its own subject. Thin the 1.2.2 changelog entries to one line each. * fix(claude-config-validator): finish retiring the inline-comment mode in the output templates SKILL.md step 5 loads examples/ as the format template for the file type under review, so the retired instruction in example-prompts-review.md, the two other examples, and the prompts checklist exemplar still propagated. Reworded all four. Correct the shared-host caveat: an allow rule applies only when the symlink and its target both match, so Edit(//tmp/validation-summary.md) prompts rather than following a hostile symlink. World-readability is the real exposure. Mirror the git-tracking correction into the plugin README, replace SKILL.md's "mental checks" with the Grep mechanism the same step prescribes, and drop the remaining unsourced statistics from the checklists, examples, and plugin README. * fix(claude-config-validator): drop the never-update rule from the comment branch The branch carried "Create new comments rather than updating existing ones" verbatim from the inline mode this PR retires, and it contradicts the plugin's only comment-posting caller: /validate-ai upserts a single sticky comment keyed on its marker. Defer to the invoking context's convention instead. State the precedence ordering in the skill README too, which presented the three destinations as co-equal. * fix(claude-config-validator): return findings to the caller instead of posting them The comment-posting branch had no caller that wanted it. bitwarden-code-reviewer holds an inline-comment tool but tells the skill to "fold its findings into your own classification and validation", and the multi-agent config subagent expects findings per its Finding Shape schema. Posting directly would bypass both pipelines, plus the false-positive gate and the low-severity cap. Step 5 is now two cases: the validate-ai report contract, or return the findings as text. State the grant-composition rule once at the top. The file previously reasoned from the skill's own allowed-tools in one place and the caller's grants in another, which cannot both be the ceiling. Carry the settings.local.json qualification up into both Features lists and the frontmatter description, scope the CRITICAL CHECKS line to the pattern checks it actually covers, give the direct-invocation path a defined scope source, note that only origin is fetched, record that the Edit grant covers the Write tool, and finish the vocabulary rename across the examples and priority framework. * fix(claude-config-validator): align the checklists and skill README with step 5 Round-2 validation caught the skill README still documenting the three-case routing order, including the posting case, and citing the SKILL.md section that now forbids it. checklists/settings.md told the reviewer to stop the review on a security finding, reversing a fix 1.2.0 already shipped, and stated the settings.local.json check as a git-tracking fact ten lines before stating it correctly. Harmonize the wording everywhere on "appears in the changeset", and scope the /validate-ai description to what it does in CI. * fix(claude-config-validator): close the last capability claims from round-3 validation The direct-invocation list still opened with detecting recently modified files, which needs a changed-files list the skill has no way to obtain. Check 1 of the security patterns reference still framed the settings.local.json check as git tracking with no changeset alternative, and two headings kept the old vocabulary. * fix(claude-config-validator): drop review-event vocabulary and repair broken references The output format still ended in APPROVE / REQUEST CHANGES, which are GitHub review-event values from the posting mode this release retires, and which the report contract cannot consume. The verdict is now Pass or Issues found, with the caller mapping to its own vocabulary, updated across the README and four examples. Nine relative references were broken: six checklists cited SKILL.md as a sibling, and two examples cited reference/ and checklists/ as if they sat alongside. This is the plugin's own broken-file-reference finding class. Scope the marketplace use case to the component files the skill can review and point manifest checks at the command that delegates them. Align the skills pass list with the checklist's five passes, restate the strategy list as the five actual steps, add checklists/ to the disclosure criteria both commands pass to the skill reviewer, and finish the changeset wording in the settings and skills checklists. Also from the notes: hooks.md licensed abandoning the review on a finding, and a heading's globs were being eaten by markdown emphasis. Trim both command descriptions, which are what /help renders. * fix(claude-config-validator): finish the verdict sweep and tighten step 2 The settings example kept a third verdict spelling, BLOCK, so the earlier sweep for APPROVE and REQUEST CHANGES missed it. All seven verdict lines now read Pass or Issues found, which is what the report contract consumes. Step 2 listed the same credential check twice and omitted the dangerous auto-approval check that both the README and the security reference advertise as always run. Step 5 stated the per-issue rule twice; the Per-Issue Rules block is the more complete of the two. Note that a changeset touching only skill support files lands in no component bucket, so the configuration and security row does not fire for it. Widening the pattern would put the reference out of step with the action it mirrors. State the 2.1.210 floor in the plugin README's Installation section, convert the new grant section to imperative bullets to match the rest of the file, and drop a step number that collided with this skill's own numbering. * fix(claude-config-validator): correct the scope gloss and repair security-scan.sh The skill-bucket gloss was written for plugin-hosted skills and inverted for a repo-level one: .claude/skills/foo/reference/bar.md matches the config bucket, so components-changed is true and the configuration row does fire, while changed-plugins is empty so plugin validation does not. A reviewer following it would have skipped the one review that applied. Qualified both paths, mirroring the hook-files bullet. security-scan.sh had two silent-pass paths and one false positive. git ls-files ran against whatever repository the shell was in, and outside a repository the error was swallowed and the check printed a pass it never performed; it now runs with -C and reports a non-repository as skipped. grep -v "example" discarded any line containing that substring, including real credentials; the anchored path exclusions already cover the fixtures. checklists/ was missing from those exclusions, so the script reported the plugin's own teaching material as a critical finding. Verified all three by running it before and after. security-patterns.md embedded a copy of the script that had drifted 134 lines from the shipped file, including a different directory resolution that would silently find nothing; it now points at the file. Also finish the verdict and finish-remaining-checks sweeps, document allowed-tools in the frontmatter reference, bridge the two meanings of a changed skill, and record the omitted changes in the changelog. * fix(claude-config-validator): close a load-dependent false negative in check 1 Under pipefail, `git ls-files | grep -q` fails when grep matches early: grep exits, git dies of SIGPIPE with 141, and pipefail makes that the pipeline's status, so the branch went false and the script printed "OK: settings.local.json not in git" for a file that was committed. Reproduced at 20k tracked files with a match that sorts early; a small .claude/ never shows it. Ask git directly instead of piping. Carry the skipped state into the summary. The branch added in 6017574 left ISSUES_FOUND at 0, so a non-repository still printed "All security checks passed" and "No committed local settings" for a check that never ran. * fix(claude-config-validator): give the verdict a threshold rule Pass and Issues found were defined as the two values without saying which findings flip one to the other, so two reviewers with identical findings could return opposite verdicts and a SUGGESTED-only review was ambiguous. * refactor(claude-config-validator): move the security-scan.sh fixes out of scope Those fixes belong in a change where shell behaviour is the subject and can carry regression coverage. Three consecutive review rounds found the same false-negative class in that file, each time in code a previous fix was meant to close, and the script has no tests. They are preserved on fix-security-scan-false-negatives. Restore the troubleshooting note that described the reverted exclusions, and drop the changelog bullet that claimed them. The security-patterns.md changes stay: pointing at the shipped script instead of a copy that had drifted 134 lines is true regardless of what that script contains. Attribute the report to the caller in the security-scan step, the last place the skill still spoke as though it owned one. * fix(claude-config-validator): correct three defects in this change's own edits The reworded settings.local.json check fired CRITICAL on any changeset where the file appeared, including the one that deletes it, which is the remediation the same section prescribes. Scope it to additions and modifications. The enrichment section said a direct invocation "cannot invoke another skill", which contradicts the allowed-tools note added to the requirements reference in this same change: the field pre-approves rather than restricts. The call prompts, which makes it unavailable in a non-interactive run, and that is what matters. The settings example closed by counting three CRITICAL findings where it has two CRITICAL and one IMPORTANT. * fix(claude-config-validator): group the settings example's bullets by severity The CRITICAL heading covered all three bullets, one of which maps to the IMPORTANT finding at line 62, so the section claimed three CRITICAL findings twelve lines above a closing line that correctly says two. Regrouped on the shape the skill example already uses, one sub-heading per severity, which also folds the separate risk list into the bullets it describes.