Skip to content

fix(normalization): update allowed integrations checks to be fully lowercase#3248

Merged
waleedlatif1 merged 1 commit intostagingfrom
fix/normalization
Feb 18, 2026
Merged

fix(normalization): update allowed integrations checks to be fully lowercase#3248
waleedlatif1 merged 1 commit intostagingfrom
fix/normalization

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

  • update allowed integrations checks to be fully lowercase

Type of Change

  • Bug fix

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Feb 18, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Feb 18, 2026 8:07pm

Request Review

@waleedlatif1 waleedlatif1 merged commit 86ca984 into staging Feb 18, 2026
7 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/normalization branch February 18, 2026 20:08
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 18, 2026

Greptile Summary

This PR adds case normalization (.toLowerCase()) to two integration allowlist checks on the client side:

  • intersectAllowlists in use-permission-config.ts: When only the permission group config has an allowlist (env allowlist is null), the returned values are now lowercased. This ensures downstream consumers that compare with .toLowerCase() (e.g., isBlockAllowed, filterBlocks) get consistent matches.
  • integrations.tsx: Service IDs are now lowercased after hyphen-to-underscore normalization before checking against the allowed integrations list, ensuring case-insensitive matching in the integrations settings UI.

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.

  • Note: The server-side mergeEnvAllowlist in permission-check.ts has a similar pattern where config.allowedIntegrations is returned without lowercasing when the env allowlist is null (line 79). This is not addressed by this PR but is worth considering for consistency.

Confidence Score: 4/5

  • This PR is safe to merge — it adds defensive case normalization to existing allowlist checks with no risk of breaking behavior.
  • The changes are minimal (2 lines), logically correct, and consistent with the existing pattern of case-insensitive comparison used elsewhere in the codebase. The only minor concern is the asymmetric normalization in intersectAllowlists, which is a style/robustness issue rather than a bug.
  • No files require special attention, though apps/sim/ee/access-control/utils/permission-check.ts has a similar unnormalized path on the server side that could be addressed in a follow-up.

Important Files Changed

Filename Overview
apps/sim/hooks/use-permission-config.ts Adds .toLowerCase() to the b === null branch of intersectAllowlists to ensure permission group config values are normalized. Correct fix but introduces asymmetry — only some branches normalize both inputs.
apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/settings-modal/components/integrations/integrations.tsx Adds .toLowerCase() to service ID normalization before checking against the allowed integrations list. Straightforward and correct.

Flowchart

flowchart 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"]
Loading

Last reviewed commit: 35b1d7e

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines 46 to 48
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))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Suggested change
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))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Comments