fix: upgrade credential encryption key derivation to PBKDF2

[kwp3]

OPEN_NOTEBOOK_ENCRYPTION_KEY was derived to a Fernet key via a single unsalted SHA-256 round - fast to compute, which is exactly the wrong property for a key derivation function: it makes offline brute-forcing a weak/short passphrase cheap.

Derive via PBKDF2-HMAC-SHA256 with a fixed domain-separation salt and 600k iterations (OWASP 2023 guidance) instead. Ciphertext written under the old scheme still decrypts via an automatic fallback to the legacy derivation, and gets transparently re-encrypted under the new scheme the next time the owning record is saved - no migration step required. Both derived keys are cached lazily per process since PBKDF2 at this iteration count is deliberately expensive (~50ms).

Stacked on #1002-#1009 (unmerged). tests/test_encryption.py passes (11/11).

[cubic-dev-ai]

16 issues found across 26 files

Confidence score: 2/5

• The highest risk is SSRF hardening still being bypassable across api/routers/sources.py, open_notebook/utils/url_validation.py, open_notebook/ai/connection_tester.py, and api/credentials_service.py: localhost/private targets can still pass in source ingestion, validation can fail open on DNS errors, and request-time DNS rebinding remains possible when httpx resolves again. Merging as-is leaves a concrete path to internal network/metadata access—add a source-specific strict validator, fail closed when hostname safety cannot be proven, and bind requests to the already-validated resolution before merge.
• open_notebook/utils/url_validation.py has an incomplete metadata denylist (missing Alibaba ECS 100.100.100.200), so cloud-instance metadata can still be reachable even with current protections. That keeps credential/token exposure risk on the table—expand metadata endpoint coverage (including non-link-local provider IPs) before merging.
• In api/routers/context.py, the batch insight fetch fallback clears insights_by_source to {}, which silently drops all source insights instead of degrading to per-source queries. Users can receive incomplete context responses without obvious errors—restore a per-source fallback path in the exception flow before merge.
• open_notebook/ai/models.py and open_notebook/utils/url_validation.py run synchronous DNS validation (socket.getaddrinfo) in async/API paths, which can block FastAPI/event-loop work under slow DNS and cause latency spikes. This is less severe than the security items but still operationally risky—move validation to async/non-blocking execution (or a threadpool) before merging.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="frontend/src/components/ui/markdown-editor.tsx">

<violation number="1" location="frontend/src/components/ui/markdown-editor.tsx:27">
P3: This UI component update still leaves the root element without the required `data-slot`, so downstream component tooling/selectors that rely on `components/ui` conventions cannot identify `MarkdownEditor`. Since this file is being modified, adding `data-slot="markdown-editor"` to the root `<div>` would align it with `frontend/src/components/ui/CLAUDE.md`.</violation>
</file>

<file name="open_notebook/ai/models.py">

<violation number="1" location="open_notebook/ai/models.py:159">
P2: Model provisioning can now stall other async work while DNS resolves because `_revalidate_config_urls()` calls synchronous `validate_url()` from inside `async def get_model`. Consider offloading this validation with `asyncio.to_thread()` or adding an async URL validator so slow/unresponsive DNS does not block API/workflow requests.</violation>
</file>

<file name="open_notebook/ai/connection_tester.py">

<violation number="1" location="open_notebook/ai/connection_tester.py:69">
P1: Request-time DNS rebinding protection is still bypassable because validation and the `httpx` request perform separate DNS resolutions of the same hostname. Consider making the request through the already-validated resolved address (preserving the Host/SNI as needed) or using a transport/resolver that binds validation and connection to the same resolution result.</violation>
</file>

<file name="open_notebook/utils/url_validation.py">

<violation number="1" location="open_notebook/utils/url_validation.py:65">
P2: The `except ValueError as ve` handler in `validate_url` uses string matching on exception messages (`"Link-local" in str(ve)`) to decide whether a ValueError came from `_reject_dangerous_ip` (re-raise) or from `ipaddress.ip_address` (fall through to DNS resolution). This is fragile: a hostname containing `Link-local` would be mis-classified and re-raised with a confusing ipaddress error. Recommend restructuring the try/except to separate the two sources: wrap the `ipaddress.ip_address(hostname)` call in its own `try/except ValueError` that cleanly falls through to DNS resolution, and let `_reject_dangerous_ip` raise unambiguously.</violation>

<violation number="2" location="open_notebook/utils/url_validation.py:72">
P2: Credential/source requests can block other FastAPI work while DNS resolution hangs because this synchronous validator calls `socket.getaddrinfo` directly. Since the utility is intended for API/services, consider an async boundary such as `asyncio.to_thread` plus a timeout or an async resolver.</violation>

<violation number="3" location="open_notebook/utils/url_validation.py:87">
P1: Provider URL metadata protection fails open when DNS resolution fails: this path allows the URL even though the validator could not prove the hostname avoids link-local/metadata IPs. For the pre-use security check, treating `socket.gaierror` as invalid (or using an explicit save-time-only mode) would keep the SSRF guard fail-closed.</violation>

<violation number="4" location="open_notebook/utils/url_validation.py:92">
P3: Unexpected URL validation failures will be hard to diagnose because this handler tells callers to check server logs but never writes the exception. Logging the traceback server-side with Loguru while keeping the sanitized `ValueError` response would match the existing error-handling convention.

(Based on your team's feedback about logging tracebacks server-side without exposing them to clients.) [FEEDBACK_USED]</violation>

<violation number="5" location="open_notebook/utils/url_validation.py:122">
P1: Cloud metadata SSRF protection is incomplete for Alibaba ECS: `100.100.100.200` is a documented metadata endpoint but this denylist only adds AWS IMDSv6 beyond link-local addresses. Adding the non-link-local metadata endpoints the app supports would prevent provider URLs from reaching them.</violation>
</file>

<file name="api/credentials_service.py">

<violation number="1" location="api/credentials_service.py:461">
P2: OpenAI custom model discovery still has the DNS-rebinding gap this change closes for other user-configured URLs. The `openai` path later uses credential `base_url` for `discovery_url` without a request-time `validate_url()`, so revalidating that path before `httpx.get` would keep Discover Models consistent with `ollama`, `openai_compatible`, and `azure`.</violation>
</file>

<file name="api/routers/sources.py">

<violation number="1" location="api/routers/sources.py:370">
P2: Creating a link source can stall other API requests while DNS resolution runs because `validate_url()` calls blocking `socket.getaddrinfo()` inside the async route. Offloading this validation to the thread pool would match the surrounding non-blocking endpoint design.</violation>

<violation number="2" location="api/routers/sources.py:370">
P1: Link source ingestion still permits SSRF to loopback/private network targets because `validate_url()` is the credential-endpoint validator and intentionally allows localhost and RFC1918 addresses. A source-specific URL guard should reject loopback/private/internal ranges before the URL is queued or fetched.</violation>
</file>

<file name="frontend/src/components/ui/markdown-editor.test.tsx">

<violation number="1" location="frontend/src/components/ui/markdown-editor.test.tsx:3">
P2: `@uiw/react-markdown-preview` is imported directly in the test but isn't listed as a direct dependency. It resolves transitively through `@uiw/react-md-editor`, which is fragile — a version bump or restructured dependency tree could break the import. Add it as a direct `devDependency` since the test file explicitly imports it.</violation>
</file>

<file name="open_notebook/graphs/prompt.py">

<violation number="1" location="open_notebook/graphs/prompt.py:27">
P2: This SSTI fix is not covered by a prompt graph regression test, so a future refactor back to `template_text=state["prompt"]` would still pass the existing graph tests. A focused test with Jinja syntax in `state["prompt"]` that asserts it is sent literally would lock in the security behavior.</violation>
</file>

<file name="open_notebook/utils/encryption.py">

<violation number="1" location="open_notebook/utils/encryption.py:104">
P2: Weak-passphrase attacks can still be amortized across Open Notebook installations because this PBKDF2 salt is constant for every deployment. Consider storing/versioning a per-install or per-record salt, or make the docs/config require a high-entropy encryption key rather than relying on this as a full password salt.</violation>

<violation number="2" location="open_notebook/utils/encryption.py:232">
P3: Unexpected decryption failures from the new PBKDF2 path become harder to diagnose and can expose raw exception text because this handler logs only `{e}` and re-raises `str(e)`. Prefer logging the traceback server-side and raising a sanitized failure message here.

(Based on your team's feedback about server-side tracebacks and sanitized client-facing errors.) [FEEDBACK_USED]</violation>
</file>

<file name="api/routers/context.py">

<violation number="1" location="api/routers/context.py:88">
P1: When the batch insight fetch fails, all source insights are silently dropped from the context response instead of falling back to per-source queries. The `except` block sets `insights_by_source = {}`, which causes `insights=[]` to be passed to `source.get_context()`. Since `Source.get_context()` checks `insights is not None` (and `[] is not None` is True), it uses that empty list directly and never calls `self.get_insights()` as a fallback. The result is a context response with zero insights, which is worse than the error the batching was meant to avoid. Consider setting `insights_by_source = None` and conditionally passing insights only when the batch succeeded.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: Request-time DNS rebinding protection is still bypassable because validation and the httpx request perform separate DNS resolutions of the same hostname. Consider making the request through the already-validated resolved address (preserving the Host/SNI as needed) or using a transport/resolver that binds validation and connection to the same resolution result.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At open_notebook/ai/connection_tester.py, line 69:

<comment>Request-time DNS rebinding protection is still bypassable because validation and the `httpx` request perform separate DNS resolutions of the same hostname. Consider making the request through the already-validated resolved address (preserving the Host/SNI as needed) or using a transport/resolver that binds validation and connection to the same resolution result.</comment>

<file context>
@@ -61,6 +63,10 @@ async def _test_azure_connection(
+        # Re-validate at request time: the endpoint may have been saved
+        # against a hostname that only later resolved to an internal
+        # address (DNS rebinding), so a save-time check alone isn't enough.
+        validate_url(test_endpoint, "azure")
         async with httpx.AsyncClient(timeout=10.0) as client:
             response = await client.get(
</file context>

[cubic-dev-ai]

P1: Link source ingestion still permits SSRF to loopback/private network targets because validate_url() is the credential-endpoint validator and intentionally allows localhost and RFC1918 addresses. A source-specific URL guard should reject loopback/private/internal ranges before the URL is queued or fetched.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At api/routers/sources.py, line 370:

<comment>Link source ingestion still permits SSRF to loopback/private network targets because `validate_url()` is the credential-endpoint validator and intentionally allows localhost and RFC1918 addresses. A source-specific URL guard should reject loopback/private/internal ranges before the URL is queued or fetched.</comment>

<file context>
@@ -360,6 +364,12 @@ async def create_source(
+            # Block SSRF to internal/metadata addresses before the server ever
+            # fetches this URL (same guard used for provider-credential URLs).
+            try:
+                validate_url(source_data.url, "source")
+            except ValueError as e:
+                raise HTTPException(status_code=400, detail=str(e))
</file context>

[cubic-dev-ai]

P1: Cloud metadata SSRF protection is incomplete for Alibaba ECS: 100.100.100.200 is a documented metadata endpoint but this denylist only adds AWS IMDSv6 beyond link-local addresses. Adding the non-link-local metadata endpoints the app supports would prevent provider URLs from reaching them.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At open_notebook/utils/url_validation.py, line 122:

<comment>Cloud metadata SSRF protection is incomplete for Alibaba ECS: `100.100.100.200` is a documented metadata endpoint but this denylist only adds AWS IMDSv6 beyond link-local addresses. Adding the non-link-local metadata endpoints the app supports would prevent provider URLs from reaching them.</comment>

<file context>
@@ -0,0 +1,131 @@
+
+    # Block AWS's IMDSv6 metadata address - a Unique Local Address, not
+    # link-local, so it needs its own explicit check.
+    if ip == _AWS_IMDS_V6_ADDRESS:
+        if resolved:
+            raise ValueError(
</file context>

[cubic-dev-ai]

P1: Provider URL metadata protection fails open when DNS resolution fails: this path allows the URL even though the validator could not prove the hostname avoids link-local/metadata IPs. For the pre-use security check, treating socket.gaierror as invalid (or using an explicit save-time-only mode) would keep the SSRF guard fail-closed.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At open_notebook/utils/url_validation.py, line 87:

<comment>Provider URL metadata protection fails open when DNS resolution fails: this path allows the URL even though the validator could not prove the hostname avoids link-local/metadata IPs. For the pre-use security check, treating `socket.gaierror` as invalid (or using an explicit save-time-only mode) would keep the SSRF guard fail-closed.</comment>

<file context>
@@ -0,0 +1,131 @@
+                # Could not resolve hostname - allow it since the URL may be
+                # valid in the deployment environment (e.g., Azure endpoints,
+                # internal DNS names). We only block link-local addresses.
+                pass
+
+    except ValueError:
</file context>

[cubic-dev-ai]

P1: When the batch insight fetch fails, all source insights are silently dropped from the context response instead of falling back to per-source queries. The except block sets insights_by_source = {}, which causes insights=[] to be passed to source.get_context(). Since Source.get_context() checks insights is not None (and [] is not None is True), it uses that empty list directly and never calls self.get_insights() as a fallback. The result is a context response with zero insights, which is worse than the error the batching was meant to avoid. Consider setting insights_by_source = None and conditionally passing insights only when the batch succeeded.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At api/routers/context.py, line 88:

<comment>When the batch insight fetch fails, all source insights are silently dropped from the context response instead of falling back to per-source queries. The `except` block sets `insights_by_source = {}`, which causes `insights=[]` to be passed to `source.get_context()`. Since `Source.get_context()` checks `insights is not None` (and `[] is not None` is True), it uses that empty list directly and never calls `self.get_insights()` as a fallback. The result is a context response with zero insights, which is worse than the error the batching was meant to avoid. Consider setting `insights_by_source = None` and conditionally passing insights only when the batch succeeded.</comment>

<file context>
@@ -77,9 +77,21 @@ async def get_notebook_context(notebook_id: str, context_request: ContextRequest
+                # Match the per-source fallback below: a hiccup fetching
+                # insights shouldn't fail the whole context request.
+                logger.warning(f"Error batch-fetching source insights: {str(e)}")
+                insights_by_source = {}
             for source in sources:
                 try:
</file context>

[cubic-dev-ai]

P2: Credential/source requests can block other FastAPI work while DNS resolution hangs because this synchronous validator calls socket.getaddrinfo directly. Since the utility is intended for API/services, consider an async boundary such as asyncio.to_thread plus a timeout or an async resolver.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At open_notebook/utils/url_validation.py, line 72:

<comment>Credential/source requests can block other FastAPI work while DNS resolution hangs because this synchronous validator calls `socket.getaddrinfo` directly. Since the utility is intended for API/services, consider an async boundary such as `asyncio.to_thread` plus a timeout or an async resolver.</comment>

<file context>
@@ -0,0 +1,131 @@
+            # Not an IP address, it's a hostname - need to resolve and check
+            try:
+                # Resolve hostname to IP address
+                resolved_ips = socket.getaddrinfo(hostname, None)
+                for family, _, _, _, sockaddr in resolved_ips:
+                    ip_addr = sockaddr[0]
</file context>

[cubic-dev-ai]

P2: Weak-passphrase attacks can still be amortized across Open Notebook installations because this PBKDF2 salt is constant for every deployment. Consider storing/versioning a per-install or per-record salt, or make the docs/config require a high-entropy encryption key rather than relying on this as a full password salt.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At open_notebook/utils/encryption.py, line 104:

<comment>Weak-passphrase attacks can still be amortized across Open Notebook installations because this PBKDF2 salt is constant for every deployment. Consider storing/versioning a per-install or per-record salt, or make the docs/config require a high-entropy encryption key rather than relying on this as a full password salt.</comment>

<file context>
@@ -92,6 +96,20 @@ def _get_or_create_encryption_key() -> str:
+# store a random one without a migration) - its job is just to stop
+# precomputed hash tables from unrelated contexts from applying here.
+# Actual brute-force resistance comes from the iteration count.
+_KDF_SALT = b"open-notebook:credential-encryption:v2"
+_KDF_ITERATIONS = 600_000  # OWASP 2023 guidance for PBKDF2-HMAC-SHA256
+
</file context>

[cubic-dev-ai]

P3: This UI component update still leaves the root element without the required data-slot, so downstream component tooling/selectors that rely on components/ui conventions cannot identify MarkdownEditor. Since this file is being modified, adding data-slot="markdown-editor" to the root <div> would align it with frontend/src/components/ui/CLAUDE.md.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/ui/markdown-editor.tsx, line 27:

<comment>This UI component update still leaves the root element without the required `data-slot`, so downstream component tooling/selectors that rely on `components/ui` conventions cannot identify `MarkdownEditor`. Since this file is being modified, adding `data-slot="markdown-editor"` to the root `<div>` would align it with `frontend/src/components/ui/CLAUDE.md`.</comment>

<file context>
@@ -14,9 +15,18 @@ const MDEditor = dynamic(
+// (katex-html spans, MathML) isn't in the default sanitize schema and gets
+// stripped if sanitize runs after it - order here is load-bearing, verified
+// against the actual rendered output for math/code/GFM before changing it.
+export const PREVIEW_OPTIONS = {
   remarkPlugins: [remarkMath],
-  rehypePlugins: [rehypeKatex],
</file context>

[cubic-dev-ai]

P3: Unexpected URL validation failures will be hard to diagnose because this handler tells callers to check server logs but never writes the exception. Logging the traceback server-side with Loguru while keeping the sanitized ValueError response would match the existing error-handling convention.

(Based on your team's feedback about logging tracebacks server-side without exposing them to clients.)

View Feedback

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At open_notebook/utils/url_validation.py, line 92:

<comment>Unexpected URL validation failures will be hard to diagnose because this handler tells callers to check server logs but never writes the exception. Logging the traceback server-side with Loguru while keeping the sanitized `ValueError` response would match the existing error-handling convention.

(Based on your team's feedback about logging tracebacks server-side without exposing them to clients.) </comment>

<file context>
@@ -0,0 +1,131 @@
+    except ValueError:
+        raise
+    except Exception:
+        raise ValueError("Invalid URL format. Check server logs for details.")
+
+
</file context>

[cubic-dev-ai]

P3: Unexpected decryption failures from the new PBKDF2 path become harder to diagnose and can expose raw exception text because this handler logs only {e} and re-raises str(e). Prefer logging the traceback server-side and raising a sanitized failure message here.

(Based on your team's feedback about server-side tracebacks and sanitized client-facing errors.)

View Feedback

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At open_notebook/utils/encryption.py, line 232:

<comment>Unexpected decryption failures from the new PBKDF2 path become harder to diagnose and can expose raw exception text because this handler logs only `{e}` and re-raises `str(e)`. Prefer logging the traceback server-side and raising a sanitized failure message here.

(Based on your team's feedback about server-side tracebacks and sanitized client-facing errors.) </comment>

<file context>
@@ -180,18 +225,25 @@ def decrypt_value(value: str) -> str:
+        return get_fernet().decrypt(value.encode()).decode()
+    except InvalidToken:
+        pass
+    except Exception as e:
+        logger.error(f"Decryption failed: {e}")
+        raise ValueError(f"Decryption failed: {str(e)}")
</file context>

Suggested change

	except Exception as e:
	logger.error(f"Decryption failed: {e}")
	raise ValueError(f"Decryption failed: {str(e)}")
	except Exception:
	logger.exception("Decryption failed")
	raise ValueError("Decryption failed")

[lfnovo]

Excellent work — we verified backward compatibility functionally (extracted the PR's encryption.py and decrypted ciphertext produced by main's current scheme via the legacy fallback; wrong-key detection preserved in both schemes; ~42ms one-time derivation cost). No concerns about the upgrade path.

Holding the merge briefly to settle the rollback story, since this is a one-way door: once a credential is (re-)saved under PBKDF2, rolling back to an older image makes that credential fail with a generic "key is incorrect". Recoverable (re-enter the key in the UI), but worth handling deliberately. Two open questions:

1. Ciphertext version marker — would you be up for prefixing new-format ciphertext with a marker (e.g. pbkdf2v1:)? Today the formats are distinguished by try-and-fallback, so a rollback failure surfaces as a misleading "key is incorrect" instead of a diagnosable "unsupported format". A marker also makes any future derivation change much cleaner.
2. Release timing — we'd ship this in a regular (non-patch) release with a prominent CHANGELOG warning about the downgrade caveat.

Neither blocks the rest of your stack — merging the others in the meantime.

[lfnovo]

The core here is genuinely good work — deriving the Fernet key via PBKDF2-HMAC-SHA256 (fixed domain-separation salt, 600k iterations per OWASP 2023) is the right hardening, especially since the app deliberately accepts a plain passphrase like my-secret. The legacy-fallback-then-transparent-reencrypt design is well reasoned, and the lazy per-process key cache is the correct call given the ~50ms derivation cost. I'd like to land this — but as a focused, standalone change, and I'd love your read on the migration story before we do.

1. Please split this into an encryption-only PR off current main (blocker)

This branch is CONFLICTING with main and is stacked on #1002–#1009 (unmerged), so the diff carries ~22 files that have nothing to do with the KDF change (chat/context/insights routers, transformation.py, prompt SSTI fix, the markdown-editor frontend, etc.). The actual change is ~4 files: open_notebook/utils/encryption.py, tests/test_encryption.py, docs/5-CONFIGURATION/security.md, open_notebook/utils/CLAUDE.md.

Two concrete reasons this matters beyond tidiness:

• The bundled open_notebook/utils/url_validation.py / SSRF changes now overlap with #1063 (DNS pinning), which merged today — several of cubic's "P1 SSRF" findings on this PR are about that stacked code, and re-proposing it here risks regressing what #1063 landed. Please don't carry those in this PR; if anything in that area is still wanted, it should be a separate PR reconciled against #1063.
• A credential-encryption change should be reviewable and revertable on its own.

Rebased onto main and reduced to the KDF files, this should be a small, clean diff (and tests/test_encryption.py already passes 11/11 in isolation).

2. The migration story — I'd like your opinion here

The PR is "no migration required," which is true for correctness (old ciphertext keeps decrypting via the SHA-256 fallback). But the re-encryption is lazy: a value only upgrades to PBKDF2 the next time its record is saved. Encrypted fields today are just the two api_key columns (Credential, ProviderConfig) — and an API key is the classic set-once-and-forget value. So in practice:

• The records we most want to protect never get re-saved, so they stay under the weak SHA-256 derivation indefinitely — the security benefit isn't realized for them.
• The legacy decrypt path then has to live in the code forever, or old-but-never-resaved ciphertext becomes undecryptable the day it's removed.

An explicit one-shot re-encryption would close both. Because decrypt already handles both schemes, the loop is trivial and idempotent:

for each Credential / ProviderConfig with an api_key:
    plaintext = decrypt_value(raw)       # transparently legacy-or-new
    new_ct    = encrypt_value(plaintext) # always PBKDF2
    if new_ct != raw: save

Note this can't be a .surrealql migration (the AsyncMigrationManager runs schema SQL, not Fernet/PBKDF2) — it'd be a Python data migration: a startup hook or a commands/ management command. Safety rules whichever way it goes: fail-closed per record (if a value decrypts under neither scheme — e.g. the key was rotated — skip and log, never overwrite with garbage), resolve the key exactly like runtime (including OPEN_NOTEBOOK_ENCRYPTION_KEY_FILE), and ship the legacy fallback before/with the migration (re-encryption is one-way — no rollback to a SHA-256-only build).

What's your take? Options as I see them:

• (a) add a one-shot re-encryption command (commands/) the operator runs once — realizes the benefit for all credentials and lets us retire the legacy fallback on a known timeline; or
• (b) keep it lazy but document explicitly that un-resaved credentials stay on the old scheme and that the fallback is permanent.

I lean (a), but you've thought about this more than anyone — curious whether you considered an active pass and chose lazy deliberately. Either way it's a structural + policy decision, so it probably deserves a short decision record in docs/7-DEVELOPMENT/decisions/.

3. Minor: nudge users toward a high-entropy key (non-blocking)

cubic's note on the constant salt is fair — a fixed salt lets identical weak passphrases be amortized across installs (PBKDF2 iterations still make each guess expensive, so it's minor). The stronger mitigation is orthogonal to the KDF: if the key is high-entropy, SHA-256 vs PBKDF2 doesn't matter at all. Worth a line in security.md recommending a long random key (e.g. openssl rand -base64 36) alongside this change.

Thanks again — the cryptographic core is right; it's mostly a matter of getting it out from under the stack and settling how aggressively we migrate existing data.