[BRE-1838] tune DevOps Actions Audit Skill

[aj-bw]

🎟️ Tracking

BRE-1838

📔 Objective

Testing the action audit skill, it gave false positives flagging internal first party actions as non-compliant being pinned to main
We account for this in the workflow linter, had claude analyze linter logic and did some iterations locally and arrived at this which seemed to provide better results. Output examples below

📸 Screenshots

[github-actions]

Checkmarx One – Scan Summary & Details – 79246eac-d792-4d19-8994-4946a1f3a502

Great job! No new security vulnerabilities introduced in this pull request

[michalchecinski]

LGTM

[theMickster]

Hi @aj-bw and @michalchecinski I have added the ai-review label to the pull request. Please ensure that all pull requests raised in this repository has the label applied at the time a PR is raised (regardless of draft or published PR).

I am going to manually re-run action now that the label has been applied to get a bitwarden Claude code review executed.

You should see the Github workflow add (and subsequently update a sticky comment) on each push.

Thanks!

[github-actions]

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR tunes the action-audit, action-remediate, and bitwarden-workflow-linter-rules skills to apply a two-tier pin compliance model (internal bitwarden/ actions pin to @main; external actions pin to a 40-char SHA with an inline version comment), and refactors action-audit to defer to bitwarden-workflow-linter-rules as the single source of truth. The plugin version is bumped to 0.1.3 with a clear changelog entry covering all five behavioral changes. Manifests, plugin.json, marketplace.json, and CHANGELOG are version-consistent.

Code Review Details

No findings.

Notes for the author (not blocking):

• The workflow-fix skill (out of PR scope) still describes a single SHA-resolution path for step_pinned findings. Now that the linter rule has two fix procedures, consider a follow-up to align that skill's exception wording with the two-tier model to fully eliminate cross-skill drift.
• The changelog skips 0.1.2 (1.1 → 1.3). Verified intentional per the revert/re-bump commits — flagging only so future contributors understand the gap.

[aj-bw]

Hi @aj-bw and @michalchecinski I have added the ai-review label to the pull request. Please ensure that all pull requests raised in this repository has the label applied at the time a PR is raised (regardless of draft or published PR).

I am going to manually re-run action now that the label has been applied to get a bitwarden Claude code review executed.

You should see the Github workflow add (and subsequently update a sticky comment) on each push.

Thanks!

thanks @theMickster ! that is good to know, I will be sure to do that in the future
I have marked this PR as ready for review

[theMickster]

@aj-bw The markdown files appear to have formatting problems. On your local workstation, please ensure that you run our standard npm install because we need prettier and the husky pre-commit hooks to do their linting work. Thank you!

[theMickster]

♻️ A local Claude Code review caught this cross-skill finding.
I think that it is worth reviewing the wording across all skills in the plugin to be sure that all the verbiage is in alignment.

`bitwarden-workflow-linter-rules` `step_pinned` contradicts the new pin compliance rules

plugins/bitwarden-devops-engineer/skills/bitwarden-workflow-linter-rules/SKILL.md:44

The PR description says the new section "mirrors the logic in step_pinned.py," but the step_pinned rule in this sibling skill explicitly states: "A uses: reference is not pinned to a full commit SHA (e.g., uses a tag like @v3 or branch like @main)."
That directly contradicts the new rule, which treats @main as the compliant state for bitwarden/* actions. Readers who consult the linter-rules skill as the reference will get the opposite answer from action-audit.

Update step_pinned in lockstep so there's a single consistent definition across the plugin.

[theMickster]

♻️ A local Claude Code review caught this cross-skill finding.
I think that it is worth reviewing the wording across all skills in the plugin to be sure that all the verbiage is in alignment.

`action-remediate` has no branch for internal `@main` remediation

plugins/bitwarden-devops-engineer/skills/action-remediate/SKILL.md:34, :52

action-audit Step 5 tells users to hand findings off to action-remediate.
After this PR, audit produces two remediation shapes — @main for internal, @<sha> # <version> for third-party. action-remediate Step 1 scopes the approach to "pin update (update to a verified SHA) or replace (swap to a different action)" and Step 2's pin-update template is uses: <action>@<sha> # <original-ref> — neither supports pinning to @main.

A user following the documented flow would pin bitwarden/gh-actions/* to a SHA, which the new audit rule would then flag as non-compliant on the next run.

Either update action-remediate to add an internal-action branch, or drop the audit→remediate hand-off reference until the remediate skill supports both shapes.

[theMickster]

Please see my general comments as there appear to be cross-skill workflow fixes to apply & we need the work to be formatted properly. Thank you!

[aj-bw]

@theMickster @SaintPatrck @michalchecinski thanks for yalls patience! I should have all the feedback addressed regarding logic, drift potential and formatting.
I treated the workflow linter rules skill as the source of truth. Our rules change very rarely ( no changes in over a year ) so that should be minimal drift potential. Also made sure langauge was better aligned.

[theMickster]

Hello @aj-bw et. al,

I am field testing the new multi-agent code review, I gave it a local run against the PR. The findings are below for your analysis. Please incorporate the findings that are true signals.

We are also looking closely into the difference in the way that Claude Code runs the review given different models for pros, cons, false-positive rates, instructions not being followed, etc. so please don't hesitate to let me know your thoughts.

Many Thanks!

---

Sonnet Code Review: [BRE-1838] tune DevOps Actions Audit Skill (#89)

Date: 2026-05-12 | Reviewed by: Claude Code | Model: claude-sonnet-4-6

Summary

Severity	Count
🛑 Blocker	1
⚠️ Important	4
♻️ Refactor	2

This PR correctly identifies and fixes the false-positive problem in the action-audit skill — the two-tier pinning model (internal bitwarden/ → @main; external → full SHA) is the right direction, and the audit→remediate flow improvements are solid. However, the new "single source of truth" design has a critical path-resolution defect that will break the skill in marketplace deployments, and several logic gaps in the security-relevant rule definitions need attention before shipping. The CHANGELOG versioning is also malformed and should be cleaned up.

Findings

🛑 Blockers

Cross-skill runtime read uses repo-relative path that won't resolve in marketplace deployments

plugins/bitwarden-devops-engineer/skills/action-audit/SKILL.md:27
Caught by: Architecture agent

Details

The new ## Pin Compliance Rules section instructs the agent at runtime:

"Before classifying any action reference, read plugins/bitwarden-devops-engineer/skills/bitwarden-workflow-linter-rules/SKILL.md..."

This path is rooted at the bitwarden/ai-plugins repo layout. When this plugin is installed and invoked through the Bitwarden marketplace, the agent's working directory is the user's project — not a checkout of bitwarden/ai-plugins. The Read call will fail silently; the agent has nothing to fall back on because the PR's stated goal was to remove the embedded copy of the pin compliance rules. The skill then operates without the compliance definition it's explicitly required to load — exactly the drift this PR was meant to eliminate, now manifesting as a runtime path failure.

No other SKILL.md in this repo uses a cross-skill runtime-read pattern. The repo's plugin-dev guidance uses ${CLAUDE_PLUGIN_ROOT} for plugin-internal file references in hook configuration.

Fix: Replace every instance of plugins/bitwarden-devops-engineer/skills/bitwarden-workflow-linter-rules/SKILL.md with ${CLAUDE_PLUGIN_ROOT}/skills/bitwarden-workflow-linter-rules/SKILL.md. This applies to:

• Line 27 (Pin Compliance Rules section)
• Line ~62 (Step 2 audit mode instruction)
• Line ~70 (Step 3 compliance column instruction)

Alternatively, inline the rule contents directly into action-audit/SKILL.md and accept the duplication risk. Do not ship the repo-rooted path in any form.

---

⚠️ Important

CHANGELOG documents 0.1.2 release that was never reflected in any version file

plugins/bitwarden-devops-engineer/CHANGELOG.md:8-22
Caught by: Architecture agent

Details

The diff adds entries for both [0.1.2] - 2026-04-16 and [0.1.3] - 2026-05-08, but all three version-tracking files jump directly from 0.1.1 to 0.1.3:

• .claude-plugin/marketplace.json: 0.1.1 → 0.1.3
• plugins/bitwarden-devops-engineer/.claude-plugin/plugin.json: 0.1.1 → 0.1.3
• README.md: 0.1.1 → 0.1.3

Version 0.1.2 was never released — no tag, no marketplace entry, no plugin.json snapshot. Documenting it in the CHANGELOG creates a phantom version: a user on 0.1.1 consulting the CHANGELOG will read about a 0.1.2 release that never existed as an installable artifact.

This violates the repo CLAUDE.md rule: "Version bump and changelog changes must be part of the same PR as the code changes. This ensures version history is accurate and traceable."

Fix: Merge the 0.1.2 and 0.1.3 entries into a single [0.1.3] block dated 2026-05-08. All changes in this PR ship together as 0.1.3.

---

sm-action exception is unscoped — could bypass SHA pinning for third-party actions named with that string

plugins/bitwarden-devops-engineer/skills/bitwarden-workflow-linter-rules/SKILL.md:47
Caught by: Security & logic agent

Details

The new internal-action trigger includes:

"Exception: paths containing sm-action are compliant at any ref and never trigger this rule."

"Paths containing sm-action" is an unanchored substring match with no owner constraint. An LLM parsing this instruction could interpret it as a global compliance exception for any action whose path contains the string sm-action — including third-party actions like attacker/sm-action or evil-org/foo-sm-action-bar. This undermines the entire purpose of the pinning rule, which exists to prevent supply-chain attacks via mutable tag/branch references.

The placement in the internal-action trigger section provides some contextual scoping, but LLM instruction parsing is not reliably context-bound for substring conditions expressed in plain prose. A well-crafted third-party action name could exploit this ambiguity to escape SHA pinning requirements.

Fix: Constrain the exception to a specific owner+repo form:

"Exception: references of the form bitwarden/sm-action[/path]@<any-ref> are compliant at any ref."

This makes the bitwarden/ owner a hard requirement and scopes the exception to exactly the intended repo, leaving no room for substring-based bypass.

---

Ambiguous or in external-action trigger conflates two compliance failures the audit flow can't distinguish

plugins/bitwarden-devops-engineer/skills/bitwarden-workflow-linter-rules/SKILL.md:48
Caught by: Security & logic agent

Details

The external-action trigger reads:

"A uses: reference not starting with bitwarden/ is not pinned to a full 40-character commit SHA, or is missing an inline version comment."

This conflates two distinct states with different remediation paths:

1. Not SHA-pinned — needs SHA resolution + comment (the full fix flow)
2. SHA-pinned but missing version comment — needs only the comment added; no SHA resolution required

The problem: the action-audit Step 3 table tracks hash/tag/branch/none pin status but has no column for comment presence. An external action correctly pinned to a 40-char SHA but lacking a # v1.2.3 comment would be flagged as non-compliant by the linter rule, yet the audit flow cannot detect this case (no comment column) and would route it through the full SHA-resolution path — resolving a SHA that's already correct, and potentially re-pinning to the current tip if the agent misinterprets the ref.

Fix (either approach):

• Option A: Split the external trigger into two sub-rules with distinct fixes — not_sha_pinned and missing_version_comment — so the audit table and remediation instructions can handle each path independently.
• Option B: Extend the action-audit Step 3 table to include a Comment column (present / absent) and add a corresponding remediation row for the comment-only case in Step 4/5.

---

Internal-action on-SHA flag recommends @main pin which silently weakens an immutable reference

plugins/bitwarden-devops-engineer/skills/action-audit/SKILL.md:75-78
Caught by: Security & logic agent

Details

The new Step 4 instruction for internal bitwarden/ actions states:

"Flag if the action is currently on a SHA — this likely means it was incorrectly treated as third-party at some point."

And the Step 5 remediation column says: "For internal actions: change ref to @main"

A SHA pin is immutable — it locks to a specific commit regardless of what happens to the branch. @main is mutable — it tracks the branch tip and can be updated by anyone with push access to bitwarden/. Following the documented audit→remediate flow on an internal action that was deliberately SHA-pinned (e.g., frozen during a security incident, locked to a known-good version, or pinned for reproducibility) would silently revert a security control without any indication to the user.

The rationale in the flag ("likely means incorrectly treated as third-party") is speculative — it does not describe a risk posed by the SHA pin itself, only an assumption about intent. A SHA-pinned internal action is not less secure than @main; it is more secure.

Fix (either approach):

• Option A: Downgrade the internal-on-SHA case from non-compliant to an informational observation: "Note: this internal action is SHA-pinned rather than @main-pinned. No remediation needed; investigate if you expect it to track main." Keep SHA-pinned internal actions out of the non-compliant list.
• Option B: Require the agent to confirm with the user before un-pinning: "This action is currently on a SHA (immutable). Changing to @main will make it mutable. Confirm before proceeding."

---

♻️ Refactor

action-audit frontmatter description still says 'unpinned' while the body now says 'non-compliant'

plugins/bitwarden-devops-engineer/skills/action-audit/SKILL.md:3-6
Caught by: Architecture agent

Details

The frontmatter description: block was only whitespace-modified in this PR. Its semantic text still reads:

"sweeps all workflow files for unpinned action references (audit mode)"

Meanwhile, the body (line 29, line 57 after the diff) has been rewritten to describe audit mode as sweeping for non-compliant references — which is the whole semantic change this PR introduces. Under the new model, a bitwarden/ action pinned to @main is compliant even though it is "unpinned" in the SHA sense.

The description is the surface Claude Code uses to match user intent to a skill. Leaving it saying "unpinned" means the skill continues to trigger on the old framing. This reintroduces exactly the drift this PR fixed between action-audit and bitwarden-workflow-linter-rules.

Fix: Update the frontmatter description: in action-audit/SKILL.md — change "unpinned action references" to "non-compliant action references". The <example> trigger phrase ("Can you find all unpinned actions across the org?") is fine to keep since users will still phrase requests that way.

---

action-remediate frontmatter description omits the new 'pin to main' remediation path

plugins/bitwarden-devops-engineer/skills/action-remediate/SKILL.md:3-6
Caught by: Architecture agent

Details

The PR adds a third remediation approach in the body (Step 1 and Step 2, line ~35+):

"pin to main — for internal bitwarden/ actions: change the ref to @main"

But the frontmatter description (untouched by this PR) still reads:

"Applies hash pins or action replacements across selected repos and creates draft PRs."

An agent or user discovering action-remediate via its description will see no mention of the pin to main path — the same discoverability gap this PR fixes elsewhere, now present in action-remediate's own frontmatter. This is the description a user would read when deciding which remediation approach applies to their audit findings for internal actions.

Fix: Update the description to cover all three paths, e.g.:

"Applies internal-action @main pins, third-party SHA pins, or action replacements across selected repos and creates draft PRs."

---

Reviewed and Dismissed

🔍 2 initial findings dismissed after validation

Frontmatter description still claims 'resolved SHAs' after two-tier pin model update

plugins/bitwarden-devops-engineer/skills/action-audit/SKILL.md:6
Caught by: Code quality agent
Original severity: ♻️ Refactor
Original confidence: 85/100
Dismissed at: Step 4 validation
Dismissed because: Duplicate of arch-3. Both findings target the same stale frontmatter text in action-audit/SKILL.md — arch-3 already covers the broader semantic drift ('unpinned' vs 'non-compliant') of which 'resolved SHAs' is a sub-point. The issue is fully captured and actionable under arch-3.

---

CHANGELOG entry for 0.1.2 is backdated but has no matching artifact version

plugins/bitwarden-devops-engineer/CHANGELOG.md:20-22
Caught by: Code quality agent
Original severity: ♻️ Refactor
Original confidence: 80/100
Dismissed at: Step 4 validation
Dismissed because: Duplicate of arch-2. Both findings describe the identical problem: CHANGELOG documents a 0.1.2 entry with no corresponding version artifact, while all version files jump directly from 0.1.1 to 0.1.3. The issue is fully captured and actionable under arch-2.

---

Opus Code Review: [BRE-1838] tune DevOps Actions Audit Skill (#89)

Date: 2026-05-12 | Reviewed by: Claude Code | Model: claude-opus-4-6

Summary

Severity	Count
🛑 Blocker	0
⚠️ Important	2
♻️ Refactor	3

This PR correctly identifies the false-positive problem in the action-audit skill and the two-tier pinning model is the right direction. The "single source of truth" design introduces a runtime path-resolution defect that will break the skill in marketplace deployments, and a logic gap in incident mode drops a key remediation branch. Three documentation-consistency issues round out the findings. No blockers, but the path issue should be fixed before merge.

Findings

⚠️ Important

Cross-skill reference uses repo-relative path that won't resolve in marketplace deployments

plugins/bitwarden-devops-engineer/skills/action-audit/SKILL.md:28
Caught by: Architecture agent

Details

The new ## Pin Compliance Rules section (line 28) instructs the agent:

"Before classifying any action reference, read plugins/bitwarden-devops-engineer/skills/bitwarden-workflow-linter-rules/SKILL.md..."

This path is rooted at the bitwarden/ai-plugins repo layout. When the plugin is installed and invoked through the Bitwarden marketplace, the agent's working directory is the user's project — not a checkout of bitwarden/ai-plugins. The Read call fails silently. The declared "single source of truth" for compliance classification is then unreachable, so the agent has no compliance definition to apply — exactly the drift problem this PR was meant to eliminate, now manifesting as a runtime failure.

Every other cross-skill reference in this repo uses canonical Skill() invocations (e.g., Skill(bitwarden-security-engineer:bitwarden-security-context), Skill(posting-review-summary)). No other SKILL.md reaches into the filesystem with a plugins/... path.

Fix: Replace the path reference with ${CLAUDE_PLUGIN_ROOT}/skills/bitwarden-workflow-linter-rules/SKILL.md, or reference the sibling skill by name and let the skill loader resolve it — e.g., prefix the section with Skill(bitwarden-workflow-linter-rules). Do not ship a repo-rooted path.

---

Step 4 drops the 'replacement action' remediation branch that Step 1 still references

plugins/bitwarden-devops-engineer/skills/action-audit/SKILL.md:79-99
Caught by: Security & logic agent

Details

Step 1 (unchanged by this PR) still states:

"If a replacement action is mentioned, note it for the remediation step (handled separately by the action-remediate skill)."

Prior to this PR, Step 4 had an explicit third branch: "If the user mentioned a replacement action: note it in the report." The diff replaces Step 4 entirely with a two-branch model (internal bitwarden/ vs third-party) and drops the replacement-action case entirely.

This matters most in incident mode — the primary use case when an action is compromised (e.g., tj-actions/changed-files). The operator's desired outcome is usually to replace the compromised action with a safe alternative, not pin to a SHA of the compromised one. The action-remediate skill still supports the replace remediation approach (new line ~55), but action-audit no longer produces the structured output that feeds that path. The two skills are now misaligned on their handoff.

Fix: Restore a third branch in Step 4:

"Replacement action provided (incident mode only): record the replacement target in the Remediation column; skip SHA resolution for the compromised action; resolve the SHA for the replacement action instead using gh api repos/<owner>/<repo>/commits/<ref> --jq '.sha'."

---

♻️ Refactor

action-remediate frontmatter description omits the new 'pin to main' remediation path

plugins/bitwarden-devops-engineer/skills/action-remediate/SKILL.md:3-6
Caught by: Architecture agent

Details

The frontmatter description: (untouched by this PR) still reads:

"Applies hash pins or action replacements across selected repos and creates draft PRs."

The body now defines three remediation approaches (new lines ~34-38):

1. pin to main — for internal bitwarden/ actions
2. pin update — SHA pin for external actions
3. replace — swap to a different action

"Pin to main" is neither a hash pin nor an action replacement. The description is the trigger surface Claude Code uses to route user requests to skills. A user saying "fix the internal actions from the audit" won't match "hash pins or action replacements." The PR introduced this gap by adding the third mode without updating the description.

Fix: Update the description to cover all three paths, e.g.:

"Applies the appropriate remediation per action type — @main ref for internal bitwarden/ actions, full SHA + version comment for external actions, or full replacement — across selected repos and creates draft PRs."

---

action-audit frontmatter description still says 'unpinned' while the body now classifies on broader compliance

plugins/bitwarden-devops-engineer/skills/action-audit/SKILL.md:3-6
Caught by: Architecture agent

Details

The frontmatter description: still reads:

"sweeps all workflow files for unpinned action references (audit mode)"

The body was rewritten in this PR to sweep for non-compliant references — a materially broader category. Under the new two-tier model, an internal bitwarden/ action on a SHA is non-compliant (not just "unpinned"), and an external action with a SHA but no version comment is also non-compliant. The description no longer accurately describes the audit behavior.

This description is the trigger surface. A user asking "audit our internal actions that are SHA-pinned to freeze a version" wouldn't trigger on "unpinned action references." The PR changed the body terminology to "non-compliant" but left the description behind.

Fix: Update the frontmatter description — change "unpinned action references" to "non-compliant action references". The <example> trigger phrases ("find all unpinned actions") are fine to keep since users phrase requests that way.

---

Step 4 contradicts the sm-action exception it claims as the single source of truth

plugins/bitwarden-devops-engineer/skills/action-audit/SKILL.md:78-79
Caught by: Code quality agent

Details

Step 4 (Internal actions branch) says:

"Flag if the action is currently on a SHA — this likely means it was incorrectly treated as third-party at some point."

But the bitwarden-workflow-linter-rules step_pinned rule — declared as the single source of truth in this same PR's new "Pin Compliance Rules" section — explicitly states:

"Exception: paths containing sm-action are compliant at any ref and never trigger this rule."

So bitwarden/sm-action/...@<sha> is fully compliant per the linter rule, but Step 4 would flag it as a likely misclassification and recommend changing to @main. This re-introduces the cross-skill drift the rest of this PR eliminates — the audit gives different compliance judgments for sm-action references than the linter rule it is supposed to defer to.

Fix: Either drop the SHA-flag instruction entirely (since the linter rule is the authority on compliance), or explicitly carve out the sm-action exception inline:

"Flag if the action is currently on a SHA and the path does not contain sm-action — this likely means it was incorrectly treated as third-party at some point."

---

Reviewed and Dismissed

🔍 2 initial findings dismissed after validation

Cross-skill reference uses repo-relative path that won't resolve at runtime

plugins/bitwarden-devops-engineer/skills/action-audit/SKILL.md:26-28
Caught by: Code quality agent
Original severity: ⚠️ Important
Original confidence: 80/100
Dismissed at: Step 5 severity audit
Dismissed because: Duplicate of arch-1 — same file, overlapping line range (lines 26-28 vs line 28), identical path-resolution defect, identical fix. Issue is fully captured and actionable under arch-1.

---

Audit mode cannot detect 'missing inline version comment' non-compliance

plugins/bitwarden-devops-engineer/skills/action-audit/SKILL.md:60
Caught by: Security & logic agent
Original severity: ⚠️ Important
Original confidence: 85/100
Dismissed at: Step 4 validation
Dismissed because: Speculative. gh search code returns the full matched uses: line including any trailing inline comment text. The agent applying the step_pinned compliance filter has the complete line available and can check for the presence of a trailing # <version> comment. Nothing in the mechanism prevents detecting missing inline comments — the data is available in the search result and the rule is explicit about the requirement. No structural evidence the check is unreachable.

[aj-bw]

@theMickster thanks for the added review! Just pushed the changes
I think the Sonnet review was generally better, Opus incorrectly missed the blocker. But it did also raise something the sonnet review missed.

[theMickster]

@theMickster thanks for the added review! Just pushed the changes I think the Sonnet review was generally better, Opus incorrectly missed the blocker. But it did also raise something the sonnet review missed.

Thanks for the feedback! I too have seen that similar pattern where the sonnet model is more consistent ( "steady eddy") with it's review feedback, and then opus finds like one really good finding that sonnet missed plus a couple, well, not real problems 🤷🏼 To be honest, though, I think that's the real power for users with this thing... Multiple reviews early and often in your work cycle before it ever hits a PR.

received approval from @saintpatrick