fix(ai-review): BYOK round 2 — key-lifecycle audit, budget separation, per-record salt

[JSONbored]

Second hardening round from the BYOK security audit (follow-up to #670).

Changes

• BYOK: audit-log provider-key lifecycle (set/replace/delete) + surface createdBy/updatedAt #671 — Audit the provider-key lifecycle. upsert/delete now emit an ai_key_change ai_usage_events row (status = set/replace/delete, the actor, and the display-only last4 — never any key material), written with a non-ok status so it never counts toward the neuron budget. getRepositoryAiKeyStatus now surfaces createdBy + updatedAt so the dashboard can show who set the key and when. The DELETE route threads the session actor through.
• BYOK: separate BYOK advisory spend from the free Workers-AI daily neuron budget #672 — Separate BYOK spend from the free budget. The daily neuron budget now meters only free Workers-AI calls (the consensus pair in block mode + the advisory leg when it is not BYOK). A BYOK advisory bills the maintainer's own account and now still runs when the free budget is exhausted.
• BYOK: per-record PBKDF2 salt for at-rest key derivation (defense-in-depth) #677 — Per-record PBKDF2 salt (envelope v2). encryptSecret generates a fresh random salt per record and stores it beside the IV; decryptSecret falls back to the legacy constant salt when no per-record salt is present, so existing v1 rows keep decrypting. Migration 0030 adds the nullable salt column.
• BYOK: clarify repository_ai_keys.key_version (crypto-envelope version, not rotation) #676 — Clarify key_version as the crypto-envelope version (1 = legacy constant-salt, 2 = per-record salt), not a rotation counter.

Not included (deferred with rationale)

• BYOK: tighten ai-key writes to repo owners (not org MEMBER / read COLLABORATOR) #674 (tighten ai-key writes to owners) is intentionally not in this PR. Code analysis shows the "owned" access axis is installation-account-based, so naively restricting to it would lock out legitimate org-repo maintainers (org repos surface no OWNER author-association). The correct fix resolves real GitHub push permission via the installation (an API call) — a larger, focused change. Tracked in BYOK: tighten ai-key writes to repo owners (not org MEMBER / read COLLABORATOR) #674.

Tests

• Lifecycle audit trail: set → replace → delete recorded with the right actors and no key material; a no-op delete records nothing.
• BYOK advisory runs with the free budget exhausted (estimatedNeurons === 0).
• Crypto: v2 round-trip (fresh salt per record) + v1 legacy decryption (constant salt) + wrong-secret rejection.

Verification

typecheck ✅ · test:coverage ✅ (97% branch, 1676 tests) · test:workers ✅ · git diff --check ✅

Closes #671, #672, #676, #677. Part of #525.

[ghost]

Note

Gittensory Gate skipped

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

Signal	Result	Evidence	Action
Gate result	⚠️ Skipped	#680 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]

gittensory · advisory review

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

Suggested action: 🛠️ Request changes. (reviewers split: request changes / merge) Address the suggestions below before merging.

Reviewer A · gpt-oss-120b — recommends 🛠️ request changes
This PR adds per‑record salts, key‑versioning, and lifecycle auditing for BYOK keys, and adjusts AI budget accounting. However, it introduces new columns (createdBy, updatedAt) in the repository key status and upserts without adding those columns to the database schema, which will cause runtime failures. It also adds an unnecessary node_modules placeholder.

Suggestions

• Add createdBy (TEXT, nullable) and updatedAt (TEXT, nullable) columns to the repository_ai_keys table via a migration and update src/db/schema.ts accordingly.
• Update the migration to set sensible defaults for the new columns (e.g., CURRENT_TIMESTAMP for updatedAt).
• Remove the stray node_modules placeholder file from the repo.
• Ensure any existing rows are back‑filled with appropriate values or handled gracefully during the migration.
• Consider whether exposing createdBy in the public RepositoryAiKeyStatus is acceptable; if not, omit it from the public type.

Worth double-checking

• Database schema mismatch leading to insert/update errors at runtime.
• Potential exposure of maintainer usernames if createdBy is publicly returned.
• Missing back‑fill of existing rows could cause nullability issues or break queries.

Reviewer B · nemotron-3-120b-a12b — recommends ✅ merge
This PR enhances BYOK key management with per-record salt for encryption (defense-in-depth), adds lifecycle audit logging (set/replace/delete) that excludes key material, and separates BYOK advisory calls from the free neuron budget. Changes are well-tested and maintain the public/private boundary by never exposing secrets in audit logs.

Suggestions

• Consider adding a comment in recordAiKeyChange clarifying why status uses the action string (non-'ok' to exclude from budget).
• In decryptSecret, the salt parameter is optional but could be made explicit with a default null for clarity.
• Ensure the migration note about v1/v2 compatibility is kept up-to-date if envelope versions evolve.

Worth double-checking

• Backward compatibility: existing v1 rows (salt=NULL) must continue decrypting with the legacy constant salt - verified in tests.
• Audit log size: key lifecycle events are infrequent, but monitor ai_usage_events growth if key rotation becomes frequent.
• Type safety: ensure the new salt column's TEXT type aligns with base64 storage (no length issues).

[superagent-security]

Superagent found 1 security concern(s).

[superagent-security]

P2: Symlink node_modules pointing to absolute local path committed to repository

Committed a node_modules symlink to an absolute local filesystem path.

Remove the symlink from git and ensure .gitignore blocks node_modules.

AI prompt

Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.

<file name="node_modules">
<violation number="1" location="node_modules:1">
<priority>P2</priority>
<title>Symlink `node_modules` pointing to absolute local path committed to repository</title>
<evidence>A symlink named `node_modules` was added to the repository root, pointing to the absolute local path `/Users/shadowbook/Documents/gittensory/node_modules`. This reveals local filesystem structure and can interfere with package installation by redirecting dependency resolution to an external directory.</evidence>
<recommendation>Remove the `node_modules` symlink from version control and add `node_modules` to `.gitignore` if not already present. Verify that no build or deployment pipeline follows this symlink unexpectedly.</recommendation>
</violation>
</file>