feat: implement candidate erasure service and GDPR retention logic#211
Conversation
- Added `eraseCandidates` function to permanently delete candidates and their associated data, ensuring S3 objects are deleted first to prevent data leakage. - Introduced `retention.ts` for computing retention states and eligibility for purging candidates based on GDPR compliance. - Created unit tests for the erasure service and retention logic to ensure correctness and compliance with expected behaviors. - Added schema validation for organization settings to normalize numeric fields.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a complete GDPR data retention and erasure system: new database schema columns and an audit table, server utilities for retention state computation and S3-first candidate erasure, admin cron and review API endpoints, candidate-level restore/export/exemption endpoints, an admin settings UI page, a privacy notice on the job application form, internationalization support in five languages, and operational documentation. ChangesGDPR Data Retention & Erasure
Sequence Diagram(s)sequenceDiagram
participant Cron
rect rgba(100, 149, 237, 0.5)
note over Cron: Automated GDPR Retention Sweep
end
participant retention-cleanup
participant computeRetentionState
participant DB
participant S3
participant eraseCandidates
Cron->>retention-cleanup: POST /api/admin/retention-cleanup<br>(x-cron-secret, dryRun, batchSize)
retention-cleanup->>DB: fetch orgs with retentionEnabled
loop each org
retention-cleanup->>DB: max(application.updatedAt) per candidate
retention-cleanup->>DB: active (non-quarantined) candidates
retention-cleanup->>computeRetentionState: status per candidate
alt status == expired
retention-cleanup->>DB: set quarantinedAt + scheduledPurgeAt
end
retention-cleanup->>DB: quarantined candidates past scheduledPurgeAt
retention-cleanup->>eraseCandidates: erase eligible batch
eraseCandidates->>S3: deleteFromS3 (each storageKey)
eraseCandidates->>DB: transactional graph delete
eraseCandidates-->>retention-cleanup: ErasureReport
end
retention-cleanup-->>Cron: counters + per-org breakdown
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| * Returns candidate PII (name/email) — this is an authenticated in-app admin | ||
| * view, distinct from the privacy-safe `retention_audit` log. | ||
| */ | ||
| import { eq, and, max } from 'drizzle-orm' |
|
🚅 Deployed to the reqcore-pr-211 environment in applirank
|
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/pages/dashboard/settings/retention.vue`:
- Around line 156-161: The eraseNow function calls DELETE /api/candidates/:id
without support for legal-hold overrides, which prevents erasure of candidates
under legal-hold. Modify the eraseNow function to check if the candidate has a
legal-hold status and pass an appropriate override parameter (such as a query
parameter or request option) to the DELETE request when legal-hold is detected,
allowing the erasure to proceed for exempt candidates. Apply the same fix to the
other location mentioned (lines 323-325) where similar erasure logic exists.
In `@app/pages/jobs/`[slug]/apply.vue:
- Around line 313-321: The privacy notice section is gated by a condition that
checks for configured fields (job.privacyPolicyText, job.privacyPolicyUrl,
job.privacyContactEmail), which prevents the entire section from rendering when
none are set. This makes the fallback default privacy consent text in the v-else
block unreachable for unconfigured organizations. Modify the v-if condition on
the privacy notice container to always render the section, ensuring the default
GDPR disclosure text displays when no organization-specific privacy fields are
configured, rather than hiding the entire notice.
In `@server/api/admin/retention-cleanup.post.ts`:
- Around line 129-139: The issue is that when remainingErasures reaches zero or
below, the code breaks out of the organization loop entirely (the `if
(remainingErasures <= 0) break` statement), which prevents Phase 1 quarantine
processing for all remaining organizations. Instead of breaking, replace the
break statement with continue so that subsequent organizations can still proceed
through their quarantine phase even after the erase batch capacity is exhausted,
allowing retention progression to continue for all tenants rather than stalling
at the first org that exhausts the batch limit.
- Around line 37-41: The fixed 64-byte buffers created with Buffer.alloc(64) for
variables a and b cause silent truncation of secrets longer than 64 bytes,
allowing authentication to pass when only the first 64 bytes match. Replace the
fixed-size Buffer.alloc(64) calls with dynamic buffers that match the actual
length of each secret (use the length property of cronSecret and
env.CRON_SECRET). This ensures that timingSafeEqual compares the complete
secrets rather than truncated versions, preventing authentication bypass for
longer secret values.
In `@server/api/admin/retention/review.get.ts`:
- Around line 55-56: The effectiveStatus assignment currently always overrides
the status with 'quarantined' when quarantinedAt exists, which masks the
'exempt' state returned from computeRetentionState. Modify the effectiveStatus
logic to preserve the 'exempt' status even when a candidate is quarantined, so
that exemption is not hidden in the review payload. Update the conditional to
check if status is 'exempt' and maintain that value regardless of quarantinedAt,
rather than unconditionally setting it to 'quarantined'.
In `@server/api/candidates/`[id].delete.ts:
- Around line 22-37: The override flag is captured on line 22 but is not being
passed to the eraseCandidates function call on line 36, which means there is no
explicit audit trail documenting that a legally held candidate was deleted with
an override acknowledgement. Update the eraseCandidates call to include the
override flag in the options object alongside the actorId, so the erasure
operation explicitly records that this action was performed with an override to
the legal hold.
- Around line 36-47: In the error handling block after the eraseCandidates call,
add a check for the case where the erasure succeeded (status is 'erased') but
the audit record failed to be created (auditFailed is true). This check should
be added after the existing not_found and skipped_s3_failure conditions. When
this condition is detected, throw an error with an appropriate HTTP status code
and message indicating that the candidate was erased but the audit record could
not be confirmed, preventing an irreversible deletion without proper audit trail
verification before the function returns successfully.
In `@server/api/candidates/`[id]/export.get.ts:
- Line 55: The recordRetentionAudit function call is not checking its boolean
return value, which means audit persistence failures are silently ignored while
still returning the export payload. Modify the code to capture the return value
from recordRetentionAudit and throw an error or fail the request if the audit
recording returns false, ensuring that export operations fail when audit
traceability cannot be persisted.
In `@server/api/candidates/`[id]/restore.post.ts:
- Line 36: The `recordRetentionAudit` function call on line 36 returns a boolean
indicating whether the audit record was successfully inserted, but this return
value is currently being ignored. To fix this, capture the boolean return value
from `recordRetentionAudit` and check if it was successful before returning the
`{ restored: true }` response. Only allow the endpoint to return a success
response if the audit trail has been persisted to the database.
In `@server/api/candidates/`[id]/retention.patch.ts:
- Line 52: The recordRetentionAudit call on line 52 does not verify whether the
audit record was successfully created before returning a success response.
Capture the return value from the recordRetentionAudit invocation and check its
success status before proceeding. If the audit recording fails, handle the
failure appropriately (either by logging an error, throwing an exception, or
returning an error response to the client) to ensure that exemption state
changes are only confirmed successful when their corresponding audit records are
properly recorded.
In `@server/api/org-settings/index.patch.ts`:
- Around line 18-27: The read-then-write pattern for `retentionActivatedAt` in
the query at lines 18-27 followed by the database update at line 55 creates a
race condition where concurrent updates can overwrite the initial activation
timestamp. Move the timestamp logic into a single atomic database statement by
using a conflict update expression (such as COALESCE) in the upsert operation at
line 55 so that the first non-null `retentionActivatedAt` value is preserved
without requiring a separate read query, ensuring the operation is atomic at the
database level.
In `@server/utils/schemas/orgSettings.ts`:
- Line 20: The `privacyPolicyUrl` field in the orgSettings schema uses
`z.string().url()` which accepts any URL scheme (including potentially dangerous
ones like javascript:, data:, mailto:), creating a security risk since this URL
is rendered directly in HTML. Replace `z.string().url()` with `z.httpUrl()` in
the `privacyPolicyUrl` field definition while keeping the rest of the validation
chain (`.max(2000).nullish().or(z.literal(''))`) intact. This restricts the URL
to only HTTP/HTTPS schemes and is the recommended approach in Zod 4.4.2+.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 70312e0c-ce02-4643-968b-5432788ac2ed
📒 Files selected for processing (28)
DATA-RETENTION.mdSELF-HOSTING.mdapp/components/SettingsSidebar.vueapp/composables/useOrgSettings.tsapp/pages/dashboard/settings/retention.vueapp/pages/jobs/[slug]/apply.vueserver/api/admin/retention-cleanup.post.tsserver/api/admin/retention/review.get.tsserver/api/candidates/[id].delete.tsserver/api/candidates/[id]/export.get.tsserver/api/candidates/[id]/restore.post.tsserver/api/candidates/[id]/retention.patch.tsserver/api/candidates/index.get.tsserver/api/org-settings/index.get.tsserver/api/org-settings/index.patch.tsserver/api/public/jobs/[slug].get.tsserver/database/migrations/0030_robust_quasimodo.sqlserver/database/migrations/0031_lonely_mister_fear.sqlserver/database/migrations/meta/0030_snapshot.jsonserver/database/migrations/meta/0031_snapshot.jsonserver/database/migrations/meta/_journal.jsonserver/database/schema/app.tsserver/utils/erasure.tsserver/utils/retention.tsserver/utils/schemas/orgSettings.tstests/unit/erasure.test.tstests/unit/org-settings-schema.test.tstests/unit/retention.test.ts
| async function eraseNow(id: string, name: string) { | ||
| actionError.value = '' | ||
| if (!window.confirm(`Permanently erase ${name} and all their data? This cannot be undone.`)) return | ||
| try { | ||
| await $fetch(`/api/candidates/${id}`, { method: 'DELETE' }) | ||
| await refreshReview() |
There was a problem hiding this comment.
Erase action does not support legal-hold override, so exempt-path erasure can’t succeed.
eraseNow always calls DELETE /api/candidates/:id (Line 160). With legal-hold enforcement on that endpoint, exempt candidates need explicit override. The UI currently exposes “Erase now” without that flow.
🔧 Suggested fix
-async function eraseNow(id: string, name: string) {
+async function eraseNow(id: string, name: string, isExempt: boolean) {
actionError.value = ''
if (!window.confirm(`Permanently erase ${name} and all their data? This cannot be undone.`)) return
+ const override = isExempt
+ ? window.confirm('This candidate is exempt (legal hold). Override and erase anyway?')
+ : false
try {
- await $fetch(`/api/candidates/${id}`, { method: 'DELETE' })
+ const qs = override ? '?override=true' : ''
+ await $fetch(`/api/candidates/${id}${qs}`, { method: 'DELETE' })
await refreshReview()
}- <button title="Erase now" ... `@click`="eraseNow(item.id, item.name)">
+ <button title="Erase now" ... `@click`="eraseNow(item.id, item.name, item.status === 'exempt')">Also applies to: 323-325
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/pages/dashboard/settings/retention.vue` around lines 156 - 161, The
eraseNow function calls DELETE /api/candidates/:id without support for
legal-hold overrides, which prevents erasure of candidates under legal-hold.
Modify the eraseNow function to check if the candidate has a legal-hold status
and pass an appropriate override parameter (such as a query parameter or request
option) to the DELETE request when legal-hold is detected, allowing the erasure
to proceed for exempt candidates. Apply the same fix to the other location
mentioned (lines 323-325) where similar erasure logic exists.
| const effectiveStatus = c.quarantinedAt ? 'quarantined' as const : status | ||
| return { |
There was a problem hiding this comment.
Quarantine override hides exemption state for quarantined candidates.
At Line 55, effectiveStatus always becomes 'quarantined' when quarantinedAt exists, even if computeRetentionState returned 'exempt'. This masks legal-hold status in the review payload and causes downstream action mismatches.
🔧 Suggested fix
- const effectiveStatus = c.quarantinedAt ? 'quarantined' as const : status
+ const effectiveStatus = status === 'exempt'
+ ? 'exempt'
+ : c.quarantinedAt
+ ? 'quarantined' as const
+ : status📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const effectiveStatus = c.quarantinedAt ? 'quarantined' as const : status | |
| return { | |
| const effectiveStatus = status === 'exempt' | |
| ? 'exempt' | |
| : c.quarantinedAt | |
| ? 'quarantined' as const | |
| : status | |
| return { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/api/admin/retention/review.get.ts` around lines 55 - 56, The
effectiveStatus assignment currently always overrides the status with
'quarantined' when quarantinedAt exists, which masks the 'exempt' state returned
from computeRetentionState. Modify the effectiveStatus logic to preserve the
'exempt' status even when a candidate is quarantined, so that exemption is not
hidden in the review payload. Update the conditional to check if status is
'exempt' and maintain that value regardless of quarantinedAt, rather than
unconditionally setting it to 'quarantined'.
| ), | ||
| ]) | ||
|
|
||
| await recordRetentionAudit(orgId, id, 'exported', 'success', session.user.id, {}) |
There was a problem hiding this comment.
Fail the request when export-audit persistence fails.
Line 55 ignores the boolean from recordRetentionAudit. That can silently lose GDPR export traceability while still returning the downloadable payload.
Suggested patch
- await recordRetentionAudit(orgId, id, 'exported', 'success', session.user.id, {})
+ const auditOk = await recordRetentionAudit(orgId, id, 'exported', 'success', session.user.id, {})
+ if (!auditOk) {
+ throw createError({ statusCode: 500, statusMessage: 'Export audit logging failed' })
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await recordRetentionAudit(orgId, id, 'exported', 'success', session.user.id, {}) | |
| const auditOk = await recordRetentionAudit(orgId, id, 'exported', 'success', session.user.id, {}) | |
| if (!auditOk) { | |
| throw createError({ statusCode: 500, statusMessage: 'Export audit logging failed' }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/api/candidates/`[id]/export.get.ts at line 55, The
recordRetentionAudit function call is not checking its boolean return value,
which means audit persistence failures are silently ignored while still
returning the export payload. Modify the code to capture the return value from
recordRetentionAudit and throw an error or fail the request if the audit
recording returns false, ensuring that export operations fail when audit
traceability cannot be persisted.
| throw createError({ statusCode: 404, statusMessage: 'Not quarantined or not found' }) | ||
| } | ||
|
|
||
| await recordRetentionAudit(orgId, id, 'restored', 'success', session.user.id, {}) |
There was a problem hiding this comment.
Do not return restore success when retention-audit insert fails.
Line 36 ignores the boolean returned by recordRetentionAudit, so the endpoint may return { restored: true } without a persisted audit trail.
Suggested patch
- await recordRetentionAudit(orgId, id, 'restored', 'success', session.user.id, {})
+ const auditOk = await recordRetentionAudit(orgId, id, 'restored', 'success', session.user.id, {})
+ if (!auditOk) {
+ throw createError({ statusCode: 500, statusMessage: 'Restore completed but audit logging failed' })
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/api/candidates/`[id]/restore.post.ts at line 36, The
`recordRetentionAudit` function call on line 36 returns a boolean indicating
whether the audit record was successfully inserted, but this return value is
currently being ignored. To fix this, capture the boolean return value from
`recordRetentionAudit` and check if it was successful before returning the `{
restored: true }` response. Only allow the endpoint to return a success response
if the audit trail has been persisted to the database.
| throw createError({ statusCode: 404, statusMessage: 'Not found' }) | ||
| } | ||
|
|
||
| await recordRetentionAudit(orgId, id, exemptUntil ? 'exempted' : 'unexempted', 'success', session.user.id, {}) |
There was a problem hiding this comment.
Check audit-write success before returning exemption update success.
Line 52 does not verify recordRetentionAudit result. The hold/exemption state can change without a corresponding retained audit record.
Suggested patch
- await recordRetentionAudit(orgId, id, exemptUntil ? 'exempted' : 'unexempted', 'success', session.user.id, {})
+ const auditOk = await recordRetentionAudit(
+ orgId,
+ id,
+ exemptUntil ? 'exempted' : 'unexempted',
+ 'success',
+ session.user.id,
+ {},
+ )
+ if (!auditOk) {
+ throw createError({ statusCode: 500, statusMessage: 'Retention update applied but audit logging failed' })
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/api/candidates/`[id]/retention.patch.ts at line 52, The
recordRetentionAudit call on line 52 does not verify whether the audit record
was successfully created before returning a success response. Capture the return
value from the recordRetentionAudit invocation and check its success status
before proceeding. If the audit recording fails, handle the failure
appropriately (either by logging an error, throwing an exception, or returning
an error response to the client) to ensure that exemption state changes are only
confirmed successful when their corresponding audit records are properly
recorded.
| const existing = await db.query.orgSettings.findFirst({ | ||
| where: eq(orgSettings.organizationId, orgId), | ||
| columns: { retentionActivatedAt: true }, | ||
| }) | ||
|
|
||
| // Stamp the activation time the first time retention is turned on — this | ||
| // anchors the review window so existing data isn't deleted immediately. | ||
| const retentionActivatedAt = body.retentionEnabled === true | ||
| ? (existing?.retentionActivatedAt ?? new Date()) | ||
| : existing?.retentionActivatedAt ?? null |
There was a problem hiding this comment.
Make retentionActivatedAt assignment atomic in the upsert path.
Lines 18–27 + Line 55 perform a non-atomic read-then-write; concurrent updates can overwrite the first activation timestamp. Use a single-statement conflict update expression (e.g., COALESCE(existing_column, NOW())) so the first non-null stamp wins.
Also applies to: 55-55
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/api/org-settings/index.patch.ts` around lines 18 - 27, The
read-then-write pattern for `retentionActivatedAt` in the query at lines 18-27
followed by the database update at line 55 creates a race condition where
concurrent updates can overwrite the initial activation timestamp. Move the
timestamp logic into a single atomic database statement by using a conflict
update expression (such as COALESCE) in the upsert operation at line 55 so that
the first non-null `retentionActivatedAt` value is preserved without requiring a
separate read query, ensuring the operation is atomic at the database level.
| retentionMonths: integerFormField(z.number().int().min(1).max(120)).optional(), | ||
| quarantineDays: integerFormField(z.number().int().min(0).max(365)).optional(), | ||
| // ── Application-form privacy notice ── | ||
| privacyPolicyUrl: z.string().url().max(2000).nullish().or(z.literal('')), |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Zod v4, does z.string().url() allow non-http schemes like "javascript:" or "data:"? What is the recommended way to restrict to only http/https URLs?
💡 Result:
Yes, in Zod v4, the z.url() validator is intentionally permissive and allows non-HTTP schemes (such as mailto:, javascript:, or data:) because it relies on the native JavaScript URL constructor for validation [1][2][3]. The recommended way to restrict URLs to only http or https schemes in Zod v4 is to use the protocol parameter within the z.url() configuration [1][2][3]. Recommended usage: const httpUrl = z.url({ protocol: /^https?$/ }); schema.parse("https://example.com"); // ✅ schema.parse("http://example.com"); // ✅ schema.parse("mailto:noreply@zod.dev"); // ❌ For more comprehensive validation, such as ensuring the URL is a standard web URL with a valid domain, the Zod documentation recommends combining this with a hostname regex [2][3]: const httpUrl = z.url({ protocol: /^https?$/, hostname: z.regexes.domain }); Additionally, Zod v4 introduces z.httpUrl(), which is a specialized helper designed specifically for validating http or https URLs [2][3]. This serves as a cleaner, more concise alternative to manually configuring z.url() with protocol regexes [2][3].
Citations:
- 1: Improve
z.url(): supporthostnameandprotocolregexes colinhacks/zod#4367 - 2: https://github.com/colinhacks/zod/blob/c7805073/packages/docs/content/api.mdx
- 3: https://zod.dev/llms-full.txt
🏁 Script executed:
# Find package.json files to determine Zod version
fd -t f "package.json" | head -5Repository: reqcore-inc/reqcore
Length of output: 76
🏁 Script executed:
# Search for privacyPolicyUrl usage to confirm it's rendered as href
rg "privacyPolicyUrl" -t ts -t vue -t js -A 3 -B 1Repository: reqcore-inc/reqcore
Length of output: 91
🏁 Script executed:
# Check the orgSettings schema file to understand the broader context
head -30 server/utils/schemas/orgSettings.tsRepository: reqcore-inc/reqcore
Length of output: 1055
🏁 Script executed:
# Read package.json to check Zod version
cat package.json | grep -A 2 -B 2 "zod"Repository: reqcore-inc/reqcore
Length of output: 171
🏁 Script executed:
# Search for privacyPolicyUrl usage without vue file type restriction
rg "privacyPolicyUrl" -A 3 -B 1Repository: reqcore-inc/reqcore
Length of output: 6577
🏁 Script executed:
# Search for all z.url() usages in the codebase to see scope of issue
rg "z\.url\(" -A 1 -B 1Repository: reqcore-inc/reqcore
Length of output: 786
🏁 Script executed:
# Search for z.httpUrl usage to see if already used elsewhere
rg "z\.httpUrl\(" -A 1 -B 1Repository: reqcore-inc/reqcore
Length of output: 45
🏁 Script executed:
# Check feedback.ts to understand if currentUrl is also public-facing
rg "currentUrl" -B 5 -A 5 server/utils/schemas/feedback.tsRepository: reqcore-inc/reqcore
Length of output: 782
🏁 Script executed:
# Search for where feedback is used to see if currentUrl is sent publicly
rg "feedback" -t ts --files-with-matches | grep -E "(api|server)" | head -5Repository: reqcore-inc/reqcore
Length of output: 201
🏁 Script executed:
# Check if z.httpUrl is available in Zod 4.4.2 by looking at documentation or usage
web_search
z.httpUrl() Zod 4.4.2 availability when introducedRepository: reqcore-inc/reqcore
Length of output: 252
🌐 Web query:
z.httpUrl() Zod version 4.4.2 availability when introduced
💡 Result:
The Zod function z.httpUrl was introduced in Zod version 4, which became available to the broader ecosystem as part of the Zod 4 release [1]. While Zod 4 was initially exported as a subpath (zod/v4) alongside Zod 3.25.0, it was later published as the primary export (zod) [1]. Version 4.4.2 of Zod was released on May 1, 2026 [2][3][4]. As of this version, z.httpUrl is a stable and documented feature used for validating HTTP/HTTPS URLs [5][6]. Notably, the functionality of z.httpUrl has evolved since its introduction. Specifically, in version 4.4.0 (released April 29, 2026), strict validation was added to z.httpUrl to reject malformed HTTP(S) URLs—such as those missing a slash after the protocol (e.g., "https:example.com")—which the native URL constructor would otherwise normalize and accept [7]. The implementation of this strict check involved creating a formal regex for httpProtocol to ensure consistent enforcement [8]. Further refinements, such as allowing underscores in hostnames, have also been addressed in subsequent updates [9].
Citations:
- 1: https://colinhacks-zod.mintlify.app/migration/versioning
- 2: https://github.com/colinhacks/zod/releases/tag/v4.4.2
- 3: https://npmx.dev/package-changelog/zod/v/4.4.2
- 4: https://newreleases.io/project/npm/zod/release/4.4.2
- 5: https://zod.dev/api
- 6: https://github.com/colinhacks/zod/blob/v4.4.3/packages/docs/content/api.mdx
- 7: https://github.com/colinhacks/zod/releases/tag/v4.4.0
- 8: colinhacks/zod@3cd45eb
- 9: fix(regexes): allow underscores inside httpUrl hostname labels colinhacks/zod#6101
🏁 Script executed:
# Check feedback.post.ts to see if currentUrl is processed/stored
cat server/api/feedback.post.tsRepository: reqcore-inc/reqcore
Length of output: 7285
Use z.httpUrl() to restrict privacyPolicyUrl to HTTP/HTTPS schemes.
Line 20: z.string().url() accepts any valid URL scheme (including javascript:, data:, mailto:, etc.). Since privacyPolicyUrl is rendered directly into a public page's <a :href> (app/pages/jobs/[slug]/apply.vue), this allows protocol injection vectors.
Zod 4.4.2 includes the purpose-built z.httpUrl() validator specifically for this use case, which is cleaner and more maintainable than a custom .refine() check.
Suggested fix
- privacyPolicyUrl: z.string().url().max(2000).nullish().or(z.literal('')),
+ privacyPolicyUrl: z.httpUrl().max(2000).nullish().or(z.literal('')),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| privacyPolicyUrl: z.string().url().max(2000).nullish().or(z.literal('')), | |
| privacyPolicyUrl: z.httpUrl().max(2000).nullish().or(z.literal('')), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/utils/schemas/orgSettings.ts` at line 20, The `privacyPolicyUrl` field
in the orgSettings schema uses `z.string().url()` which accepts any URL scheme
(including potentially dangerous ones like javascript:, data:, mailto:),
creating a security risk since this URL is rendered directly in HTML. Replace
`z.string().url()` with `z.httpUrl()` in the `privacyPolicyUrl` field definition
while keeping the rest of the validation chain
(`.max(2000).nullish().or(z.literal(''))`) intact. This restricts the URL to
only HTTP/HTTPS schemes and is the recommended approach in Zod 4.4.2+.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/critical-flows/privacy-retention-gdpr.spec.ts`:
- Around line 72-94: In the backdateForRetention function, add validation at the
beginning to check if the candidateIds array is empty and throw an error if it
is to prevent undefined values in the subquery. After each SQL update statement,
verify that the command affected the expected number of rows by checking the
result returned from the postgres client, and throw an error or log a warning if
the update count is zero or unexpected to catch cases where the SQL fails
silently with invalid IDs or mismatched organization data. This ensures the SQL
operations actually succeed before the function completes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9b834fe4-bc5f-453c-9cf3-fc31da38f99c
📒 Files selected for processing (1)
e2e/critical-flows/privacy-retention-gdpr.spec.ts
| async function backdateForRetention(candidateIds: string[]) { | ||
| const sql = postgres(databaseUrl(), { max: 1 }) | ||
| const oldDate = new Date() | ||
| oldDate.setFullYear(oldDate.getFullYear() - 3) | ||
|
|
||
| try { | ||
| await sql` | ||
| update candidate | ||
| set created_at = ${oldDate}, updated_at = ${oldDate} | ||
| where id in ${sql(candidateIds)} | ||
| ` | ||
| await sql` | ||
| update org_settings | ||
| set retention_activated_at = ${oldDate} | ||
| where organization_id = ( | ||
| select organization_id from candidate where id = ${candidateIds[0]} | ||
| ) | ||
| ` | ||
| } | ||
| finally { | ||
| await sql.end() | ||
| } | ||
| } |
There was a problem hiding this comment.
Consider verifying that SQL updates affected the expected rows.
The SQL updates succeed silently even if candidateIds contains invalid IDs or the candidates don't belong to the expected organization. This could lead to false-positive test results if the backdating fails to apply. Additionally, if candidateIds is empty, candidateIds[0] returns undefined.
🛡️ Suggested improvement
async function backdateForRetention(candidateIds: string[]) {
+ if (candidateIds.length === 0) {
+ throw new Error('backdateForRetention requires at least one candidate ID')
+ }
const sql = postgres(databaseUrl(), { max: 1 })
const oldDate = new Date()
oldDate.setFullYear(oldDate.getFullYear() - 3)
try {
- await sql`
+ const candidateResult = await sql`
update candidate
set created_at = ${oldDate}, updated_at = ${oldDate}
where id in ${sql(candidateIds)}
`
- await sql`
+ if (candidateResult.count !== candidateIds.length) {
+ throw new Error(`Expected to backdate ${candidateIds.length} candidates, but updated ${candidateResult.count}`)
+ }
+ const settingsResult = await sql`
update org_settings
set retention_activated_at = ${oldDate}
where organization_id = (
select organization_id from candidate where id = ${candidateIds[0]}
)
`
+ if (settingsResult.count !== 1) {
+ throw new Error('Failed to backdate org_settings.retention_activated_at')
+ }
}
finally {
await sql.end()
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/critical-flows/privacy-retention-gdpr.spec.ts` around lines 72 - 94, In
the backdateForRetention function, add validation at the beginning to check if
the candidateIds array is empty and throw an error if it is to prevent undefined
values in the subquery. After each SQL update statement, verify that the command
affected the expected number of rows by checking the result returned from the
postgres client, and throw an error or log a warning if the update count is zero
or unexpected to catch cases where the SQL fails silently with invalid IDs or
mismatched organization data. This ensures the SQL operations actually succeed
before the function completes.
- Added candidate retention management with quarantine and purge features. - Introduced functions to find active candidates and restore them from quarantine. - Implemented retention cleanup task to automate candidate quarantine and erasure based on organizational policies. - Enhanced error handling and logging for retention operations. - Updated API endpoints to reflect changes in candidate status and retention actions. - Added tests for candidate retention lifecycle and retention cleanup functionality.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/utils/erasure.ts (1)
245-320:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPrevent dangling document rows when the purge race is lost.
Line 245-320 deletes S3 objects before the atomic guarded candidate delete. If the guard fails (Line 307-319), the transaction rolls back but S3 objects are already gone, leaving DB document rows pointing to missing files and potentially blocking future uploads via stale document counts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/utils/erasure.ts` around lines 245 - 320, The S3 deletion in Step 1 happens before the atomic database transaction in Step 2, which means S3 objects are deleted even if the transaction fails due to the purge guard (when PurgeNoLongerEligibleError is thrown). Move the S3 deletion loop that calls deleteFromS3 to execute after the database transaction block completes successfully, ensuring that S3 objects are only deleted when the database deletion is confirmed. This prevents dangling document rows in the database that point to missing S3 files.
🧹 Nitpick comments (1)
DATA-RETENTION.md (1)
61-65: ⚡ Quick winAdd language specifier to fenced code block.
The code block at line 61 is missing a language label. Add a language identifier (e.g.,
bashorcurl) to improve syntax highlighting and comply with markdown linting standards.📝 Proposed fix
-``` +```bash POST /api/admin/retention-cleanup🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DATA-RETENTION.md` around lines 61 - 65, Add a language specifier to the fenced code block containing the retention cleanup API endpoint documentation. Change the opening fence from ``` to ```bash to enable proper syntax highlighting and comply with markdown linting standards. The code block displays the POST /api/admin/retention-cleanup endpoint with headers and optional body parameters.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/components/CandidateDetailSidebar.vue`:
- Around line 641-649: The hardcoded English strings "Data retention" and
"Quarantined · purge" in the CandidateDetailSidebar.vue component bypass the
i18n localization system. Replace the literal string "Data retention" in the dt
element with a reference to an i18n key using the appropriate Vue i18n syntax
(such as $t()). Similarly, replace the hardcoded "Quarantined · purge" template
text with a corresponding i18n key reference. Ensure these new i18n keys are
defined in the locale files alongside the existing retention keys mentioned in
the PR to maintain consistent localization across the UI.
In `@server/api/public/jobs/`[slug]/apply.post.ts:
- Around line 412-414: The restoreCandidateForPublicApplication function returns
a boolean indicating success or failure, but this return value is currently
being ignored in the if (restoreFromQuarantine) block. This creates a TOCTOU
vulnerability where if the restore operation fails (due to candidate state
changes), the code continues with stale state. Capture the boolean return value
from restoreCandidateForPublicApplication and gate the continuation of the flow
on successful restoration; if the restore fails, handle the failure
appropriately by returning an error or stopping further execution rather than
proceeding with the application creation.
In `@server/utils/candidate-retention.ts`:
- Around line 46-49: The recordRetentionAudit call on line 46 returns a boolean
indicating success or failure, but the return value is not being checked. Modify
the code to capture and validate the boolean return value from the
recordRetentionAudit function call, and add error handling logic to ensure that
if the audit write fails, the restoration process either stops gracefully or at
minimum logs an appropriate error message to maintain compliance observability.
Do not allow the restore operation to complete silently when the audit write
fails.
---
Outside diff comments:
In `@server/utils/erasure.ts`:
- Around line 245-320: The S3 deletion in Step 1 happens before the atomic
database transaction in Step 2, which means S3 objects are deleted even if the
transaction fails due to the purge guard (when PurgeNoLongerEligibleError is
thrown). Move the S3 deletion loop that calls deleteFromS3 to execute after the
database transaction block completes successfully, ensuring that S3 objects are
only deleted when the database deletion is confirmed. This prevents dangling
document rows in the database that point to missing S3 files.
---
Nitpick comments:
In `@DATA-RETENTION.md`:
- Around line 61-65: Add a language specifier to the fenced code block
containing the retention cleanup API endpoint documentation. Change the opening
fence from ``` to ```bash to enable proper syntax highlighting and comply with
markdown linting standards. The code block displays the POST
/api/admin/retention-cleanup endpoint with headers and optional body parameters.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dff1de7d-87ca-4bc6-8322-205c4a5ed340
📒 Files selected for processing (35)
.env.exampleCHANGELOG.mdDATA-RETENTION.mdSELF-HOSTING.mdapp/components/CandidateDetailSidebar.vueapp/components/SettingsSidebar.vueapp/pages/dashboard/settings/retention.vueapp/pages/jobs/[slug]/apply.vuee2e/critical-flows/privacy-retention-gdpr.spec.tsi18n/locales/de.jsoni18n/locales/en.jsoni18n/locales/es.jsoni18n/locales/fr.jsoni18n/locales/nb.jsoni18n/locales/vi.jsonnuxt.config.tsserver/api/admin/retention-cleanup.post.tsserver/api/admin/retention/review.get.tsserver/api/applications/index.post.tsserver/api/candidates/[id].delete.tsserver/api/candidates/[id].get.tsserver/api/candidates/[id].patch.tsserver/api/candidates/[id]/documents/index.post.tsserver/api/candidates/[id]/restore.post.tsserver/api/candidates/[id]/retention.patch.tsserver/api/org-settings/index.patch.tsserver/api/public/jobs/[slug]/apply.post.tsserver/tasks/retention-cleanup.tsserver/utils/candidate-retention.tsserver/utils/env.tsserver/utils/erasure.tsserver/utils/retention-cleanup.tstests/unit/candidate-retention.test.tstests/unit/erasure.test.tstests/unit/retention-cleanup.test.ts
✅ Files skipped from review due to trivial changes (3)
- server/tasks/retention-cleanup.ts
- CHANGELOG.md
- SELF-HOSTING.md
🚧 Files skipped from review as they are similar to previous changes (9)
- app/components/SettingsSidebar.vue
- app/pages/jobs/[slug]/apply.vue
- server/api/candidates/[id].delete.ts
- server/api/candidates/[id]/restore.post.ts
- server/api/candidates/[id]/retention.patch.ts
- server/api/org-settings/index.patch.ts
- server/api/admin/retention/review.get.ts
- e2e/critical-flows/privacy-retention-gdpr.spec.ts
- app/pages/dashboard/settings/retention.vue
| <dt class="text-xs font-medium text-surface-400 dark:text-surface-500 mb-1"> | ||
| Data retention | ||
| </dt> | ||
| <dd class="text-surface-800 dark:text-surface-200 font-medium"> | ||
| <template v-if="candidateData.retention.quarantinedAt"> | ||
| Quarantined · purge {{ formatDateTime(candidateData.retention.scheduledPurgeAt) }} | ||
| </template> | ||
| <template v-else> | ||
| {{ candidateData.retention.status }} · {{ formatDateTime(candidateData.retention.expiresAt) }} |
There was a problem hiding this comment.
Localize the new retention labels instead of hardcoding English text.
Line 641 and Line 646 introduce hardcoded strings ("Data retention" and "Quarantined · purge"), which will bypass locale selection while this PR already ships localized retention keys. Please route these labels through i18n keys to avoid mixed-language UI.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/components/CandidateDetailSidebar.vue` around lines 641 - 649, The
hardcoded English strings "Data retention" and "Quarantined · purge" in the
CandidateDetailSidebar.vue component bypass the i18n localization system.
Replace the literal string "Data retention" in the dt element with a reference
to an i18n key using the appropriate Vue i18n syntax (such as $t()). Similarly,
replace the hardcoded "Quarantined · purge" template text with a corresponding
i18n key reference. Ensure these new i18n keys are defined in the locale files
alongside the existing retention keys mentioned in the PR to maintain consistent
localization across the UI.
| if (restoreFromQuarantine) { | ||
| await restoreCandidateForPublicApplication(orgId, candidateId) | ||
| } |
There was a problem hiding this comment.
Gate continuation on restore success to avoid TOCTOU failures.
Line 412-414 ignores the boolean from restoreCandidateForPublicApplication. If candidate state changes between the earlier read and this restore attempt, the flow can continue with stale state and fail later during application creation.
Suggested fix
if (restoreFromQuarantine) {
- await restoreCandidateForPublicApplication(orgId, candidateId)
+ const restored = await restoreCandidateForPublicApplication(orgId, candidateId)
+ if (!restored) {
+ throw createError({ statusCode: 409, statusMessage: 'Candidate state changed; please retry' })
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (restoreFromQuarantine) { | |
| await restoreCandidateForPublicApplication(orgId, candidateId) | |
| } | |
| if (restoreFromQuarantine) { | |
| const restored = await restoreCandidateForPublicApplication(orgId, candidateId) | |
| if (!restored) { | |
| throw createError({ statusCode: 409, statusMessage: 'Candidate state changed; please retry' }) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/api/public/jobs/`[slug]/apply.post.ts around lines 412 - 414, The
restoreCandidateForPublicApplication function returns a boolean indicating
success or failure, but this return value is currently being ignored in the if
(restoreFromQuarantine) block. This creates a TOCTOU vulnerability where if the
restore operation fails (due to candidate state changes), the code continues
with stale state. Capture the boolean return value from
restoreCandidateForPublicApplication and gate the continuation of the flow on
successful restoration; if the restore fails, handle the failure appropriately
by returning an error or stopping further execution rather than proceeding with
the application creation.
| await recordRetentionAudit(orgId, candidateId, 'restored', 'success', null, { | ||
| source: 'public_application', | ||
| }) | ||
| logInfo('retention.candidate_restored_on_application', { |
There was a problem hiding this comment.
Handle failed audit writes during restore.
Line 46-49 ignores the boolean returned by recordRetentionAudit. If the insert fails, restore succeeds silently without audit trace, which weakens compliance observability.
Suggested fix
- await recordRetentionAudit(orgId, candidateId, 'restored', 'success', null, {
+ const audited = await recordRetentionAudit(orgId, candidateId, 'restored', 'success', null, {
source: 'public_application',
})
+ if (!audited) {
+ logError('retention.restore_audit_write_failed', {
+ org_id: orgId,
+ candidate_id: candidateId,
+ })
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await recordRetentionAudit(orgId, candidateId, 'restored', 'success', null, { | |
| source: 'public_application', | |
| }) | |
| logInfo('retention.candidate_restored_on_application', { | |
| const audited = await recordRetentionAudit(orgId, candidateId, 'restored', 'success', null, { | |
| source: 'public_application', | |
| }) | |
| if (!audited) { | |
| logError('retention.restore_audit_write_failed', { | |
| org_id: orgId, | |
| candidate_id: candidateId, | |
| }) | |
| } | |
| logInfo('retention.candidate_restored_on_application', { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/utils/candidate-retention.ts` around lines 46 - 49, The
recordRetentionAudit call on line 46 returns a boolean indicating success or
failure, but the return value is not being checked. Modify the code to capture
and validate the boolean return value from the recordRetentionAudit function
call, and add error handling logic to ensure that if the audit write fails, the
restoration process either stops gracefully or at minimum logs an appropriate
error message to maintain compliance observability. Do not allow the restore
operation to complete silently when the audit write fails.
…irmation and settings
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/api/candidates/[id].delete.ts (1)
86-91: 💤 Low valueUnused
eventparameter can be removed.The
event: H3Eventparameter is never referenced in the function body. Removing it simplifies the signature.♻️ Suggested diff
async function softDeleteCandidate( - event: H3Event, orgId: string, id: string, actorId: string, ) {And at the call site (line 29):
- return softDeleteCandidate(event, orgId, id, session.user.id) + return softDeleteCandidate(orgId, id, session.user.id)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/api/candidates/`[id].delete.ts around lines 86 - 91, The `event: H3Event` parameter in the `softDeleteCandidate` function is never used within the function body and should be removed. Remove the `event: H3Event` parameter from the function signature of `softDeleteCandidate`, and then update the function call site (at line 29) to not pass the event argument when invoking `softDeleteCandidate`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@server/api/candidates/`[id].delete.ts:
- Around line 86-91: The `event: H3Event` parameter in the `softDeleteCandidate`
function is never used within the function body and should be removed. Remove
the `event: H3Event` parameter from the function signature of
`softDeleteCandidate`, and then update the function call site (at line 29) to
not pass the event argument when invoking `softDeleteCandidate`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4f02f4da-6720-4fa9-80ac-1001c3369657
📒 Files selected for processing (12)
.env.exampleDATA-RETENTION.mdSELF-HOSTING.mdapp/pages/dashboard/candidates/[id].vueapp/pages/dashboard/settings/retention.vuei18n/locales/en.jsoni18n/locales/es.jsoni18n/locales/fr.jsoni18n/locales/nb.jsoni18n/locales/vi.jsonserver/api/candidates/[id].delete.tsserver/utils/env.ts
✅ Files skipped from review due to trivial changes (8)
- .env.example
- app/pages/dashboard/candidates/[id].vue
- i18n/locales/es.json
- SELF-HOSTING.md
- i18n/locales/fr.json
- i18n/locales/nb.json
- i18n/locales/en.json
- DATA-RETENTION.md
🚧 Files skipped from review as they are similar to previous changes (2)
- i18n/locales/vi.json
- app/pages/dashboard/settings/retention.vue
…e legal hold handling in delete API
Signed-off-by: Joachim LK <joachim.l.kolle.pers@gmail.com>
eraseCandidatesfunction to permanently delete candidates and their associated data, ensuring S3 objects are deleted first to prevent data leakage.retention.tsfor computing retention states and eligibility for purging candidates based on GDPR compliance.Summary
Type of change
Validation
DCO
Signed-off-by) viagit commit -sSummary by CodeRabbit