fix(normalization): update allowed integrations checks to be fully lowercase#3248
fix(normalization): update allowed integrations checks to be fully lowercase#3248waleedlatif1 merged 1 commit intostagingfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile SummaryThis PR adds case normalization (
Both changes are defensive — block types from the registry and env allowlist values are already lowercase in practice — but they close a gap where mixed-case values from permission group configs could slip through unmatched.
Confidence Score: 4/5
Important Files Changed
Flowchartflowchart TD
A["intersectAllowlists(a, b)"] --> B{"a === null?"}
B -->|Yes| C["Return b (env allowlist, already lowercase)"]
B -->|No| D{"b === null?"}
D -->|Yes| E["Return a.map(toLowerCase) ✅ NEW"]
D -->|No| F["Return a.map(toLowerCase).filter(i => b.includes(i))"]
G["integrations.tsx groupedServices"] --> H["service.id.replace(/-/g, '_').toLowerCase() ✅ NEW"]
H --> I{"permissionConfig.allowedIntegrations.includes(normalized)?"}
I -->|Yes| J["Include service in UI"]
I -->|No| K["Filter out service"]
Last reviewed commit: 35b1d7e |
| if (a === null) return b | ||
| if (b === null) return a | ||
| if (b === null) return a.map((i) => i.toLowerCase()) | ||
| return a.map((i) => i.toLowerCase()).filter((i) => b.includes(i)) |
There was a problem hiding this comment.
Inconsistent lowercasing across branches
When a === null, b is returned as-is (line 46), but when b === null, a is now lowercased (line 47). On line 48, a is lowercased but b is not lowercased before filtering. While this currently works because b (the env allowlist) is already lowercase from getAllowedIntegrationsFromEnv(), the asymmetry makes the function fragile — if b ever comes from a source that isn't pre-lowercased, the intersection on line 48 would silently drop valid entries.
Consider normalizing both inputs uniformly:
| if (a === null) return b | |
| if (b === null) return a | |
| if (b === null) return a.map((i) => i.toLowerCase()) | |
| return a.map((i) => i.toLowerCase()).filter((i) => b.includes(i)) | |
| if (a === null) return b?.map((i) => i.toLowerCase()) ?? null | |
| if (b === null) return a.map((i) => i.toLowerCase()) | |
| return a.map((i) => i.toLowerCase()).filter((i) => b.map((j) => j.toLowerCase()).includes(i)) |
Summary
Type of Change
Testing
Tested manually
Checklist