← Back to PRs

#17900: refactor(security): extract shared normalizeAllowFromList into audit-helpers

by iyoda open 2026-02-16 08:48 View on GitHub →
stale size: S
## Summary - Problem: `normalizeAllowFromList` was duplicated identically in `src/security/audit.ts` and `src/security/audit-channel.ts` - Why it matters: Duplicated logic risks divergence during future edits — one copy could be updated while the other is missed - What changed: Extracted the shared function into a new `src/security/audit-helpers.ts` module; both files now import from it. Added 7 unit tests for the extracted function. - What did NOT change (scope boundary): No behavioral changes. The `normalizeAllowFromList` in `pairing-store.ts` (different signature, different purpose) is NOT touched. ## Change Type (select all) - [ ] Bug fix - [ ] Feature - [x] Refactor - [ ] Docs - [ ] Security hardening - [ ] Chore/infra ## Scope (select all touched areas) - [x] Gateway / orchestration - [ ] Skills / tool execution - [ ] Auth / tokens - [ ] Memory / storage - [ ] Integrations - [ ] API / contracts - [ ] UI / DX - [ ] CI/CD / infra ## Linked Issue/PR - Related #17770, #17793, #17865, #17878, #17887, #17897 (prior refactor PRs in the same series) ## User-visible / Behavior Changes None ## Security Impact (required) - New permissions/capabilities? No - Secrets/tokens handling changed? No - New/changed network calls? No - Command/tool execution surface changed? No - Data access scope changed? No ## Repro + Verification ### Environment - OS: macOS (darwin) - Runtime/container: Node 22 / pnpm - Model/provider: N/A - Integration/channel (if any): N/A - Relevant config (redacted): N/A ### Steps 1. `corepack pnpm tsgo` — zero errors 2. `corepack pnpm build` — succeeds 3. `corepack pnpm vitest run src/security/audit-helpers.test.ts src/security/audit.test.ts` — 73 tests pass (7 new + 66 existing) ### Expected - Type check, build, and all security audit tests pass with no changes in behavior ### Actual - All pass ## Evidence - [x] Failing test/log before + passing after — 7 new unit tests for `normalizeAllowFromList` cover: undefined, null, non-array, numeric coercion, whitespace trimming, empty-string filtering, mixed entries - [ ] Trace/log snippets - [ ] Screenshot/recording - [ ] Perf numbers (if relevant) ## Human Verification (required) - Verified scenarios: `tsgo` clean, `build` succeeds, 73 tests pass (audit + audit-helpers) - Edge cases checked: Circular dependency avoidance (audit.ts imports audit-channel.ts; both now import from audit-helpers.ts — no cycle) - What you did **not** verify: Runtime gateway behavior (refactor only, no logic change) ## Compatibility / Migration - Backward compatible? Yes - Config/env changes? No - Migration needed? No ## Failure Recovery (if this breaks) - How to disable/revert this change quickly: Revert commit, inline the function back into both files - Files/config to restore: `src/security/audit.ts`, `src/security/audit-channel.ts` - Known bad symptoms reviewers should watch for: Import resolution errors in `audit-helpers.js` ## Risks and Mitigations None — pure extraction of identical code with no behavioral change. <!-- greptile_comment --> <h3>Greptile Summary</h3> Extracted identical `normalizeAllowFromList` function from `audit.ts` and `audit-channel.ts` into new shared module `audit-helpers.ts`. Both files now import from the helper module, eliminating duplication. Added 7 comprehensive unit tests covering undefined/null inputs, type coercion, whitespace handling, and edge cases. No behavioral changes—pure refactor to prevent future divergence. <h3>Confidence Score: 5/5</h3> - This PR is safe to merge with no risk - Perfect refactor: extracted byte-for-byte identical duplicated function into shared module with no logic changes, no circular dependencies (audit-channel.ts only imports types from audit.ts), comprehensive test coverage (7 tests), and verified import structure is clean - No files require special attention <sub>Last reviewed commit: 79de6cd</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs