[AI-76] llm: Fix Bash allowlist and Resolve routing in evaluating-sdk-internal-updates

[SaintPatrck]

🎟️ Tracking

AI-76

📔 Objective

The SDLC / SDK Update Evaluation action hit 13 permission denials on PR #7254 (comment) because two --allowedTools Bash patterns mix Claude Code's legacy trailing :* prefix-match syntax with an inline * wildcard — a combination the CLI itself treats as invalid. Every real git -C and ./gradlew invocation was denied, forcing the run to skip the local compile check and commit-range scan the skill's own steps depend on.

This also rewrites the skill's Resolve step to commit fixes directly instead of routing through /plan-android-work → /work-on-android, both of which are multi-phase commands gated on human confirmation at every step and incompatible with this skill's unattended CI invocation.

And it qualifies sdk-internal commit/PR references as bitwarden/sdk-internal#<NNN> in the generated report, since a bare #<NNN> copied from a commit subject auto-links within bitwarden/android, tagging unrelated issues there.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.46%. Comparing base (8842524) to head (2fbd817).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7260      +/-   ##
==========================================
+ Coverage   86.31%   86.46%   +0.15%     
==========================================
  Files         921      919       -2     
  Lines       66443    66223     -220     
  Branches     9794     9794              
==========================================
- Hits        57347    57263      -84     
+ Misses       5592     5457     -135     
+ Partials     3504     3503       -1     

Flag	Coverage Δ
app-data	17.58% <ø> (-0.12%)	⬇️
app-ui-auth-tools	18.67% <ø> (+0.05%)	⬆️
app-ui-platform	16.48% <ø> (-0.10%)	⬇️
app-ui-vault	28.02% <ø> (+0.14%)	⬆️
authenticator	6.09% <ø> (ø)
lib-core-network-bridge	4.12% <ø> (ø)
lib-data-ui	1.20% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

Claude Config Validation — PR #7260

Scope: 1 changed Claude file — .claude/skills/evaluating-sdk-internal-updates/SKILL.md (reviewed from the PR-authored snapshot under .claude-pr/; line numbers below refer to that copy — 45 lines, 716 words).

Section	Status
1. Plugin validation	Skipped — no plugin directories changed; this is a repo-own .claude/ change
2. Skill review (skill-reviewer)	Run
3. Config & security review (reviewing-claude-config)	Run — skills checklist + security scan

Verdict: request changes. No security issues, and the frontmatter rewrite is a genuine bug fix. But the new Resolve section commits before it verifies inside a workflow that auto-pushes, and the allow-list doesn't cover what the commit skill it now delegates to actually needs.

---

Security scan — clean

• No settings.local.json added or committed; no settings/permission files changed by this PR.
• No hardcoded secrets, API keys, tokens, or passwords in the changed file.
• The new grants are write-capable (Edit, Write, Bash(git add:*), Bash(git commit:*)), which is justified — the PR's purpose is to let the skill apply and commit fixes itself. Scoping is appropriate: no git push grant (pushing correctly stays in the workflow's own step) and no blanket Bash(git:*).
• Note on semantics: skill allowed-tools grants per-turn pre-approval, it does not restrict. Everything below is about whether a grant matches, not about blocking. In headless CI a tool without a grant is effectively denied, not prompted.

---

Errors (must fix)

1. MAJOR — the commit happens before verification, and CI pushes it regardless

.claude/skills/evaluating-sdk-internal-updates/SKILL.md:44-45

9.  Implement every fix from step 8 directly: … then commit with `Skill(bitwarden-delivery-tools:committing-changes)`.
10. Verify with the same compile task used in step 3.

Step 9 commits; step 10 verifies afterward. .github/workflows/sdlc-sdk-update-evaluate.yml then pushes any new commit unconditionally — its guard only hard-fails on uncommitted changes, so a commit that fails step 10 still lands on sdlc/sdk-update. The base version had the same 9/10 numbering, but there it was harmless because the delegated /work-on-android owned the build-then-commit ordering internally. Removing that delegation exposes the ordering.

Fix: verify, then commit.

9.  Implement every fix from step 8 directly: invoke `Skill(implementing-android-code)` first if the fix isn't
    purely mechanical, then verify with the same compile task used in step 3.
10. Once the compile is clean, commit with `Skill(bitwarden-delivery-tools:committing-changes)`. If it isn't clean,
    revert the edits and report instead of committing.

2. MAJOR — step 9 skips the pre-commit gate that both CLAUDE.md and the commit skill require

.claude/skills/evaluating-sdk-internal-updates/SKILL.md:44

Two independent sources mandate preflight before staging:

• .claude/CLAUDE.md:133 — "Before committing: Use bitwarden-delivery-tools:perform-preflight skill, then bitwarden-delivery-tools:committing-changes skill for message format."
• bitwarden-delivery-tools/skills/committing-changes/SKILL.md:56 — "Before staging, run the perform-preflight skill for the full quality gate checklist (tests, lint, security, architecture)."

Step 9 goes straight to the commit skill, and step 10 verifies compile only — no detekt, no lint, no unit tests. Detekt is enabled repo-wide (100-char line limit), so a formatting violation in an applied fix gets pushed and fails CI after the fact.

Dropping the interactive confirmation gates of /plan-android-work and /work-on-android is a sound call for an unattended run; losing preflight alongside them reads as collateral rather than intent.

Fix: invoke preflight in step 9 and add - Skill(perform-preflight) to allowed-tools. If preflight is deliberately out of scope for a mechanical UniFFI stub, say so explicitly in the skill so the deviation is a documented decision.

3. MAJOR — allowed-tools omits the git commands committing-changes needs

.claude/skills/evaluating-sdk-internal-updates/SKILL.md:5-11 vs :44

committing-changes opens with a mandatory Branch Check: "Resolve the repository's default branch from the remote rather than assuming main." That requires git rev-parse / git symbolic-ref / git branch / git remote, and composing the message needs git status / git diff --staged. The frontmatter grants only git add and git commit; the workflow's --allowedTools adds git log/show/diff but not the rest. In headless CI the branch check is silently skipped, and Claude cannot run git status — the exact command the workflow's push step gates on.

Fix: add to allowed-tools (and mirror the ones the surrounding prompt needs in the workflow):

  - Bash(git status:*)
  - Bash(git diff:*)
  - Bash(git rev-parse:*)
  - Bash(git symbolic-ref:*)
  - Bash(git branch:*)
  - Bash(git remote:*)

4. MAJOR (pre-existing, but now load-bearing) — the compile task is wrong for every module except :app

.claude/skills/evaluating-sdk-internal-updates/SKILL.md:33 and :45 — ./gradlew <module>:compileStandardDebugKotlin

standard is a product flavor declared only in app/build.gradle.kts:142-146. authenticator/build.gradle.kts declares no productFlavors, and neither do the other modules in settings.gradle.kts. For every module but :app the task is compileDebugKotlin. This text is unchanged by the PR, but the PR makes it load-bearing twice: step 3 says "A failure confirms a compile-time break directly", so a Task 'compileStandardDebugKotlin' not found in project ':authenticator' error is liable to be read as a confirmed SDK break — and step 10 reuses the same task as the pre-push gate.

Fix: ./gradlew app:compileStandardDebugKotlin for :app, ./gradlew <module>:compileDebugKotlin for every other module (only :app has the standard flavor); and add that a "task not found" error is a wrong task name, not a break.

---

Warnings (should fix)

5. MINOR — plugin-qualified Skill() entry may not match

:18 — Skill(bitwarden-delivery-tools:committing-changes). The documented permission syntax is Skill(name) (exact) or Skill(name *) (prefix); the plugin-qualified form is invocation syntax and is not documented as a permission specifier. Against that, non-Bash tool rules match rule content exactly and plugin skills are named plugin:skill, so it may well work. Unresolved — impact is low either way, since the workflow grants bare Skill, so CI is unaffected and only an interactive run would see an unexpected prompt. Either confirm on the next run or use Skill(committing-changes).

6. MINOR — steps 1-2 prescribe actions with no granted tooling

:31-32 — "check sibling directories to this repo" has no directory-listing grant (Bash(ls:*) is absent from both the skill and the workflow; out-of-tree Glob is the only route). "resolve a branch name as origin/<branch> in the clone" has no git -C * rev-parse or git -C * fetch for a stale local clone. CI is unaffected (the workflow clones fresh and hands over the path); local use stalls.
Fix: add Bash(ls:*) and Bash(git -C * rev-parse *), or reword step 2 to "use origin/<branch> directly in the step 4 range; if it errors as an unknown revision, tell the user to fetch."

7. MINOR — two-star patterns require a trailing argument

:6-7 — Bash(git -C * log *) compiles to ^git -C .* log .*$, which does not match a bare git -C /path log. The trailing-optional rewrite applies only to single-star patterns. Harmless for steps 4-5 as written (both always pass arguments), but Bash(git -C * log*) is more robust.

8. MINOR — the allow-list is duplicated in two places and has already drifted

:4-18 vs the workflow's claude_args — the workflow carries a near-copy plus git log/show/diff, bare Skill, and the MCP comment tool. Since skill allowed-tools is additive on invocation, the workflow only needs the bootstrap grants it can't get from the skill. Trimming it makes the frontmatter the single source of truth and stops the two lists diverging again.

9. MINOR — Write is broader than the described work

:16 — every fix in step 8 (adjusting a call site, stubbing a newly required interface method) is an Edit on an existing file; Write additionally permits whole-file overwrite of any repo file. Drop it, or state in step 8 that creating a new file is an expected outcome so the grant is justified.

10. MINOR — style inconsistency

Within the list, Bash(gh pr diff:*) uses the colon form while Bash(./gradlew *) and Bash(git -C * log *) use the space form; both are valid and equivalent at trailing position. Separately, .claude/skills/interacting-with-android-device/SKILL.md:4 uses the inline comma-separated form for allowed-tools while this file now uses a block list — also both valid. Worth settling a house style. Do not "normalize" the mid-string * to colons: the :* form is only recognized at the end of a pattern.

---

Advisory (verify on the first real run)

• Commit message ticket prefix. committing-changes requires [PM-XXXXX] on every commit and says to ask the user when the type is unclear — an unattended SDK bump has neither a ticket nor a user to ask. Consider having the workflow prompt state the expected prefix for this path (repo precedent exists: recent llm: … subjects).
• Git identity. Neither the frontmatter nor the workflow grants Bash(git config:*); a missing user.name/user.email would make step 9 unrecoverable. The action-provisioned checkout does carry an identity, so this likely works — an explicit git config step before Run Claude Code is cheap insurance.
• Reading the sdk-internal clone. Step 5 reads files in a sibling clone outside the workspace, and the workflow passes no --add-dir. Bare Read/Grep/Glob grants probably suffice; if the first run shows out-of-tree read denials, add --add-dir "${{ github.workspace }}/../sdk-internal" to claude_args.

---

Confirmed correct (checked, no action needed)

• YAML block-list allowed-tools is valid. The frontmatter reference documents "comma-separated string or YAML list", and the validator accepts a string or an array of strings. The conversion is not a regression.
• The wildcard rewrite is a real bug fix, not a style change. Prefix rules (…:*) are matched by literal startsWith, with no glob expansion — so the base Bash(git -C *:*) and Bash(./gradlew*:*) were unmatchable, i.e. the base skill effectively had no git and no gradle permission. The new Bash(git -C * log *) and Bash(./gradlew *) are the wildcard forms the validator itself recommends, and Bash(./gradlew *) also matches a bare ./gradlew. Worth calling out in the PR description.
• Step 2's pipe gh pr diff <PR> -R bitwarden/android | grep bitwardenSdk — compound commands are matched per subcommand; both Bash(gh pr diff:*) and Bash(grep:*) are present.
• Skill(implementing-android-code) (:17) — valid syntax, and the skill exists at .claude/skills/implementing-android-code/.
• Dropping /plan-android-work → /work-on-android is right-sized. That pair is a full ticket→PR lifecycle and would have tried to open its own PR inside a workflow that does its own pushing.
• Step 7 citation guidance (:37) is accurate and non-obvious: a bare #<NNN> lifted from an sdk-internal commit subject auto-links against bitwarden/android when posted to the sticky comment, tagging an unrelated issue. Good encoding of a failure mode the model would otherwise repeat.
• Frontmatter — name present, kebab-case, matches the directory; description present (322 chars), third-person, with concrete triggers; valid YAML, spaces only, no tabs.
• Structure — 45 lines (well under the 500-line guideline); no references/, examples/, or scripts/ referenced, so no broken file references; progressive disclosure isn't warranted at this size. Body word count (623) is under the 1,000–3,000 target, but reads as a deliberately dense procedural checklist — no change recommended.
• Body quality — states repo-specific facts a model can't infer (crate-scattered #[uniffi::export], bitwarden-ffi as a decoy, per-crate Kotlin packages, with_foreign callback interfaces never being additive-safe) and gives explicit anti-heuristics ("classify per hunk, not per commit", "never TODO()", "a sibling's structure is a template; its behavior is not evidence").

---

Remediation checklist

1. Swap steps 9 and 10 so the compile gate runs before the commit, with an explicit "revert and report instead of committing" branch on failure.
2. Invoke perform-preflight in step 9 and add Skill(perform-preflight) to allowed-tools — or document why compile-only is sufficient here.
3. Add the git inspection grants (status, diff, rev-parse, symbolic-ref, branch, remote) that committing-changes requires.
4. Fix the gradle task template: app:compileStandardDebugKotlin vs <other>:compileDebugKotlin, plus a note that "task not found" is a wrong task name, not a break.
5. Optional: resolve the Skill(plugin:name) syntax question, add Bash(ls:*) for step 1, drop Write, unify wildcard style, and collapse the duplicated allow-list.

[github-actions]

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Re-reviewed the head state of evaluating-sdk-internal-updates and the SDLC / SDK Update Evaluation workflow allowlist, including the follow-up commit that converts the skill's allowed-tools to a YAML block sequence. The frontmatter parses as valid YAML (the :* suffixes stay plain scalars since no colon-space appears), the skill remains well under the size guideline, and both referenced skills exist — implementing-android-code in this repo and bitwarden-delivery-tools:committing-changes via the plugin the workflow installs. The workflow's Bash patterns are strictly narrower than the previous Bash(git -C *:*) / Bash(./gradlew*:*) rules and still cover every command steps 3–5 issue, and the bitwarden/sdk-internal#<NNN> citation rule in step 7 correctly prevents cross-repo autolinking. Worth noting for the next run: this workflow is gated on head.ref == 'sdlc/sdk-update', so neither the narrowed patterns nor the new list-form frontmatter is exercised by this PR's own CI — the next SDK bump is the first live validation.

No blocking findings. The two earlier threads were addressed by the author and are not re-raised here.