refactor: run database encryption as an operational worker

[think-in-universe]

Summary

• remove the permanent admin database-encryption HTTP routes and startup recovery task
• add a one-off database_encryption_worker binary with scan, migrate, verify, scoped batches, resume support, and a machine-readable completion marker
• add a fail-closed inventory that explicitly classifies every application text/JSON column as encrypt, remove, or approved plaintext with a reason
• allow operational jobs without a synthetic admin actor while recording the workflow operator

This is intentionally stacked on #968 and retains its database-encryption naming and job engine. The companion deployment PR creates and tears down an ephemeral CVM to run the worker.

Validation

• cargo check -p api --bin database_encryption_worker
• cargo test -p api database_encryption_inventory::tests::sensitive_examples_have_explicit_policies
• isolated PostgreSQL E2E: operational migrate/verify and full live-schema classification (2 passed)
• cargo fmt --all -- --check
• git diff --check

Depends on #968.

[think-in-universe]

Companion operational deployment PR: https://github.com/nearai/cvm-ansible-playbooks/pull/536

[ironloopai]

Review · Status

🟩 Completed

IronLoop completed the review and posted it to GitHub.

Result

Open submitted review →

Run details

• Run: e709f645-88c9-40f5-9d6b-ffb611d9c9bf
• Base: feat/database-encryption-admin-api at 0db8d28
• Head: feat/database-encryption-worker at 2c1bec6
• Created: 2026-08-25 19:44 UTC
• Updated: 2026-08-25 20:14 UTC

_{Automatic trigger · attempt 1 of 3 · completed in 29m 45s}

[claude]

Review: operational database-encryption worker

Reviewed against 0db8d28 (the feat/database-encryption-admin-api base). No prior review threads on this PR to build on — only the author's link to cvm-ansible-playbooks#536.

The shape is right: moving a destructive backfill out of the always-on admin plane into a one-off, scoped, resumable worker with a fail-closed schema inventory is a clear improvement over #968. The inventory in database_encryption_inventory.rs plus the worker_inventory_classifies_the_live_schema e2e test is a genuinely good gate — a new migration that adds an unclassified text column will fail CI. Envelope AAD binding (table:column:id) and the tightened JSON predicate (now checking version/alg/key_id/nonce/ciphertext instead of bare marker presence) are both correct hardening.

Findings below, most severe first.

---

⚠️ 1. --max-rows marks the job completed, fails the run, and leaves it unresumable

crates/api/src/database_encryption.rs:736 / :817

while field_index < fields.len() && max.is_none_or(|limit| processed < limit) { ... }
// loop exits on the max_rows cap, then unconditionally:
client.execute("UPDATE database_encryption_jobs SET status='completed',completed_at=NOW() WHERE id=$1", ...)

Three compounding consequences when --max-rows actually truncates:

1. The job is recorded completed despite unprocessed fields remaining.
2. operational_migrate then runs operational_verify over the full scope, finds the remaining plaintext, and bails with post-migration verification failed → exit 1 and a failed-status marker. So the deployment automation reads a successful canary as a failed migration.
3. --resume is then impossible: database_encryption.rs:463 only accepts status IN ('queued','running','failed'), and the row is completed.

So a partial/canary run — the only reason --max-rows exists — is unrecoverable and always reports failure. Fix: distinguish cap-exhaustion from field-exhaustion, e.g.

let truncated = max.is_some_and(|limit| processed >= limit) && field_index < fields.len();
let status = if truncated { "queued" } else { "completed" };

and skip the post-migrate full-scope verify (or scope it to what was processed) when truncated.

⚠️ 2. The worker discards every error detail

crates/api/src/bin/database_encryption_worker.rs:41-49

if run().await.is_err() {
    println!("DATABASE_ENCRYPTION_WORKER_RESULT {}",
        serde_json::json!({"status":"failed","error_class":"worker_failed"}));
    std::process::exit(1);
}

The anyhow::Error — with its whole context chain — is dropped on the floor. Nothing goes to stderr. And the binary never initialises a tracing subscriber, so the tracing::error! calls inside database_encryption.rs (internal, spawn_job) emit nothing either. The worker is therefore completely silent about why it failed, in an ephemeral CVM that the companion PR tears down immediately after.

The failure messages here carry no customer data — they are table/column names, job status, and scope ("unclassified text/JSON database columns: [...]", "resume scope does not match the persisted job scope", "job ended with status failed") — so printing them is safe under CLAUDE.md's logging rules.

if let Err(error) = run().await {
    eprintln!("{error:#}");   // context chain, no customer payloads
    println!("DATABASE_ENCRYPTION_WORKER_RESULT {}",
        serde_json::json!({"status":"failed","error_class":"worker_failed"}));
    std::process::exit(1);
}

Please also init tracing_subscriber in run().

⚠️ 3. --batch-size and --max-rows are silently ignored on --resume

crates/api/src/database_encryption.rs:458-478

The resume branch only resets status/completed_at/last_error_*. run_locked_job reads batch_size and max_rows back out of the job row (:724-725), so the CLI values are validated at :447-451 and then discarded. An operator who resumes with --batch-size 50 after a timeout still gets the original 500 and hits the same timeout. Either persist the new values on resume, or reject the flags when --resume is set (clap conflicts_with).

⚠️ 4. The fail-closed inventory gate runs after the migration writes

operational_migrate (:489-504) calls run_job first and only reaches verify_classification_inventory via the trailing operational_verify. An unclassified schema aborts the run after every row has already been rewritten — the opposite of fail-closed, and the operator sees failed on a run whose writes actually landed. Call verify_classification_inventory(state).await? before run_job.

5. Resume cursor stores a positional index into a code-defined array

cursor.field_index (:728, :810) indexes into selected(&scope), whose ordering is the declaration order of the FIELDS const. Resume support is new in this PR, which makes the coupling reachable: if FIELDS is reordered or an entry inserted between the original run and the resume, the cursor points at a different column and rows are silently skipped. Persisting the table/column pair instead of an index removes the hazard entirely.

6. ~300 lines of unreachable admin surface retained

The PR removes the routes from lib.rs but keeps every handler. grep confirms nothing references them: scan, create_job, get_job, get_inner, cancel_job, verify, recover_jobs, spawn_job, plus Mode, CreateJobRequest, ScanRequest, VerifyRequest, JobResponse, and the AdminUser import. They are pub in a lib crate so dead_code stays quiet. Two of them still carry no-op fields (let _ = req.include_approved_plaintext; at :605, let _ = req.fail_on_approved_plaintext_without_reason; at :903) that were never implemented. Given the PR's stated goal is removing the permanent admin surface, deleting these keeps a dormant code path from being re-wired later.

7. Verify page size is hardcoded at 1000 rows under a 5s statement timeout

counts() (:547-558) pages SELECT id, <col>::text ... LIMIT 1000 with SET LOCAL statement_timeout = '5s', regardless of --batch-size. For response_items.item — full transcripts and tool payloads — 1000 rows can be tens of MB. A timeout here aborts the entire worker, and per finding #2 the operator gets no indication it was a timeout rather than a real plaintext leak. Consider deriving the page size from --batch-size and raising the verify timeout, as the migrate path already does (30s).

8. Minor

• V0075 edited in place. Widening admin_actor to nullable and adding operator are both safe changes, but refinery aborts on a checksum divergence for an already-applied migration. Fine while feat: add admin database encryption backfill API #968 is unmerged and undeployed; if V0075 has run in any environment, this needs a V0076 instead.
• S3_ENCRYPTION_KEY is not trimmed (database_encryption_worker.rs:99-101) while the _FILE variant is. A trailing newline from the env injection path fails hex::decode with — per Develop #2 — no visible reason. Apply .trim() to both.
• No test coverage for --resume, --max-rows, or inventory-gate failure. Findings #1 and Add TLS support #3 would both have been caught by a resume test.

---

Findings #1 and #2 together are the blocker: a truncated run reports failure and cannot be resumed, and the operator has no output explaining any of it. #3 and #4 are cheap fixes in the same functions.

⚠️

[github-actions]

🔍 OpenCodeReview found 3 issue(s) in this PR.

• ✅ 3 posted as inline comment(s)
• 📝 0 posted as summary

[github-actions]

When run().await returns an error, the actual error message, error chain, and context are completely discarded — only a generic {"status":"failed","error_class":"worker_failed"} is printed. For a one-off worker that touches encrypted production data, the operator needs to see the real error to diagnose failures. The actual anyhow::Error (including its context chain from .context() calls throughout operational_migrate, run_job, etc.) should be serialized or at minimum printed to stderr before exiting.

Suggestion:

Suggested change

	if run().await.is_err() {
	println!(
	"DATABASE_ENCRYPTION_WORKER_RESULT {}",
	serde_json::json!({"status":"failed","error_class":"worker_failed"})
	);
	std::process::exit(1);
	}
	if let Err(err) = run().await {
	eprintln!("{err:#}");
	println!(
	"DATABASE_ENCRYPTION_WORKER_RESULT {}",
	serde_json::json!({"status":"failed","error_class":"worker_failed","error": format!("{err:#}")})
	);
	std::process::exit(1);
	}

[github-actions]

When operational_migrate returns an error, the job ID is lost because operational_migrate returns Result<Uuid> and the ? operator discards the ID on failure. Tracing into database_encryption.rs, operational_migrate creates a job record (setting status to 'queued') and then calls run_job directly. run_job sets the job status to 'running' at the start of run_locked_job, but if it errors mid-batch, only the spawned (spawn_job) path marks the job as 'failed' — the worker path via operational_migrate does not. This leaves the job stuck in 'running' status indefinitely. The operator has no job ID in the worker output to use with --resume, requiring a manual database query to find and recover the stuck job. Consider having operational_migrate either mark the job as failed on error or include the job ID in the error context.

Suggestion:

Suggested change

	Command::Migrate {
	scope,
	batch_size,
	max_rows,
	resume,
	operator,
	} => {
	let id =
	operational_migrate(&state, scope, batch_size, max_rows, resume, &operator).await?;
	print_success(Some(id));
	}
	Command::Migrate {
	scope,
	batch_size,
	max_rows,
	resume,
	operator,
	} => {
	let id =
	operational_migrate(&state, scope, batch_size, max_rows, resume, &operator)
	.await
	.map_err(|err| {
	eprintln!("migration failed; check database_encryption_jobs for stuck 'running' jobs: {err:#}");
	err
	})?;
	print_success(Some(id));
	}

[github-actions]

The key-source precedence here is inverted compared to the canonical implementation in config::S3Config::from_env() (crates/config/src/types.rs lines 969-985): the config layer checks S3_ENCRYPTION_KEY_FILE first and falls back to S3_ENCRYPTION_KEY, whereas this worker checks the env var first and only tries the file path on failure. If both variables are set in the same deployment environment (e.g., an env var from legacy config plus a mounted secret file), the main API process and this migration worker will resolve different keys. The worker would then encrypt production data with a key the API cannot decrypt, silently corrupting records. This worker should reuse the existing config::S3Config::from_env() key-loading logic (or at minimum match its FILE-before-ENV precedence and empty-key validation) to guarantee both processes agree on the same encryption key.

Suggestion:

Suggested change

	fn read_encryption_key() -> Result<String> {
	if let Ok(key) = std::env::var("S3_ENCRYPTION_KEY") {
	return Ok(key);
	}
	let path = std::env::var("S3_ENCRYPTION_KEY_FILE")
	.context("S3_ENCRYPTION_KEY or S3_ENCRYPTION_KEY_FILE is required")?;
	std::fs::read_to_string(path)
	.context("failed to read S3_ENCRYPTION_KEY_FILE")
	.map(|key| key.trim().to_string())
	}
	fn read_encryption_key() -> Result<String> {
	// Match the precedence used by config::S3Config::from_env():
	// file-based secret first, env var as fallback.
	if let Ok(path) = std::env::var("S3_ENCRYPTION_KEY_FILE") {
	let key = std::fs::read_to_string(path)
	.context("failed to read S3_ENCRYPTION_KEY_FILE")?
	.trim()
	.to_string();
	if key.is_empty() {
	anyhow::bail!("S3 encryption key cannot be empty");
	}
	return Ok(key);
	}
	let key = std::env::var("S3_ENCRYPTION_KEY")
	.context("S3_ENCRYPTION_KEY or S3_ENCRYPTION_KEY_FILE is required")?;
	if key.is_empty() {
	anyhow::bail!("S3 encryption key cannot be empty");
	}
	Ok(key)
	}

[ironloopai]

Review · Summary

Found three actionable rollout and operational issues.

Findings: 🔴 High 1 · 🟠 Medium 2

_{Code-specific findings are attached to the diff.}

Validation

• ✅ Worker build — The database-encryption worker compiled successfully.
• ✅ Inventory policy unit test — The sensitive policy examples passed.

Review details

• Run: e709f645-88c9-40f5-9d6b-ffb611d9c9bf
• Attempts: 1

[ironloopai]

🔴 High · Add a new migration for the worker schema

V0075 is already applied by the prerequisite change. Changing its contents changes its recorded checksum, so existing databases will abort migration startup; they also will not receive the nullable admin_actor or operator schema required by the worker. Keep V0075 immutable and add a new ALTER TABLE migration.

[ironloopai]

🟠 Medium · Match the API's encryption-key source precedence

The API configuration prefers S3_ENCRYPTION_KEY_FILE, while this worker returns S3_ENCRYPTION_KEY first. When both are supplied with different values, the worker verifies with the wrong key and can write envelopes the API cannot decrypt. Prefer the file source consistently or reject conflicting configuration.

[ironloopai]

🟠 Medium · Keep capped migrations resumable

If max_rows is smaller than the remaining plaintext rows, the job reaches its cap and is marked completed, but this full-scope verification then necessarily fails. Completed jobs are rejected by --resume, so bounded runs emit a failed worker result and cannot continue through the advertised resume flow. Defer full verification for partial work or preserve a resumable partial status.