Skip to content

release: v1.25.0 — DuckDB excerpt encryption (SEC-6) + doc-truth fixes - #303

Merged
qnbs merged 11 commits into
mainfrom
feat/duckdb-excerpt-encryption-v1.25.0
Jul 31, 2026
Merged

release: v1.25.0 — DuckDB excerpt encryption (SEC-6) + doc-truth fixes#303
qnbs merged 11 commits into
mainfrom
feat/duckdb-excerpt-encryption-v1.25.0

Conversation

@qnbs

@qnbs qnbs commented Jul 31, 2026

Copy link
Copy Markdown
Owner

Summary

Release-cut PR for v1.25.0, bundling the remaining SEC-6 security work and doc-truth cleanup that followed the Grok/Claude/Browser-Ollama provider PRs (#300, #301, #302).

  • feat(security): codex_mentions.excerpt (the one DuckDB analytics column holding literal manuscript prose) is now encrypted at rest with AES-256-GCM when enableIdbAtRestEncryption is active, reusing the existing IDB at-rest encryption key via duckdbEncryption.ts. New excerpt_enc BLOB column (schema v3), backfill migration (codexExcerptEncryptionMigration.ts) for pre-existing plaintext rows, write-path gating in duckdbCodexWrite(). Tests exercise the real AES-256-GCM crypto path (only the DuckDB transport is mocked).
  • docs: Fixed a stale "Plan only — do not implement yet" status header in GROK-PROVIDER-INTEGRATION-PLAN.md (all phases were already merged), archived the completed provider workstream from TODO.md to docs/history/completed-v1.25.0-providers.md per the existing archival convention.
  • docs(security): Corrected the SEC-6 status rows in .github/SECURITY.md and docs/SECURITY-THREAT-MODEL.md to reflect the new cell-level encryption (and that full OPFS file-level encryption is an accepted, permanent limitation — DuckDB-WASM owns the OPFS file handle directly). Added a short note on why the Claude serverless proxy has no in-app request monitoring (would violate its stateless zero-console-call guarantee) and pointing at platform-native Vercel/Cloudflare analytics instead.
  • chore(release): Cut [1.25.0] in CHANGELOG.md, bumped package.json / README badge / AGENTS.md to 1.25.0, synced src-tauri/Cargo.toml, src-tauri/tauri.conf.json, and public/sw.js via the existing sync scripts.

Verification (local quick-tier, per repo's low-end-hardware policy)

  • pnpm run lint — clean
  • pnpm run typecheck — clean (tsgo, 4 checkers)
  • pnpm run i18n:check — clean, 2861 keys × 19 locales, no content drift
  • pnpm exec tsx scripts/audit-feature-parity.ts — 0 drifts, 23 flags consistent
  • node scripts/check-doc-metrics.mjs — clean
  • Targeted vitest run across all touched test files — all green (real-crypto assertions for the new encryption paths, no over-mocking)
  • Full CI (security / quality ×2 / build / e2e / e2e-deep / storybook / lighthouse / vrt) — pending, will monitor after opening this PR

Not included in this PR

The final git tag v1.25.0 + gh release create step is intentionally not part of this PR — per the release plan, that hand-off requires explicit maintainer confirmation and happens only after this PR is merged with fully green CI.

Summary by CodeRabbit

  • Security

    • Codex manuscript excerpts are now encrypted at rest when browser encryption is enabled.
    • Existing excerpts are migrated automatically with privacy safeguards and safe recovery if interrupted.
    • Full database-file and metadata encryption remain unsupported.
  • Documentation

    • Updated security guidance, release records, and version information for 1.25.0.
    • Documented Grok, Claude, and opt-in browser Ollama integrations as completed for the release.

qnbs added 4 commits July 31, 2026 21:56
…ytics (SEC-6)

Adds cell-level AES-256-GCM encryption for the one DuckDB analytics column that holds
literal manuscript prose. When enableIdbAtRestEncryption is active, duckdbCodexWrite()
encrypts new excerpts into a new excerpt_enc BLOB column (via the existing
duckdbEncryption.ts facade, reusing the IDB at-rest encryption key) and nulls the
plaintext excerpt column; a new codexExcerptEncryptionMigration.ts backfills any
pre-existing plaintext rows once encryption is unlocked, without blocking the schema's
other migrations for the common case where encryption isn't active this session.

Schema bumped to v3 (DUCKDB_MIGRATION_V3_DDL adds codex_mentions.excerpt_enc). Full
OPFS file-level encryption remains infeasible since DuckDB-WASM owns the OPFS file
handle directly; other metadata columns stay intentionally plaintext.

Tests exercise the real AES-256-GCM crypto path (only duckdbClient transport is
mocked), per the lesson recorded in TODO.md from a prior over-mocking incident that
hid a missing ensureInferencePool export.
…stream

- GROK-PROVIDER-INTEGRATION-PLAN.md still said 'Plan only — do not implement yet'
  despite all phases being merged; updated status header to reflect completion.
- TODO.md's Native Grok + Claude + Ollama-in-PWA section had a stray bullet
  referencing WS-11 (an unrelated, already-closed 07-29 workstream); replaced with
  the actual remaining release-cut task and archived the completed checklist to
  docs/history/completed-v1.25.0-providers.md, matching the completed-v1.22.md
  archival convention.
…dance

- .github/SECURITY.md / docs/SECURITY-THREAT-MODEL.md SEC-6 rows updated to reflect
  that codex_mentions.excerpt is now encrypted at rest when enableIdbAtRestEncryption
  is active (see the DuckDB excerpt-encryption commit); clarified that full OPFS
  file-level encryption is an accepted, permanent limitation rather than pending work,
  and that other DuckDB metadata columns remain intentionally plaintext.
- docs/SECURITY-THREAT-MODEL.md's Claude serverless proxy section now documents why
  in-app request monitoring is intentionally absent (would violate the proxy's
  zero-console-call stateless guarantee) and recommends platform-native Vercel/
  Cloudflare request analytics instead — doc-only, no code.
- CHANGELOG.md: moved [Unreleased] into a dated [1.25.0] section, added a Security
  entry for the DuckDB codex_mentions.excerpt encryption fix and a Docs entry for the
  SEC-6/Claude-proxy-monitoring/GROK-plan doc-truth fixes; fresh empty [Unreleased]
  header restored above it.
- package.json, README.md version badge, AGENTS.md: 1.24.3 → 1.25.0.
- src-tauri/Cargo.toml, src-tauri/tauri.conf.json, public/sw.js: synced via
  scripts/sync-tauri-version.mjs + scripts/sync-sw-version.mjs.

Native Grok/Claude cloud providers, opt-in Browser-Ollama, and DuckDB excerpt
encryption (SEC-6) all ship in this release.
Copilot AI review requested due to automatic review settings July 31, 2026 19:58
@vercel

vercel Bot commented Jul 31, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
worldscript-studio Ready Ready Preview Jul 31, 2026 9:41pm

@codeant-ai

codeant-ai Bot commented Jul 31, 2026

Copy link
Copy Markdown

🏁 CodeAnt Quality Gate Results

Commit: fdf47d97
Scan Time: 2026-07-31 21:12:14 UTC

✅ Overall Status: PASSED

Quality Gate Details

Quality Gate Status Details
Secrets ✅ PASSED 0 secrets found
Duplicate Code ✅ PASSED 0.0% duplicated
SAST ✅ PASSED No security issues
Bugs ✅ PASSED Rating S: No bugs
IAC ✅ PASSED Rating S: No issues

View Full Results

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 25 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d726a1fa-c256-4b09-9f99-402d41e5d518

📥 Commits

Reviewing files that changed from the base of the PR and between fdf47d9 and f91574c.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • services/duckdb/codexExcerptEncryptionMigration.ts
  • tests/unit/services/duckdb/codexExcerptEncryptionMigration.test.ts
📝 Walkthrough

Walkthrough

The PR releases v1.25.0, adds DuckDB schema v3 support for encrypted Codex excerpts, encrypts new and existing excerpts when enabled, wires migration execution into analytics backfill, expands tests, and updates release and security documentation.

Changes

Codex excerpt encryption

Layer / File(s) Summary
DuckDB schema v3 and bootstrap
services/duckdb/duckdbSchema.ts, hooks/useDuckDb.ts, tests/unit/services/duckdbSchema.test.ts, tests/unit/hooks/useDuckDb.test.ts
DuckDB schema version 3 adds the excerpt_enc BLOB column. Bootstrap executes the v3 migration and tests validate schema and failure handling.
Encrypted Codex mention writes
services/duckdb/duckdbAnalytics.ts, tests/unit/duckdbAnalytics.test.ts
Codex writes conditionally encrypt excerpts, pass ciphertext as SQL parameters, clear plaintext storage, and preserve existing ciphertext during conflicts.
Excerpt backfill migration and listener wiring
services/duckdb/codexExcerptEncryptionMigration.ts, services/duckdb/duckdbListenerLoader.ts, app/listenerMiddleware.ts, tests/unit/services/duckdb/*, tests/unit/duckdbListenerLoader.test.ts, tests/unit/listenerMiddleware.test.ts
The migration processes existing plaintext rows with readiness and privacy-gate checks, handles aborts and failures, records completion, and runs during analytics backfill.

Release and security records

Layer / File(s) Summary
v1.25.0 release and security records
.github/SECURITY.md, AGENTS.md, CHANGELOG.md, GROK-PROVIDER-INTEGRATION-PLAN.md, README.md, TODO.md, docs/SECURITY-THREAT-MODEL.md, docs/history/completed-v1.25.0-providers.md, package.json, public/sw.js, src-tauri/Cargo.toml, src-tauri/tauri.conf.json
Version metadata and release records reference v1.25.0. Security documentation describes encrypted excerpts, plaintext metadata, unsupported OPFS file encryption, and platform-hosted Claude proxy monitoring.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ListenerMiddleware
  participant DuckDBListenerLoader
  participant CodexExcerptEncryptionMigration
  participant IDBEncryptionService
  participant DuckDB
  ListenerMiddleware->>DuckDBListenerLoader: Load migration module
  DuckDBListenerLoader->>CodexExcerptEncryptionMigration: Return migration function
  ListenerMiddleware->>CodexExcerptEncryptionMigration: Run migration with privacy gate
  CodexExcerptEncryptionMigration->>IDBEncryptionService: Check encryption readiness
  CodexExcerptEncryptionMigration->>DuckDB: Read plaintext excerpts
  CodexExcerptEncryptionMigration->>IDBEncryptionService: Encrypt excerpts
  CodexExcerptEncryptionMigration->>DuckDB: Store ciphertext and clear plaintext
  CodexExcerptEncryptionMigration->>DuckDB: Record completion marker
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the v1.25.0 release, DuckDB excerpt encryption, and documentation corrections.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/duckdb-excerpt-encryption-v1.25.0

Comment @coderabbitai help to get the list of available commands.

OSV scan flagged event-listener 5.4.1 as vulnerable (RUSTSEC-2026-0221), fixed
upstream in 5.4.2. Transitive dependency only; cargo update -p pins the patched
version with no other lockfile churn.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Release-cut for v1.25.0, primarily finalizing SEC-6 DuckDB prose-excerpt at-rest encryption and syncing doc-truth + version bumps across the repo.

Changes:

  • Adds cell-level AES-256-GCM encryption for codex_mentions.excerpt via new excerpt_enc BLOB, including a backfill migration and wiring into the DuckDB codex write path.
  • Updates analytics boot/migration wiring and expands unit tests to exercise real crypto (DuckDB transport mocked only).
  • Cuts the 1.25.0 release across changelog/docs/versioned artifacts (package, SW, Tauri, README, security docs, TODO archival).

Reviewed changes

Copilot reviewed 24 out of 25 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
services/duckdb/duckdbAnalytics.ts Encrypts codex_mentions.excerpt into excerpt_enc when IDB encryption is ready; updates exec helper to accept params.
services/duckdb/codexExcerptEncryptionMigration.ts Adds one-time migration to encrypt historical plaintext codex excerpts into excerpt_enc.
services/duckdb/duckdbSchema.ts Bumps schema version to v3; adds excerpt_enc BLOB and v3 DDL migration.
services/duckdb/duckdbListenerLoader.ts Adds dynamic loader for the new excerpt encryption migration.
app/listenerMiddleware.ts Wires the new migration into the analytics-migration pipeline and abort-handling.
hooks/useDuckDb.ts Runs v3 DuckDB migration DDL during bootstrap.
tests/unit/duckdbAnalytics.test.ts Adds real-crypto tests for excerpt encryption gating on the write path.
tests/unit/services/duckdbSchema.test.ts Updates schema expectations for v3 and the new excerpt_enc column/migration DDL.
tests/unit/services/duckdb/codexExcerptEncryptionMigration.test.ts New test file validating the migration with real AES-GCM encryption output.
tests/unit/duckdbListenerLoader.test.ts Adds coverage for the new migration loader.
tests/unit/hooks/useDuckDb.test.ts Updates DuckDB schema mocks to include v3 DDL.
CHANGELOG.md Adds the [1.25.0] release section, including SEC-6 excerpt encryption notes.
TODO.md Archives completed provider cycle work; adds v1.25.0 release publish checklist.
docs/history/completed-v1.25.0-providers.md New archival doc for completed v1.25.0 provider + SEC-6 checklist.
docs/SECURITY-THREAT-MODEL.md Updates SEC-6 description to reflect excerpt cell encryption + accepted OPFS limitation; adds proxy monitoring note.
.github/SECURITY.md Updates supported versions and SEC-6 status text to reflect excerpt encryption + accepted OPFS limitation.
GROK-PROVIDER-INTEGRATION-PLAN.md Fixes stale “plan only” header; marks as complete historical record.
package.json Bumps version to 1.25.0.
public/sw.js Bumps APP_VERSION to 1.25.0.
README.md Updates version badge to v1.25.0.
AGENTS.md Updates documented version to 1.25.0.
src-tauri/Cargo.toml Bumps Tauri package version to 1.25.0.
src-tauri/tauri.conf.json Bumps Tauri app version to 1.25.0.

Comment thread services/duckdb/codexExcerptEncryptionMigration.ts Outdated
Comment thread services/duckdb/duckdbAnalytics.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 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 `@docs/SECURITY-THREAT-MODEL.md`:
- Around line 227-236: Update the “Monitoring / anomaly detection” section to
name Vercel Function Logs or Vercel Observability for Vercel deployments, and
Cloudflare Workers Metrics/Analytics plus Workers Logs for Cloudflare Pages.
Explicitly state that Cloudflare Workers observability must be enabled, while
preserving the guidance that these services monitor request counts, status
codes, and latency without adding application logging.

In `@services/duckdb/codexExcerptEncryptionMigration.ts`:
- Line 12: Update runCodexExcerptEncryptionMigration and its
CODEX_EXCERPT_ENCRYPTION_KEY usage so the migration marker is stored and checked
per projectId rather than globally, allowing each project to migrate
independently. Add a regression test covering two projects and verifying both
are migrated; leave the separate ragVectorMigration.ts marker issue for its own
fix.

In `@TODO.md`:
- Around line 19-22: Remove or rewrite the release checklist entry in TODO.md so
it no longer contains the v1.25.0 tag, release, or publish references that
scanForDrift() detects. Preserve any necessary operational-safety guidance
without leaving an open bullet matching the post-tag documentation check.
🪄 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: 4703bf73-5cea-40aa-b038-11c7e2bcf51b

📥 Commits

Reviewing files that changed from the base of the PR and between 5f7d2e9 and 51cd0cc.

📒 Files selected for processing (24)
  • .github/SECURITY.md
  • AGENTS.md
  • CHANGELOG.md
  • GROK-PROVIDER-INTEGRATION-PLAN.md
  • README.md
  • TODO.md
  • app/listenerMiddleware.ts
  • docs/SECURITY-THREAT-MODEL.md
  • docs/history/completed-v1.25.0-providers.md
  • hooks/useDuckDb.ts
  • package.json
  • public/sw.js
  • services/duckdb/codexExcerptEncryptionMigration.ts
  • services/duckdb/duckdbAnalytics.ts
  • services/duckdb/duckdbListenerLoader.ts
  • services/duckdb/duckdbSchema.ts
  • src-tauri/Cargo.toml
  • src-tauri/tauri.conf.json
  • tests/unit/duckdbAnalytics.test.ts
  • tests/unit/duckdbListenerLoader.test.ts
  • tests/unit/hooks/useDuckDb.test.ts
  • tests/unit/listenerMiddleware.test.ts
  • tests/unit/services/duckdb/codexExcerptEncryptionMigration.test.ts
  • tests/unit/services/duckdbSchema.test.ts

Comment thread docs/SECURITY-THREAT-MODEL.md
Comment thread services/duckdb/codexExcerptEncryptionMigration.ts
Comment thread TODO.md Outdated
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.07692% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
services/duckdb/codexExcerptEncryptionMigration.ts 97.43% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

qnbs added 2 commits July 31, 2026 22:24
- codexExcerptEncryptionMigration.ts: isCodexExcerptEncryptionMigrationDone()
  now takes a projectId and scopes the _meta done-marker key per project.
  Previously the marker was a single global key, so once ANY project
  completed the backfill, every other project's plaintext excerpts were
  silently never migrated. Corrected the aborted:true doc comment (can
  also result from the initial SELECT query failing, not just a privacy
  opt-out). (CodeRabbit + Copilot)
- duckdbAnalytics.ts: duckdbCodexWrite()'s ON CONFLICT clause for
  codex_mentions no longer lets an unencrypted write (session where IDB
  encryption isn't unlocked) clobber an existing ciphertext row back to
  plaintext; uses COALESCE/CASE to always preserve prior excerpt_enc.
  (Copilot)
- Added a two-project regression test proving the marker fix, and a test
  asserting the ON CONFLICT clause never downgrades ciphertext.
- TODO.md: reword the open v1.25.0 tag/publish bullet so it no longer
  embeds the literal version number on the '- ⬜' bullet line itself,
  removing the risk that scripts/check-doc-metrics.mjs's scanForDrift()
  would flag it as stale drift once the v1.25.0 tag is actually pushed.
  Verified via a scanForDrift() simulation with latestVersion=1.25.0:
  zero findings.
- docs/SECURITY-THREAT-MODEL.md: name the specific platform observability
  products (Vercel Function Logs / Vercel Observability; Cloudflare
  Workers Metrics/Analytics and Workers Logs) instead of vague
  'platform-native analytics', and note that Cloudflare Workers Logs is
  opt-in and must be enabled via the Wrangler observability config.
@qnbs

qnbs commented Jul 31, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

qnbs added 2 commits July 31, 2026 22:45
… wiring

- listenerMiddleware: wire the analytics reducer into the test store and
  fix stale duckdbMigration/ragVectorMigration mocks (real listener calls
  runMigrationWithRollback, not runIfNeeded; RAG mock now resolves
  {aborted:false} instead of undefined) so the previously-never-exercised
  DuckDB migration listener effect can run. Add 2 tests covering the
  codexEncResult.aborted branch that gates migrationStatus done/idle.
- codexExcerptEncryptionMigration: add tests for the post-loop
  shouldPersist() abort, a failed row UPDATE (migrated not incremented),
  and the modulo-10 Promise.resolve() yield point.
- useDuckDb: add a test for the v3 migration DDL failure branch
  (bootstrapDuckDbSchema throws 'DuckDB v3 migration DDL failed').
…ion mock

The abort-path test for the DuckDB analytics migration listener mocked
loadCodexExcerptEncryptionMigration with only runCodexExcerptEncryptionMigration,
missing the required isCodexExcerptEncryptionMigrationDone export. This broke
tsgo --noEmit in CI (TS2741) on Node 22/24 Quality Gate. Also reverts a stray
4-space indent that crept into the featureFlagsActions import.
@qnbs

qnbs commented Jul 31, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
services/duckdb/codexExcerptEncryptionMigration.ts (1)

77-107: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Fix: a failed row UPDATE still lets the migration mark itself complete, permanently leaving plaintext excerpts.

When updRes.ok is false, the code does not record the failure. migrated isn't incremented, but nothing prevents the completion marker at line 100 from being written afterward. Once the marker exists, isCodexExcerptEncryptionMigrationDone returns true on every future run, so the failed row's plaintext excerpt is never retried and never encrypted.

This is inconsistent with the initial SELECT failure path (Lines 72-75), which correctly returns aborted: true and skips the marker so the migration stays retryable. Apply the same treatment to row UPDATE failures.

Additionally, encryptDuckDbData(row.excerpt) at line 83 has no try/catch. If it throws, the loop aborts without any per-row failure handling, unlike the updRes.ok check that at least completes gracefully.

🔒 Proposed fix to keep partially failed migrations retryable
   let migrated = 0;
+  let hadRowFailure = false;
   const rows = (res.rows ?? []) as unknown as PlaintextExcerptRow[];
   for (const row of rows) {
     // QNBS-v3: SEC — re-check at each write so an opt-out mid-run stops further persistence.
     if (!shouldPersist()) return { migrated, aborted: true };

     const bytes = await encryptDuckDbData(row.excerpt);
     const updRes = await duckdbClient.exec(
       `UPDATE codex_mentions SET excerpt = NULL, excerpt_enc = ?
        WHERE entity_id = '${esc(row.entity_id)}' AND project_id = '${esc(row.project_id)}'
          AND section_id = '${esc(row.section_id)}'`,
       [bytes],
     );
-    if (updRes.ok) migrated++;
+    if (updRes.ok) {
+      migrated++;
+    } else {
+      hadRowFailure = true;
+      logger.warn('[codexExcerptEncryptionMigration] Row update failed (non-fatal):', updRes.error);
+    }
     if (migrated % 10 === 0) await Promise.resolve();
   }

   // QNBS-v3: SEC — final gate check before the done-marker: an opt-out landing during the awaited
   // batch above must not let us record the migration as complete (else re-opt-in never reruns it).
   if (!shouldPersist()) {
     return { migrated, aborted: true };
   }

+  if (hadRowFailure) {
+    // Skip the done-marker so the still-plaintext rows are retried on the next run
+    // instead of being silently left unencrypted forever.
+    return { migrated, aborted: true };
+  }
+
   await duckdbClient.exec(
     `INSERT INTO _meta (key, value) VALUES ('${esc(markerKey(projectId))}', '1')
      ON CONFLICT (key) DO UPDATE SET value = EXCLUDED.value`,
   );

Note: the test at tests/unit/services/duckdb/codexExcerptEncryptionMigration.test.ts lines 231-248 currently asserts the marker is written on row-UPDATE failure and will need to assert aborted: true with no marker call instead.

🤖 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 `@services/duckdb/codexExcerptEncryptionMigration.ts` around lines 77 - 107,
Update the row-processing loop in the codex excerpt migration to treat any
failed UPDATE as aborted: return { migrated, aborted: true } immediately and
skip writing the completion marker. Wrap encryptDuckDbData in per-row error
handling so encryption failures follow the same retryable aborted path. Update
the related test to expect aborted: true and no marker write for either failure
case.

Source: Coding guidelines

🤖 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.

Outside diff comments:
In `@services/duckdb/codexExcerptEncryptionMigration.ts`:
- Around line 77-107: Update the row-processing loop in the codex excerpt
migration to treat any failed UPDATE as aborted: return { migrated, aborted:
true } immediately and skip writing the completion marker. Wrap
encryptDuckDbData in per-row error handling so encryption failures follow the
same retryable aborted path. Update the related test to expect aborted: true and
no marker write for either failure case.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 56ebba83-f279-4d86-8e3a-b9b8d8417d04

📥 Commits

Reviewing files that changed from the base of the PR and between 51cd0cc and fdf47d9.

⛔ Files ignored due to path filters (1)
  • src-tauri/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (8)
  • TODO.md
  • docs/SECURITY-THREAT-MODEL.md
  • services/duckdb/codexExcerptEncryptionMigration.ts
  • services/duckdb/duckdbAnalytics.ts
  • tests/unit/duckdbAnalytics.test.ts
  • tests/unit/hooks/useDuckDb.test.ts
  • tests/unit/listenerMiddleware.test.ts
  • tests/unit/services/duckdb/codexExcerptEncryptionMigration.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • tests/unit/duckdbAnalytics.test.ts
  • services/duckdb/duckdbAnalytics.ts
  • TODO.md
  • docs/SECURITY-THREAT-MODEL.md

qnbs added 2 commits July 31, 2026 23:39
… fails (CodeRabbit)

A failed per-row UPDATE (or encryptDuckDbData() throwing) during the
codex excerpt-encryption backfill could still let the migration write
its completion marker. Once written, isCodexExcerptEncryptionMigrationDone()
would return true forever, so the failed row's plaintext excerpt was
never retried or encrypted.

- Track row-level failures via a hadRowFailure flag.
- Wrap encryptDuckDbData() in try/catch so a throw is treated the same
  as a failed UPDATE (log + skip row) instead of crashing the run.
- Skip the done-marker INSERT and return { aborted: true } when any row
  failed, so the run stays retryable on the next pass.
- Update/add tests: failed-UPDATE case now expects aborted:true with no
  marker call; new test covers an encryption throw taking the same path.
…d commits

Retroactively documents everything landed on this branch since the
premature 51cd0cc release cut: the SEC-6 marker-scoping fix + ON
CONFLICT ciphertext-downgrade fix, the event-listener RUSTSEC-2026-0221
bump, the TODO.md/SECURITY-THREAT-MODEL.md doc fixes, the Codecov
coverage-gap test additions, and the CodeRabbit row-failure done-marker
fix, so the release text is exhaustive rather than only the last few
commits.
@qnbs

qnbs commented Jul 31, 2026

Copy link
Copy Markdown
Owner Author

Fixed the outside-diff-range finding from review 4832245658 (codexExcerptEncryptionMigration.ts, lines 77-107): a failed row UPDATE (or an encryptDuckDbData() throw) no longer lets the migration write its done-marker. Both failure modes now set a hadRowFailure flag and the run returns { aborted: true }, skipping the marker so the still-plaintext rows are retried next run. Also wrapped encryptDuckDbData() in try/catch per the review's note. Updated the existing failed-UPDATE test's expectations and added a new test for the encryption-throw path. Fixed in 2972137.

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

@qnbs I will review the update in 29721372, including the failure paths and the added test coverage.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@qnbs
qnbs merged commit 256264d into main Jul 31, 2026
27 checks passed
@qnbs
qnbs deleted the feat/duckdb-excerpt-encryption-v1.25.0 branch July 31, 2026 22:06
qnbs added a commit that referenced this pull request Jul 31, 2026
…port refresh (#304)

* chore: post-merge housekeeping — TODO.md release status + graphify report refresh

QNBS-v3: TODO.md documents PR #303's admin-bypass squash-merge to main; graphify-out/GRAPH_REPORT.md refreshed after the merge (7558 nodes, 14036 edges, 764 communities).

* fix: exclude vendored DuckDB WASM worker from graphify scan

Root-causes 3 of 4 PR #304 review findings: the AST scanner was
treating public/duckdb/duckdb-browser-eh.worker.js (a minified
third-party bundle) as application source, polluting God Nodes
(duplicate readInt32() entries), Communities (0.00-cohesion blobs
of single-letter minified names), and Markdown output (unescaped
_emscripten_enter_soft_fullscreen()/_glutInit() identifiers).

- .graphifyignore: exclude public/duckdb/
- graphify-out/GRAPH_REPORT.md: regenerated with graphifyy==0.8.26
  after excluding the vendor bundle; God Nodes/Communities now
  reflect only real application code
- TODO.md: apply CodeRabbit wording fix ('so deploy can run' instead
  of 'so deploy succeeds', since green build+e2e only gate deploy,
  they don't guarantee actions/deploy-pages itself succeeds)

* fix: mark v1.25.0 release as done in TODO.md (doc-metrics gate)

CI's doc-metrics drift gate correctly flagged the '⬜ Tag and publish
v1.25.0' bullet as stale since v1.25.0 was already tagged and
released. Flip to ✅ done with a GitHub Release link, and update the
section header/status accordingly.
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.

2 participants