← Back to PRs

#22198: fix(skills): treat empty allowBundled array as block-all

by haitao-sjsu open 2026-02-20 21:39 View on GitHub →
agents size: S
## Summary Fixes #21709 Setting `skills.allowBundled: []` was silently allowing all bundled skills instead of blocking them. The root cause was a two-part logic error: 1. **`normalizeAllowlist()`** collapsed empty arrays to `undefined` 2. **`isBundledSkillAllowed()`** treated `undefined` as "no filter → allow all" ### Changes | Before | After | |--------|-------| | `allowBundled: []` → all bundled skills allowed | `allowBundled: []` → all bundled skills blocked | | `allowBundled: ["", " "]` → all bundled skills allowed | `allowBundled: ["", " "]` → all bundled skills blocked | | `allowBundled` not set → all allowed | `allowBundled` not set → all allowed (unchanged) | | `allowBundled: ["web"]` → only "web" allowed | `allowBundled: ["web"]` → only "web" allowed (unchanged) | | Workspace skills always allowed | Workspace skills always allowed (unchanged) | ### What was changed - `src/agents/skills/config.ts`: 2 lines of logic (5 insertions, 2 deletions) - `normalizeAllowlist()`: return `normalized` directly instead of collapsing `[]` → `undefined` - `isBundledSkillAllowed()`: check `isBundledSkill()` before `allowlist.length === 0`, so workspace skills always pass through; then treat empty array as "block all bundled" - `src/agents/skills/config.test.ts`: new test file with 11 cases covering normal operation, empty arrays, whitespace-only entries, misspelled names, and workspace skill passthrough ## AI-assisted contribution This fix was pair-programmed with [Claude Code](https://claude.com/claude-code) (claude-opus-4-6). Claude handled the codebase analysis, wrote the fix and tests; I (haitao) drove the design decisions, caught edge cases (what about malformed inputs? what about misspelled skill names?), and reviewed all changes. Claude's initial fix actually had a subtle ordering bug — the empty-array check ran before the `isBundledSkill()` check, which would have blocked workspace skills too. Our test suite caught it before commit. A good reminder that AI-generated code still needs human review and thorough tests. ## Test plan - [x] `npx vitest run src/agents/skills/config.test.ts` — 11/11 pass - [x] `pnpm build` — pass - [x] `pnpm lint` — 0 errors, 0 warnings - [ ] CI full test suite 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- greptile_comment --> <h3>Greptile Summary</h3> Fixed critical security issue where `skills.allowBundled: []` silently allowed all bundled skills instead of blocking them. The two-line fix corrects the logic by preserving empty arrays through `normalizeAllowlist()` and treating them as "block all bundled" in `isBundledSkillAllowed()` while still allowing workspace skills to pass through. **Key changes:** - `normalizeAllowlist()` now returns `[]` instead of collapsing to `undefined` when the filtered result is empty - `isBundledSkillAllowed()` checks `isBundledSkill()` first to ensure workspace skills always pass, then treats `allowlist.length === 0` as block-all for bundled skills - Comprehensive test coverage with 11 test cases covering edge cases including empty arrays, whitespace-only entries, misspelled names, and workspace skill passthrough The PR description notes that Claude's initial fix had the check ordering wrong (would have blocked workspace skills too), but the test suite caught it before commit. <h3>Confidence Score: 5/5</h3> - This PR is safe to merge with minimal risk - The fix is minimal (2 lines changed), addresses a well-defined security issue, and includes comprehensive test coverage. The logic change is correct and the ordering of checks ensures workspace skills remain unaffected while properly blocking bundled skills when an empty array is specified. - No files require special attention <sub>Last reviewed commit: 83fb806</sub> <!-- greptile_other_comments_section --> <sub>(3/5) Reply to the agent's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!</sub> <!-- /greptile_comment -->

Most Similar PRs