feat(server)!: crash-safe storage, write preconditions, and honest deletion

README.md

@@ -167,15 +167,29 @@ All bodies are ciphertext; the server validates sizes/hashes without decrypting.
 
 | Method | Route | Purpose |
 |---|---|---|
-| `GET`  | `/health` | liveness + active store |
+| `GET`  | `/health` | liveness |
 | `GET`  | `/api/memory/:ns` | full data (ciphertext entries + checksums) |
 | `GET`  | `/api/memory/:ns?view=hashes` | per-key checksums only (for delta) |
-| `PUT`  | `/api/memory/:ns` | delta upsert `{ entries, deletions? }` |
-| `DELETE` | `/api/memory/:ns?key=<entryKey>` | remove one entry |
+| `PUT`  | `/api/memory/:ns` | delta upsert `{ entries, deletions?, base? }` |
+| `DELETE` | `/api/memory/:ns?key=<entryKey>[&base=sha256:<hex>]` | remove one entry |
 
 A *namespace* (`user:me`, `repo:owner/name`, `agent:intern`) is the unit of
 scoping. Entry keys mirror the memdir layout (`MEMORY.md`, `feedback/x.md`).
 
+The hashes view also reports `supports` (server capabilities a client can rely
+on) and `erasure` (whether this deployment's store actually removes bytes on
+delete); write responses carry `erasure` too.
+
+`base` is an optional precondition: a map of entry key to the ciphertext hash
+the caller believes that key holds, or `null` for "should not exist". A request
+that disagrees with the stored manifest is refused with `409 stale_base_version`
+and a `details.conflicts` map naming what each key actually holds. Omitting
+`base` writes unconditionally, which is what a client that has not adopted it
+still does. This guards the caller's own turn, not the moment between its last
+read and its write, which is why the check is per entry rather than a namespace
+version. A namespace whose manifest cannot be parsed answers
+`503 manifest_unreadable` rather than appearing empty.
+
 ## Configuration
 
 See [`.env.example`](./.env.example). Key knobs: `STORE` (`fs`|`s3`),

RELEASE-0.1.md

@@ -98,7 +98,10 @@ Make the server safe to expose to strangers before anyone connects.
   with explicit segment matching + an ACL hook stub. **Security-critical.**
 - **Request hardening**: enforce `MAX_BODY_BYTES` even when `content-length` is
   absent (stream cap), reject unknown methods early, add security headers.
-- **Health/readiness**: `/health` already exists; add store round-trip check.
+- **Health/readiness**: `/health` reports liveness only. The store round trip
+  moved to a startup probe rather than the route: `/health` is unauthenticated,
+  so a store check there is an anonymous write against the store holding every
+  tenant's ciphertext, and echoing the store's label leaked it to anyone.
 
 ### M2 — MCP server (the headline) · ~3–4d
 `memlawb mcp` stdio subcommand wrapping the already-bundled `MemlawbClient`.

src/handler.ts

@@ -16,6 +16,7 @@
 
 import { authenticate, authorizeNamespace } from './auth.ts'
 import { config } from './config.ts'
+import { logRejection } from './log.ts'
 import { getData, getHashes, upsert } from './memory.ts'
 import {
   InvalidNameError,
@@ -25,8 +26,7 @@ import {
 } from './namespace.ts'
 import { QuotaError } from './quota.ts'
 import { take } from './ratelimit.ts'
-import { getStore } from './store/index.ts'
-import { parseUpsertRequest } from './types.ts'
+import { isBaseHash, parseUpsertRequest, StaleBaseError, UnreadableManifestError } from './types.ts'
 
 // Applied to every response. The API serves only JSON and is consumed by
 // programmatic clients, so we lock down sniffing/caching/referrer leakage.
@@ -61,27 +61,115 @@ function parseMemoryPath(pathname: string): { namespace: string } | null {
   return { namespace: rest }
 }
 
+/**
+ * Map the two refusals `upsert` can raise. Deliberately not folded into the
+ * outer catch: that would widen these statuses to cover every read path in the
+ * handler, so a future read that threw QuotaError would silently answer 413
+ * instead of 500. Returns null for anything else, which the caller rethrows.
+ */
+function upsertFailure(err: unknown): Response | null {
+  if (err instanceof QuotaError) return apiError(err.code, err.message, 413, err.details)
+  if (err instanceof StaleBaseError) return apiError(err.code, err.message, 409, err.details)
+  return null
+}
+
+/**
+ * A namespace whose index cannot be parsed answers 503 with its own code rather
+ * than a generic 500, on reads as well as writes. The refusal is right, but
+ * "internal error" tells a caller to retry something no retry can fix, and
+ * leaves an operator unable to tell this apart from any other server fault.
+ */
+function manifestFailure(err: unknown): Response | null {
+  if (err instanceof UnreadableManifestError) return apiError(err.code, err.message, 503)
+  return null
+}
+
+/**
+ * Log every refusal from one place, after the response is built, so the code
+ * recorded is literally the code the caller received and no future refusal
+ * branch can be added without being covered.
+ */
 export async function handleRequest(req: Request): Promise<Response> {
+  const ctx: RequestContext = { ...DEFAULT_CONTEXT }
+  // respond() parses the path before its own try block, so a malformed percent
+  // escape throws past it. Without this guard that reaches the runtime as an
+  // unhandled error: no envelope, no security headers, and no log line, which
+  // would make the claim above false for a request anyone can send.
+  let res: Response
+  try {
+    res = await respond(req, ctx)
+  } catch (err) {
+    console.error(`[memlawb] handler error (${(err as Error)?.constructor?.name ?? 'unknown'})`)
+    res = apiError('internal', 'internal error', 500)
+  }
+  if (res.status >= 400) {
+    let code = 'unknown'
+    try {
+      const body = (await res.clone().json()) as { error?: { code?: string } }
+      if (body.error?.code) code = body.error.code
+    } catch {
+      // A refusal with a non-JSON body still gets a line; the code stays unknown.
+    }
+    logRejection({ owner: ctx.owner, code, status: res.status, route: ctx.route })
+  }
+  return res
+}
+
+/** Per-request facts the rejection log needs, filled in as they become known. */
+type RequestContext = { owner: string; route: string }
+
+/**
+ * What a request is assumed to be before anything is known about it. Exported
+ * so the defaults are pinned somewhere: under an open-auth configuration every
+ * caller authenticates, so no test driving the handler can observe them.
+ */
+export const DEFAULT_CONTEXT: Readonly<RequestContext> = Object.freeze({
+  owner: 'anonymous',
+  route: 'other',
+})
+
+/**
+ * Which token bucket a request draws from: its own account when it
+ * authenticated, one shared anonymous bucket when it did not. Keying everything
+ * on the shared bucket would let anonymous abuse throttle real accounts; keying
+ * nothing on it leaves the pre-auth refusal branches, which now write a log
+ * line each, unthrottled entirely.
+ */
+export function bucketKey(identity: { owner: string } | null): string {
+  return identity?.owner ?? 'anonymous'
+}
+
+async function respond(req: Request, ctx: RequestContext): Promise<Response> {
   const url = new URL(req.url)
   const { pathname } = url
 
   if (pathname === '/health') {
-    return json({ ok: true, store: getStore().describe(), service: 'memlawb' })
+    return json({ ok: true, service: 'memlawb' })
   }
 
   const parsed = parseMemoryPath(pathname)
-  if (!parsed) return apiError('not_found', 'unknown route', 404)
+  if (parsed) ctx.route = 'memory'
 
+  // Authenticate before refusing anything, so the throttle below can key on the
+  // caller when there is one. Every refusal writes a log line, and the unknown
+  // route and unauthorized branches used to sit ahead of the bucket entirely,
+  // which let an unauthenticated caller turn a trivially cheap request into
+  // unbounded log volume on the machine holding every tenant's ciphertext.
   const identity = await authenticate(req)
-  if (!identity) return apiError('unauthorized', 'missing or invalid API key', 401)
+  ctx.owner = bucketKey(identity)
 
-  const rate = take(identity.owner, Date.now())
+  // One shared bucket for callers who did not authenticate, their own bucket
+  // for those who did, so anonymous abuse cannot throttle a real account.
+  const rate = take(ctx.owner, Date.now())
   if (!rate.ok) {
     return apiError('rate_limited', 'too many requests', 429, undefined, {
       'retry-after': String(rate.retryAfterSec),
     })
   }
 
+  if (!parsed) return apiError('not_found', 'unknown route', 404)
+  if (!identity) return apiError('unauthorized', 'missing or invalid API key', 401)
+
   let namespace: string
   try {
     namespace = validateNamespace(parsed.namespace)
@@ -135,9 +223,8 @@ export async function handleRequest(req: Request): Promise<Response> {
         )
         return json(result)
       } catch (err) {
-        if (err instanceof QuotaError) {
-          return apiError(err.code, err.message, 413, err.details)
-        }
+        const mapped = upsertFailure(err)
+        if (mapped) return mapped
         throw err
       }
     }
@@ -150,19 +237,36 @@ export async function handleRequest(req: Request): Promise<Response> {
       } catch (err) {
         return apiError('invalid_key', (err as Error).message, 400)
       }
-      const result = await upsert(
-        namespace,
-        nsSlug,
-        identity.owner,
-        { entries: {}, deletions: [key] },
-        new Date().toISOString(),
-      )
-      return json(result)
+      // The base rides the query here rather than a body, so it needs its own
+      // shape check: parseUpsertRequest never sees a DELETE.
+      const rawBase = url.searchParams.get('base')
+      if (rawBase !== null && !isBaseHash(rawBase)) {
+        return apiError('bad_request', '`base` must be a sha256:<hex> hash', 400)
+      }
+      const base = rawBase === null ? undefined : { [key]: rawBase }
+      try {
+        const result = await upsert(
+          namespace,
+          nsSlug,
+          identity.owner,
+          { entries: {}, deletions: [key], base },
+          new Date().toISOString(),
+        )
+        return json(result)
+      } catch (err) {
+        const mapped = upsertFailure(err)
+        if (mapped) return mapped
+        throw err
+      }
     }
 
     return apiError('method_not_allowed', `${req.method} not supported`, 405)
   } catch (err) {
-    console.error('[memlawb] handler error', err)
+    const manifest = manifestFailure(err)
+    if (manifest) return manifest
+    // The error's class only. A store error commonly carries an endpoint, a
+    // bucket and an object path, and that path carries a namespace slug.
+    console.error(`[memlawb] handler error (${(err as Error)?.constructor?.name ?? 'unknown'})`)
     return apiError('internal', 'internal error', 500)
   }
 }

src/index.ts

@@ -9,6 +9,14 @@
 import { config } from './config.ts'
 import { handleRequest } from './handler.ts'
 import { getStore } from './store/index.ts'
+import { probeStore } from './store/probe.ts'
+
+const probe = await probeStore()
+if (!probe.ok) {
+  // Refuse to serve rather than answer 200 over a store we cannot reach.
+  process.stderr.write(`[memlawb] store probe failed: ${probe.detail}\n`)
+  process.exit(1)
+}
 
 const server = Bun.serve({
   port: config.port,

src/log.ts

@@ -0,0 +1,82 @@
+/**
+ * The rejection log.
+ *
+ * Observability here is one line per refused request, and the field set is an
+ * allowlist rather than a denylist. The space of things that must never appear
+ * in a log on a crypto-blind server is open-ended (entry keys, raw namespaces,
+ * ciphertext, tokens, whatever a future error object carries), so a denylist
+ * only ever catches what someone thought to forbid. A fixed set of five fields
+ * cannot carry any of it, because there is nowhere for it to go.
+ *
+ * A namespace slug is deliberately absent. It reads as opaque, but it is a hash
+ * of a low-entropy namespace like `user:alice`, so it is a stable per-tenant
+ * identifier anyone can reverse by dictionary.
+ */
+
+/** The complete set of keys a rejection line may carry. */
+export const ALLOWED_FIELDS = ['timestamp', 'owner', 'code', 'status', 'route'] as const
+
+/**
+ * One refusal. The typed shape has no index signature on purpose: a field that
+ * is not one of these has no way into the object, so the allowlist is enforced
+ * by the compiler and not only by the test.
+ */
+export type Rejection = {
+  timestamp: string
+  /** The authenticated account, or `anonymous` before authentication. */
+  owner: string
+  /** The API error code already returned to the caller. */
+  code: string
+  status: number
+  /** Route class, not the path: `memory` or `other`. Never a namespace. */
+  route: string
+}
+
+type Sink = ((line: Rejection) => void) | null
+let sink: Sink = null
+
+/** Tests capture lines instead of writing them. Passing null restores stderr. */
+export function setRejectionSink(next: Sink): void {
+  sink = next
+}
+
+/**
+ * An operational event that is not a request refusal: something the server did
+ * or failed to do on its own. Separate from Rejection because it carries a
+ * namespace slug, which a refusal line deliberately never does -- here the slug
+ * is the whole point, since an operator cannot act on "collection failed
+ * somewhere". A slug is a hash of a namespace, so it identifies a tenant to
+ * anyone who can already read the storage layout, and nothing more.
+ */
+export type OperationalEvent = {
+  timestamp: string
+  event: string
+  nsSlug: string
+  reason: string
+}
+
+type EventSink = ((line: OperationalEvent) => void) | null
+let eventSink: EventSink = null
+
+/** Tests capture events instead of writing them. Passing null restores stderr. */
+export function setEventSink(next: EventSink): void {
+  eventSink = next
+}
+
+export function logEvent(fields: Omit<OperationalEvent, 'timestamp'>): void {
+  const line: OperationalEvent = { timestamp: new Date().toISOString(), ...fields }
+  if (eventSink) {
+    eventSink(line)
+    return
+  }
+  process.stderr.write(`${JSON.stringify(line)}\n`)
+}
+
+export function logRejection(fields: Omit<Rejection, 'timestamp'>): void {
+  const line: Rejection = { timestamp: new Date().toISOString(), ...fields }
+  if (sink) {
+    sink(line)
+    return
+  }
+  process.stderr.write(`${JSON.stringify(line)}\n`)
+}