feat(pii): gate data retention PII redaction behind feature flag#5144
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@greptile review |
PR SummaryMedium Risk Overview Persist path: Config API: GET and PUT on organization data retention include server-resolved UI: The PII Redaction block in Data Retention settings renders only when Contract schema and feature-flag tests were updated for the new flag and response field. Reviewed by Cursor Bugbot for commit ebdd773. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f8ca77c. Configure here.
Greptile SummaryThis PR gates the Data Retention PII redaction feature behind a
Confidence Score: 5/5Safe to merge — the flag is evaluated consistently without context across all three paths (logger, GET, PUT), and all changes are additive with a clean default-off fallback. The logger, GET route, and PUT route all call isFeatureEnabled with no context, so the flag state is always in sync. The fail-safe direction (flag off → no redaction skipped, UI hidden, writes blocked) is correct for a PII feature. No schema migrations, no new DB writes outside the existing piiRedaction field, and the existing enterprise billing gate is unchanged. No files require special attention. The route.ts 403 error message wording is a minor polish item. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Request: GET /data-retention] --> B[isFeatureEnabled 'pii-redaction'\nno context — global only]
B -->|false| C[Return response\npiiRedactionEnabled: false]
B -->|true| D[Return response\npiiRedactionEnabled: true]
C --> E[UI hides PII Redaction section]
D --> F[UI renders PII Redaction section]
G[Request: PUT /data-retention] --> H{body.piiRedaction\npresent?}
H -->|no| I[Update other settings normally]
H -->|yes| J[isFeatureEnabled 'pii-redaction'\nno context — global only]
J -->|false| K[Return 403]
J -->|true| L[Merge piiRedaction rules into DB]
M[ExecutionLogger.applyPiiRedaction] --> N{workspaceId present?}
N -->|no| O[Return payload unchanged]
N -->|yes| P[isFeatureEnabled 'pii-redaction'\nno context — global only]
P -->|false| Q[Return payload unchanged\nskip all DB lookups]
P -->|true| R[Resolve effective PII rules\nfrom org/workspace settings]
R --> S{Rules enabled?}
S -->|no| T[Return payload unchanged]
S -->|yes| U[redactPIIFromExecution]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Request: GET /data-retention] --> B[isFeatureEnabled 'pii-redaction'\nno context — global only]
B -->|false| C[Return response\npiiRedactionEnabled: false]
B -->|true| D[Return response\npiiRedactionEnabled: true]
C --> E[UI hides PII Redaction section]
D --> F[UI renders PII Redaction section]
G[Request: PUT /data-retention] --> H{body.piiRedaction\npresent?}
H -->|no| I[Update other settings normally]
H -->|yes| J[isFeatureEnabled 'pii-redaction'\nno context — global only]
J -->|false| K[Return 403]
J -->|true| L[Merge piiRedaction rules into DB]
M[ExecutionLogger.applyPiiRedaction] --> N{workspaceId present?}
N -->|no| O[Return payload unchanged]
N -->|yes| P[isFeatureEnabled 'pii-redaction'\nno context — global only]
P -->|false| Q[Return payload unchanged\nskip all DB lookups]
P -->|true| R[Resolve effective PII rules\nfrom org/workspace settings]
R --> S{Rules enabled?}
S -->|no| T[Return payload unchanged]
S -->|yes| U[redactPIIFromExecution]
Reviews (3): Last reviewed commit: "fix(pii): evaluate pii-redaction flag gl..." | Re-trigger Greptile |
Greptile SummaryThis PR gates the existing Data Retention PII redaction feature behind a
Confidence Score: 4/5Safe to merge for org-level flag targeting; a gap exists if the AppConfig document targets this flag by userId or adminEnabled rather than orgId. The logger evaluates the flag with only orgId — no userId is available at execution persist time. If the AppConfig document is ever configured with userIds or adminEnabled for this flag, those users would see the PII section in the UI and successfully save rules via PUT, but the logger would silently skip redaction at persist time. This is an inherent design limitation given the lack of user context in background processing, but it is currently undocumented in the flag description, making misconfiguration easy. In practice the PR intends org-level rollout and the env-var fallback is org-agnostic, so day-one deployments are unaffected. apps/sim/lib/logs/execution/logger.ts — the isFeatureEnabled call should document that only orgId context is available here, and the flag should not be targeted by userIds or adminEnabled in AppConfig. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant UI as DataRetentionSettings (UI)
participant GET as GET /data-retention
participant PUT as PUT /data-retention
participant Logger as ExecutionLogger
participant FF as isFeatureEnabled
participant AppConfig as AppConfig / ENV
UI->>GET: fetch settings (userId + orgId)
GET->>FF: "isFeatureEnabled('pii-redaction', {userId, orgId})"
FF->>AppConfig: getFeatureFlags() (~30s TTL)
AppConfig-->>FF: FeatureFlagsConfig
FF-->>GET: piiRedactionEnabled: boolean
GET-->>UI: "{..., piiRedactionEnabled}"
note over UI: Renders PII section only if piiRedactionEnabled
UI->>PUT: save piiRedaction rules (userId + orgId)
PUT->>FF: "isFeatureEnabled('pii-redaction', {userId, orgId})"
FF-->>PUT: piiRedactionEnabled
alt flag OFF
PUT-->>UI: 403 PII redaction not enabled
else flag ON
PUT-->>UI: 200 updated settings + piiRedactionEnabled
end
note over Logger: Execution persist path (no user context)
Logger->>FF: "isFeatureEnabled('pii-redaction', {orgId})"
FF-->>Logger: flagEnabled
alt flag OFF
Logger-->>Logger: return unredacted payload
else flag ON
Logger->>Logger: resolveEffectivePiiRedaction
Logger->>Logger: redactPIIFromExecution
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant UI as DataRetentionSettings (UI)
participant GET as GET /data-retention
participant PUT as PUT /data-retention
participant Logger as ExecutionLogger
participant FF as isFeatureEnabled
participant AppConfig as AppConfig / ENV
UI->>GET: fetch settings (userId + orgId)
GET->>FF: "isFeatureEnabled('pii-redaction', {userId, orgId})"
FF->>AppConfig: getFeatureFlags() (~30s TTL)
AppConfig-->>FF: FeatureFlagsConfig
FF-->>GET: piiRedactionEnabled: boolean
GET-->>UI: "{..., piiRedactionEnabled}"
note over UI: Renders PII section only if piiRedactionEnabled
UI->>PUT: save piiRedaction rules (userId + orgId)
PUT->>FF: "isFeatureEnabled('pii-redaction', {userId, orgId})"
FF-->>PUT: piiRedactionEnabled
alt flag OFF
PUT-->>UI: 403 PII redaction not enabled
else flag ON
PUT-->>UI: 200 updated settings + piiRedactionEnabled
end
note over Logger: Execution persist path (no user context)
Logger->>FF: "isFeatureEnabled('pii-redaction', {orgId})"
FF-->>Logger: flagEnabled
alt flag OFF
Logger-->>Logger: return unredacted payload
else flag ON
Logger->>Logger: resolveEffectivePiiRedaction
Logger->>Logger: redactPIIFromExecution
end
Reviews (2): Last reviewed commit: "feat(pii): gate data retention PII redac..." | Re-trigger Greptile |
|
@greptile review |
Summary
pii-redactionfeature flag (AppConfig-gated by org/user/admin on prod,PII_REDACTIONsecret fallback off-prod) gating the Data Retention PII redaction feature end-to-end.logger.applyPiiRedactionshort-circuits to the unredacted payload when the flag is off for the workspace's org, before resolving rules — covers all persist paths.PUT /api/organizations/[id]/data-retentionreturns 403 if a caller setspiiRedactionwhile the flag is off; GET/PUT now return server-resolvedpiiRedactionEnabled.piiRedactionEnabled(resolved server-side, piped through the existing GET response). Gated just that section, not the whole tab, since the tab also owns log-retention / soft-delete / task-cleanup.Type of Change
Testing
bun run lint— cleanbun run check:api-validation:strict— passedbun run type-check— cleanfeature-flags.test.ts— 13 passed (addedpii-redactionregistry assertion)Checklist