Repository navigation
Security hardening: password-hash upgrade path at the Workers PBKDF2 cap, secret-store AAD/versioning, prod rate-limiter fail-closed, audit fixes, SECURITY.md #1382
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| # Security Policy | ||
|
|
||
| Kody is a multi-user personal assistant. Every user gets a fully isolated | ||
| assistant, and the core security invariant is that **all data access is scoped | ||
| by `userId`** — cross-user data exposure of any kind is a vulnerability, not a | ||
| feature gap. | ||
|
|
||
| ## Reporting a vulnerability | ||
|
|
||
| Please report vulnerabilities privately — do not open a public issue. | ||
|
|
||
| - Preferred: | ||
| [GitHub private vulnerability reporting](https://github.com/kentcdodds/kody/security/advisories/new) | ||
| - Alternative: email [me@kentcdodds.com](mailto:me@kentcdodds.com) with | ||
| "SECURITY" in the subject line | ||
|
|
||
| Kody is maintained by a solo developer, so please allow a few days for an | ||
| initial response. You'll get an acknowledgment, an assessment, and updates until | ||
| the issue is resolved. Good-faith security research against your own account is | ||
| welcome; please don't access or disrupt other users' data. | ||
|
|
||
| ## Scope | ||
|
|
||
| Reports we especially care about: | ||
|
|
||
| - Cross-user data access (any read/write path missing `userId` scoping) | ||
| - Sandbox escapes from hosted package code (Worker Loader isolates, the fetch | ||
| gateway, credential stripping, package-app origin isolation) | ||
| - Authentication/session flaws (password auth, OAuth/PKCE, MCP bearer tokens, | ||
| 2FA, passkeys) | ||
| - Secret-store weaknesses (encryption at rest, secret substitution through the | ||
| fetch gateway) | ||
|
|
||
| Out of scope: vulnerabilities that only affect your own account's data, missing | ||
| best-practice headers without a demonstrated impact, and denial of service via | ||
| volume alone. | ||
|
|
||
| ## Supported versions | ||
|
|
||
| Only the latest deployed version (the `main` branch) is supported. There are no | ||
| maintained release lines. | ||
|
|
||
| ## Security model documentation | ||
|
|
||
| The authoritative record of the security invariants, threat model, and accepted | ||
| residual risks lives in | ||
| [docs/contributing/security.md](./docs/contributing/security.md). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,3 +42,30 @@ registry, the `MCP_OBJECT` Durable Object, and the session purge path. Tasks | |
| yet: the SDK v2 ships the vocabulary without a runtime, no major client supports | ||
| it, and execute's idempotency-key + `run_get` flow already covers the need; | ||
| revisit when a major host ships task support. | ||
|
|
||
| ## Retirement criterion (added 2026-08-10) | ||
|
|
||
| "Zero or negligible" is made mechanical so the retirement call is a readout, not | ||
| a judgment. Run the weighted lane query from | ||
| `packages/worker/src/mcp/protocol-metrics.ts` over a trailing 30-day window | ||
| (Analytics Engine retains 90 days, so the window is always fully covered): | ||
|
|
||
| ```sql | ||
| SELECT blob1 AS lane, blob4 AS client_name, SUM(_sample_interval) AS requests | ||
| FROM kody_mcp_protocol_events | ||
| WHERE timestamp > NOW() - INTERVAL '30' DAY | ||
| GROUP BY lane, client_name | ||
| ORDER BY requests DESC | ||
| ``` | ||
|
|
||
| Retire the legacy lane when **both** hold for the window: | ||
|
|
||
| 1. legacy-lane requests are **< 1% of total `/mcp` requests**, and | ||
| 2. **no single `client_name` accounts for more than 100 legacy requests** (a | ||
| named client above that is a real installation that would break, not noise; | ||
| unnamed/blank clients count as noise). | ||
|
There was a problem hiding this comment. 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift Do not treat blank client identities as noise.
🤖 Prompt for AI Agents
Contributor
Author
There was a problem hiding this comment. Fixed in 034bf62 — the blank/unnamed There was a problem hiding this comment.
|
||
|
|
||
| When both hold, remove the `McpAgent` lane, the `mcp_agent_sessions` registry, | ||
| the `MCP_OBJECT` Durable Object, and the session purge path in one change. If a | ||
| later reading fails the criterion again before the removal ships, reset the | ||
|
There was a problem hiding this comment. 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files 'docs/contributing/decisions/0005-mcp-dual-lane-stateless-migration.md' \
'packages/worker/src/mcp/session-registry.ts' \
'packages/worker/src/mcp/protocol-metrics.ts' \
'packages/worker/src/mcp/*' \
| sed -n '1,120p'
printf '%s\n' '--- decision excerpt ---'
sed -n '35,90p' docs/contributing/decisions/0005-mcp-dual-lane-stateless-migration.md
printf '%s\n' '--- session registry ---'
sed -n '1,130p' packages/worker/src/mcp/session-registry.ts
printf '%s\n' '--- protocol metrics ---'
sed -n '1,220p' packages/worker/src/mcp/protocol-metrics.tsRepository: kentcdodds/kody Length of output: 17287 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- all legacy-session references ---'
rg -n -C 3 \
'mcp_agent_sessions|purgePersistedMcpAgentSession|listMcpAgentSessionsForUser|MCP_OBJECT|McpAgent' \
--glob '!node_modules' --glob '!dist' --glob '!build' \
packages docs migrations 2>/dev/null | sed -n '1,260p'
printf '%s\n' '--- relevant worker files ---'
fd -i -t f 'mcp|migration|wrangler' packages/worker | \
rg 'session|durable|migration|wrangler|mcp' | sed -n '1,220p'
printf '%s\n' '--- database schema references ---'
rg -n -C 4 'CREATE TABLE[^;]*mcp_agent_sessions|mcp_agent_sessions' \
--glob '*.sql' --glob '*.ts' --glob '*.md' . | sed -n '1,240p'Repository: kentcdodds/kody Length of output: 41622 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- account deletion implementation ---'
ast-grep outline packages/worker/src/app/account-deletion.ts
rg -n -C 8 \
'listMcpAgentSessionsForUser|purgeForAccountDeletion|purgePersistedMcpAgentSession|mcp_agent_sessions|MCP_OBJECT' \
packages/worker/src/app/account-deletion.ts packages/worker/src/mcp packages/worker/src/index.ts packages/worker/src/mcp-auth.ts \
| sed -n '1,300p'
printf '%s\n' '--- MCP Durable Object implementation ---'
fd -i -t f 'index.ts' packages/worker/src/mcp --exec sh -c '
for f do
if rg -q "class MCP|purgeForAccountDeletion|McpAgent" "$f"; then
printf "%s\n" "--- $f ---"
ast-grep outline "$f"
rg -n -C 8 "class MCP|purgeForAccountDeletion|purgePersistedMcpAgentSession|McpAgent" "$f"
fi
done
' sh
printf '%s\n' '--- Durable Object migration/config declarations ---'
rg -n -C 8 \
'MCP|migrations|new_sqlite_classes|deleted_classes|Analytics Engine|MCP_PROTOCOL_EVENTS' \
packages/worker/wrangler.jsonc packages/worker/worker-configuration.d.ts packages/worker/src \
| sed -n '1,300p'Repository: kentcdodds/kody Length of output: 42728 🌐 Web query:
💡 Result: Yes, deleting a Durable Object class permanently removes its namespace and all associated stored data [1]. To safely retire a Durable Object class, you must follow the migration process for declarative exports (or the legacy migration flow for older projects) [1][2]. Simply removing the binding from your configuration file is not sufficient and will generally prevent the deployment from succeeding if the class is still defined in your code or referenced in other ways [1][3]. Recommended process for retiring/deleting a Durable Object class: 1. Remove references: Ensure your Worker code no longer references the Durable Object class [1]. 2. Update configuration: In your Wrangler configuration file, change the class entry in the exports map to a deleted tombstone [1][4]. { "exports": { "YourClassName": { "type": "durable-object", "state": "deleted" } } } 3. Check for external dependencies: Cloudflare will prevent the deletion if other Workers in your account still bind to the namespace [1]. You must remove those bindings first [1]. 4. Redeploy: Apply the change by deploying your Worker [1]. Once the deployment is successful and the namespace is deleted, the tombstone becomes stale [1][5]. 5. Cleanup: After the namespace is deleted, the tombstone entry will appear in the removable_entries list during your deployment output [1]. You can then safely remove the class entry entirely from your exports map in your next configuration update [1][5]. Regarding data cleanup, while deleting the class wipes all data, you may want to explicitly call storage.deleteAll within your Durable Object code before retiring a class if you need to ensure complete deallocation or handle specific application-level data cleanup before the object ceases to exist [6][4]. Keep in mind that for SQLite-backed objects, deleteAll is atomic, whereas for legacy key-value backends, it may require retries if it hits time limits [7][8]. Citations:
🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- migration inventory ---'
git ls-files packages/worker/migrations | sort
for f in $(git ls-files packages/worker/migrations | sort); do
printf '%s\n' "--- $f ---"
rg -n -C 3 'mcp_agent_sessions|DROP TABLE|deleted_classes|MCP' "$f" || true
done
printf '%s\n' '--- deletion ordering and row cleanup ---'
sed -n '360,470p' packages/worker/src/app/account-deletion.ts
sed -n '788,910p' packages/worker/src/app/account-deletion.ts
sed -n '1035,1145p' packages/worker/src/app/account-deletion.ts
printf '%s\n' '--- all direct D1 table cleanup declarations ---'
rg -n -C 3 'DROP TABLE|DELETE FROM .*mcp_agent_sessions|mcp_agent_sessions' \
packages/worker/migrations packages/worker/src \
| sed -n '1,260p'Repository: kentcdodds/kody Length of output: 35886 🏁 Script executed: python3 - <<'PY'
from pathlib import Path
import re
migration_dir = Path("packages/worker/migrations")
migrations = sorted(migration_dir.glob("*.sql"))
registry_creators = []
registry_drops = []
for path in migrations:
text = path.read_text()
if re.search(r"\b(?:CREATE\s+TABLE|INSERT\s+INTO|DELETE\s+FROM|SELECT\b.*\bFROM)\s+mcp_agent_sessions\b", text, re.I | re.S):
registry_creators.append(path.name)
if re.search(r"\bDROP\s+TABLE\s+(?:IF\s+EXISTS\s+)?mcp_agent_sessions\b", text, re.I):
registry_drops.append(path.name)
consumers = []
for root in (Path("packages/worker/src"), Path("packages/worker/worker-configuration.d.ts")):
paths = [root] if root.is_file() else root.rglob("*")
for path in paths:
if path.is_file() and path.suffix in {".ts", ".tsx", ".js", ".jsonc"}:
text = path.read_text(errors="replace")
if "mcp_agent_sessions" in text:
consumers.append(str(path))
print({"migrations": [p.name for p in migrations]})
print({"registry_references_in_migrations": registry_creators})
print({"registry_drop_migrations": registry_drops})
print({"runtime_or_config_consumers": sorted(consumers)})
PYRepository: kentcdodds/kody Length of output: 975 Drop
🤖 Prompt for AI AgentsSource: Coding guidelines
Contributor
Author
There was a problem hiding this comment. Fixed in 034bf62 — the retirement list now includes a D1 migration dropping There was a problem hiding this comment.
|
||
| clock — the window must be the 30 days immediately preceding the removal PR. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,6 +62,13 @@ package-app surfaces: | |
| package-app origin, and do not reintroduce cookie forwarding "just for | ||
| convenience". See | ||
| [Hosted package app origin isolation](#hosted-package-app-origin-isolation). | ||
| 10. **CSRF protection is `SameSite=Lax` + JSON content types, not tokens.** | ||
| Every mutating first-party endpoint must keep requiring a JSON | ||
| `Content-Type` (which cross-site form posts cannot send) and the | ||
| `kody_session` cookie must stay `SameSite=Lax`. Do not add a mutating | ||
| endpoint that accepts `application/x-www-form-urlencoded` or | ||
| `multipart/form-data` from the browser, and do not relax the cookie to | ||
| `SameSite=None`, without adding CSRF tokens at the same time. | ||
|
|
||
| ## First-party HTTP security headers | ||
|
|
||
|
|
@@ -188,10 +195,14 @@ session is bound to one account), but it is not zero. | |
| ## Auth rate limiting | ||
|
|
||
| Credential-accepting POST endpoints share one per-IP auth rate-limit bucket | ||
| (`auth:ip:<ip>`, `packages/worker/src/app/rate-limit.ts`, default 10 requests | ||
| per 60-second window). Deployed environments use the Cloudflare rate-limit | ||
| binding (`AUTH_RATE_LIMITER` in `packages/worker/wrangler.jsonc`); local dev, | ||
| tests, and self-hosted configs fall back to a D1 atomic limiter. Production | ||
| fails closed: env validation (`packages/worker/src/app/env.ts`) rejects a | ||
| production runtime without the binding, so the D1 fallback can never silently | ||
| become the production limiter. The shared bucket means brute-force attempts | ||
| cannot fan out across parallel paths. Covered paths (`rateLimitedAuthPaths` in | ||
| `packages/worker/src/index.ts`): | ||
|
|
||
| - `POST /auth` (password login/signup) | ||
|
|
@@ -298,7 +309,12 @@ guarded by a bearer secret comparison. | |
| ## Secrets and user code execution (in-scope model) | ||
|
|
||
| - Saved secrets are encrypted at rest with AES-GCM under `SECRET_STORE_KEY` and | ||
| scoped by `userId` (`packages/worker/src/mcp/secrets/`). Ciphertexts are | ||
| versioned (`v2.<iv>.<ct>`) and bound via AES-GCM additional authenticated data | ||
| to their purpose and owning identity (`user:<userId>` for user secrets, | ||
| `app:<slug>` for platform OAuth client secrets), so a ciphertext copied into | ||
| another user's row fails to decrypt. Legacy unversioned ciphertexts still | ||
| decrypt and upgrade to `v2` whenever the value is re-encrypted on write. | ||
|
There was a problem hiding this comment. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 8 \
'legacy|v2\.|additionalData|decryptSecret|encryptSecret|re-?encrypt' \
packages/worker/src/mcp/secretsRepository: kentcdodds/kody Length of output: 27831 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- crypto implementation ---'
cat -n packages/worker/src/mcp/secrets/crypto.ts | sed -n '70,115p'
printf '%s\n' '--- secret read and write paths ---'
cat -n packages/worker/src/mcp/secrets/service.ts | sed -n '600,750p'
printf '%s\n' '--- documentation ---'
cat -n docs/contributing/security.md | sed -n '300,325p'
printf '%s\n' '--- relevant ownership and lookup helpers ---'
rg -n -C 5 \
'getSecretEntry|bucketId|userId|owner|secret.*scope|scope.*secret' \
packages/worker/src/mcp/secrets packages/worker/src/mcp \
-g '*.ts' | head -n 300
printf '%s\n' '--- standalone no-AAD AES-GCM probe ---'
node - <<'JS'
const { webcrypto } = require('crypto');
const crypto = webcrypto;
(async () => {
const keyBytes = await crypto.subtle.digest(
'SHA-256',
new TextEncoder().encode('mcp-secret-store:primary-secret-store-key-at-least-32-chars!!')
);
const key = await crypto.subtle.importKey('raw', keyBytes, 'AES-GCM', false, ['encrypt', 'decrypt']);
const iv = crypto.getRandomValues(new Uint8Array(12));
const plaintext = new TextEncoder().encode('legacy-value');
const ciphertext = await crypto.subtle.encrypt({ name: 'AES-GCM', iv }, key, plaintext);
for (const aad of [undefined, new TextEncoder().encode('secret-store:user-a'), new TextEncoder().encode('secret-store:user-b')]) {
try {
const options = { name: 'AES-GCM', iv };
if (aad !== undefined) options.additionalData = aad;
const result = await crypto.subtle.decrypt(options, key, ciphertext);
console.log(aad === undefined ? 'no AAD: decrypts' : `AAD ${new TextDecoder().decode(aad)}: decrypts`, new TextDecoder().decode(result));
} catch {
console.log(aad === undefined ? 'no AAD: rejects' : `AAD ${new TextDecoder().decode(aad)}: rejects`);
}
}
})();
JSRepository: kentcdodds/kody Length of output: 31343 Document the legacy ciphertext isolation gap. If a legacy ciphertext is copied into another user's row, 🤖 Prompt for AI AgentsSource: Coding guidelines
Contributor
Author
There was a problem hiding this comment. Fixed in 034bf62 — security.md now documents that only v2 provides owner binding, that legacy ciphertexts remain swap-decryptable until rewritten (metadata-only writes preserve them), and that the compatibility branch is deliberately upgrade-on-write and gets removed once no legacy ciphertexts remain. There was a problem hiding this comment.
|
||
| - `SECRET_STORE_KEY` is escrowed for disaster recovery as a passphrase-sealed | ||
| blob in the DR backup bucket (solo operator; see | ||
| [Disaster recovery](./disaster-recovery.md) and | ||
|
|
@@ -386,9 +402,15 @@ change to these decisions here so future agents do not relitigate them. | |
| - **Sandbox `fetch` has no general SSRF denylist.** Secret-bearing requests are | ||
| constrained by per-secret host allowlists; non-secret requests rely on the | ||
| Cloudflare Workers platform egress model. | ||
| - **PBKDF2-SHA256 (600k iterations)** is used for password hashing rather than a | ||
| memory-hard KDF. Acceptable here: it meets OWASP's PBKDF2-SHA256 guidance, and | ||
| Workers' WebCrypto has no argon2/scrypt. Verification accepts any iteration | ||
| count up to a hard ceiling well above the generation setting, and | ||
| lower-iteration hashes are transparently re-hashed after a successful login | ||
| (`packages/worker/src/password-upgrade.ts`), so the setting can be raised | ||
| again later without a migration. | ||
| - **No CSRF tokens.** State-changing requests are protected by `SameSite=Lax` | ||
| cookies plus JSON `Content-Type` on mutating endpoints. This is a deliberate | ||
| decision, restated as invariant 10 above: it holds only while both halves | ||
| hold, so revisit if any mutating endpoint starts accepting cross-site form | ||
| posts or `SameSite=None`. | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
Repository: kentcdodds/kody
Length of output: 29749
🏁 Script executed:
Repository: kentcdodds/kody
Length of output: 50373
🏁 Script executed:
Repository: kentcdodds/kody
Length of output: 32366
🏁 Script executed:
Repository: kentcdodds/kody
Length of output: 9533
🏁 Script executed:
Repository: kentcdodds/kody
Length of output: 442
Align the denominator with the recorded metric scope.
The metric records only authenticated, verified, non-suspended requests. The query therefore does not represent total
/mcproute traffic. Define criterion 1 using total authenticated, instrumented/mcprequests, or add a metric for every/mcprequest.🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 034bf62 — the criterion now uses total instrumented
/mcprequests as the denominator and explains why that population is the right one.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.