Skip to content

fix(db): exclude webhook_events from destructive retention policy - #666

Merged
JSONbored merged 2 commits into
mainfrom
codex/fix-webhook-replay-cache-pruning-issue
Jun 14, 2026
Merged

fix(db): exclude webhook_events from destructive retention policy#666
JSONbored merged 2 commits into
mainfrom
codex/fix-webhook-replay-cache-pruning-issue

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • Prevent deletion of webhook delivery idempotency records that the webhook handler relies on to suppress duplicate deliveries. Removing these rows enables replay attacks of previously valid signed webhooks.

Description

  • Remove webhook_events from the RETENTION_POLICY in src/db/retention.ts so replay-prevention state is not eligible for destructive pruning.
  • Clarify the retention comment to call out webhook delivery idempotency records as intentionally EXCLUDED from pruning in src/db/retention.ts.
  • Update test/unit/retention.test.ts to assert that webhook_events is excluded from dry-run results, scheduled pruning, and the preview route, and to exercise pruning logic against ai_usage_events instead of webhook_events.
  • Keep the existing batched delete, identifier guard, and audit behavior unchanged for other append-only tables.

Testing

  • Ran the unit test file test/unit/retention.test.ts with npm test -- --run test/unit/retention.test.ts; all tests passed.
  • Ran TypeScript type-check with npm run typecheck; it completed with no errors.
  • Verified the modified tests reflect that webhook_events is no longer targeted by the default retention policy.

Codex Task

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@ghost

ghost commented Jun 14, 2026

Copy link
Copy Markdown

Note

Gittensory Gate skipped

PR closed before full evaluation. No late first comment was created.

Signal Result Evidence Action
Gate result ⚠️ Skipped #666 is no longer open. No action.

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

@ghost ghost added the gittensory:reviewed label Jun 14, 2026
@ghost

ghost commented Jun 14, 2026

Copy link
Copy Markdown

gittensory · advisory review

Reviewed 2 changed file(s) — two independent AI reviewers.

Suggested action:Safe to merge — both reviewers found no blocking issues.

Reviewer A · gpt-oss-120b — recommends ✅ merge
The PR removes webhook_events from the default retention policy and updates related tests to reflect that webhook events are now considered durable idempotency records and should not be pruned. The changes are straightforward, well‑documented, and the test suite has been adjusted accordingly.

Suggestions

  • Add a brief comment in the retention module explaining why webhook_events are excluded (e.g., for replay/idempotency safety).
  • Consider documenting this policy change in the project README or migration guide to inform operators about potential DB growth.

Worth double-checking

  • Long‑term accumulation of webhook_events could increase DB storage; ensure monitoring/alerts are in place.
  • Any external scripts that relied on the previous 30‑day prune for webhook_events may need to be updated.

Reviewer B · nemotron-3-120b-a12b — recommends ✅ merge
The PR removes the webhook_events table from the retention policy, preventing its deletion after 30 days. The change is justified as webhook_events are durable replay/idempotency records requiring preservation. The test suite is updated consistently to reflect that webhook_events are no longer pruned, and all tests pass. No public/private boundary concerns are introduced.

Suggestions

  • Consider adding a brief comment in the RETENTION_POLICY array near the removed line to explain why webhook_events were excluded (though the JSDoc above already covers this).
  • The test file's removal of the unused eq import is appropriate and improves clarity.

Worth double-checking

  • Verify that webhook_events growth remains bounded (e.g., via idempotency key limits or upstream deduplication) to prevent uncontrolled storage accumulation.
  • Confirm no other retention policies or external processes implicitly rely on webhook_events deletion for cleanup.

@ghost ghost added the gittensory-review label Jun 14, 2026
@JSONbored JSONbored self-assigned this Jun 14, 2026
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 14, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui dd220eb Commit Preview URL

Branch Preview URL
Jun 14 2026, 02:15 PM

@JSONbored
JSONbored merged commit 67c11f2 into main Jun 14, 2026
11 checks passed
@JSONbored
JSONbored deleted the codex/fix-webhook-replay-cache-pruning-issue branch June 14, 2026 14:22
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jun 14, 2026
JSONbored added a commit that referenced this pull request Jul 7, 2026
Every table originally flagged as unbounded in #3896 except one turned
out already handled: audit_events/ai_usage_events/product_usage_events
already have cron-scheduled retention via RETENTION_POLICY (the
original audit's grep for literal .delete() calls missed the dynamic
DELETE FROM ${table} SQL this module uses), and webhook_events is
deliberately excluded per a prior fix (#666) since GitHub can redeliver
a webhook long after the original event and this table is the
idempotency record that detects that -- adding it back would
reintroduce that fixed bug.

agent_context_snapshots (one payloadJson blob per agent run, capped
only by a per-runId .limit(50) READ, never deleted) was genuinely the
only unaddressed table. Add it to RETENTION_POLICY with a 30-day
window, reusing the existing generic prune mechanism.

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

Labels

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant