# Per-page PIN locks — implementation plan

Migrating agent-pages from *one instance-wide index secret* to *per-page 4-digit PINs*, with a public page index.

Target branch: `feat/page-pin-locks`, cut from `master` at `7ad6b01` (PR #15 merged).

---

## 1. What changes, and the assumption this rests on

**Today.** Every page is `unlisted`; the link is the credential. The index at `/` is gated behind `AGENT_PAGES_INDEX_TOKEN` (256-bit, HMAC-derived cookie) precisely *because* listing every page would hand out every secret link.

**After.** A page is open by default. At publish time the agent may attach a 4-digit PIN, which is then required to view that page. Locked pages still appear in the ledger with a lock marker — title and slug visible, content gated. The index itself becomes publicly viewable, so `AGENT_PAGES_INDEX_TOKEN` stops being the thing that protects content.

**The assumption:** the index becomes public and the index token gate is retired. This follows from "certain pages are locked and the rest are open to view" plus "migrating from the current long HMAC secret". Everything in §3 and §11 depends on it. If the intent is instead *keep the gate and add PINs as a second layer for links you hand out*, then §11 is dropped and the rest stands unchanged — say so before implementation starts, because it is a one-line config difference now and an awkward reversal later.

**Non-negotiable consequence to decide consciously:** every page published so far exists under a link-is-secret promise. Making the index public lists all of them. §11 handles this with an opt-in switch rather than a flag day, so the exposure is an explicit operator action, not a side effect of deploying.

---

## 2. Threat model — why the rate limiter *is* the security boundary

This is the single most important framing in the document, and it drives most decisions below.

| Credential | Entropy | What protects it |
|---|---|---|
| `AGENT_PAGES_INDEX_TOKEN` | 256 bits | Entropy. The limiter was about noise and table growth. |
| 4-digit PIN | ~13.3 bits (10,000 values) | **The limiter, and nothing else.** |

Time to sweep the full 4-digit keyspace at various ceilings:

| Limit | Full sweep | Expected hit (~50%) |
|---|---|---|
| 1000 / 15 min (the index's `_MAX_SHARED` escape hatch) | 2.5 h | ~1.25 h |
| 100 / 15 min | 25 h | ~12 h |
| 20 / 15 min | 5.2 days | ~2.6 days |
| **5 / 15 min (chosen, per page)** | **20.8 days** | **~10 days** |

Four consequences:

1. **`index_signin_rate_limit_max_shared = 1000` must never apply to PIN attempts.** It is correct for a 256-bit token and catastrophic for a 4-digit one. The PIN limiter gets its own settings and, unlike the index, **fails closed** when it cannot identify a client.
2. **A slow hash buys minutes, not security.** 10,000 candidates × ~50 ms scrypt ≈ 8 minutes of offline work. Worth doing (it is free, and it bounds damage from a DB leak), but it is not the defence.
3. **A public index is a target list.** The lock marker tells an attacker exactly which pages are worth attacking. That is inherent to the design, not a bug — it just removes "they don't know it exists" from the protection stack.
4. **Per-IP limiting alone is insufficient**, because IP is attacker-controlled to a degree (rotation, botnet) and, when `TRUSTED_PROXY_CIDRS` is misconfigured, unavailable entirely. Hence the two-layer design in §6.

### PIN length

4 digits is the specification and the plan implements it. It is a UX choice (memorable, phone-keypad-friendly) and it is defensible **only because** the per-page counter in §6 is mandatory. `PIN_LENGTH` is exposed as config (default `4`, validated 4–8) so an operator can trade typing effort for entropy without a code change:

| Length | Keyspace | Sweep at 5 / 15 min |
|---|---|---|
| 4 | 10⁴ | 21 days |
| 6 | 10⁶ | 5.7 years |
| 8 | 10⁸ | 570 years |

At 6+ digits the limiter stops being load-bearing and the design becomes robust to limiter misconfiguration. Recommended for anything genuinely sensitive; not the default, because the ask was 4.

---

## 3. Data model and migration

### Columns on `pages`

```python
# app/models.py — class Page
pin_hash: Mapped[str | None] = mapped_column(String(255), nullable=True, default=None)
pin_set_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True)
```

`pin_hash IS NULL` means open. No backfill: every existing page is already open under this rule, which is exactly the desired "all pages available right now are public" outcome.

**PIN is page-level, not version-level.** If it lived on `page_versions`, the version menu becomes a bypass — v1 open, v2 locked, and `/p/{slug}/v/1` serves the content anyway. Page-level puts it alongside `slug` and `format`, which are already fixed-per-page properties, and means locking a page locks its whole history.

`pin_set_at` exists so the ledger can show *when* a page was locked and so a future audit can find PINs set before a params change.

### Migration mechanics

`create_all()` does not add columns to existing tables, and there is no Alembic (AGENTS.md lists it under v1.1). Follow the precedent PR #15 set in `app/cli.py` with `CREATE INDEX IF NOT EXISTS` — but SQLite has no `ADD COLUMN IF NOT EXISTS`, so the check is explicit:

```python
# app/cli.py — init_db(), after Base.metadata.create_all()
with engine.begin() as conn:
    existing = {row[1] for row in conn.execute(text("PRAGMA table_info(pages)"))}
    for column, ddl in (
        ("pin_hash", "ALTER TABLE pages ADD COLUMN pin_hash VARCHAR(255)"),
        ("pin_set_at", "ALTER TABLE pages ADD COLUMN pin_set_at DATETIME"),
    ):
        if column not in existing:
            conn.execute(text(ddl))
```

`init-db` runs on every container start (`scripts/docker-entrypoint.sh`), so the deploy migrates the existing volume with no manual step. Both columns are nullable with no default, so `ADD COLUMN` is instant regardless of table size.

**This is the second hand-rolled migration.** A third should be the trigger to introduce Alembic rather than growing this block further — note it in `MEMORY.md` v1.1.

---

## 4. PIN storage

Format, stored in `pin_hash` as one self-describing string:

```
scrypt$16384$8$1$<b64-salt>$<b64-derived-key>
```

```python
# app/pins.py (new)
_N, _R, _P, _DKLEN, _SALT_BYTES = 2**14, 8, 1, 32, 16

def hash_pin(pin: str) -> str:
    salt = secrets.token_bytes(_SALT_BYTES)
    dk = hashlib.scrypt(pin.encode(), salt=salt, n=_N, r=_R, p=_P, dklen=_DKLEN)
    return f"scrypt${_N}${_R}${_P}${b64(salt)}${b64(dk)}"

def verify_pin(pin: str, stored: str) -> bool:
    algo, n, r, p, salt, expected = stored.split("$")
    if algo != "scrypt":
        return False
    dk = hashlib.scrypt(pin.encode(), salt=b64d(salt), n=int(n), r=int(r),
                        p=int(p), dklen=len(b64d(expected)))
    return hmac.compare_digest(dk, b64d(expected))
```

**Why these choices:**

- **`hashlib.scrypt`, not passlib/argon2** — stdlib, so no new dependency. AGENTS.md keeps the dependency set deliberately small, and a new transitive tree for a 13-bit secret is a poor trade.
- **`n=2**14` (16 MiB, ~50 ms)** — the memory cost is per in-flight attempt, and §6 caps concurrent attempts hard, so 16 MiB cannot be turned into a memory-exhaustion vector. Higher `n` would buy proportionally more offline time but starts to matter on a small VPS.
- **Per-page random salt** — stops one precomputed table covering every page on the instance. With only 10,000 candidates a per-page table is still cheap to build, which is the point made in §2.2: this limits blast radius, it does not prevent cracking.
- **Params in the string** — lets `verify_pin` keep validating old hashes if `_N` changes later, with rehash-on-successful-unlock as the upgrade path.
- **`compare_digest`** — the comparison is on derived keys, so timing leakage would be low-value, but it costs nothing.

Validation, in one place (`app/schemas.py`): `^\d{4}$` per `PIN_LENGTH`, rejecting anything non-numeric. Do **not** reject "weak" PINs like `1234` or `0000`: with a 10,000-value space, blocklisting a handful of values shifts nothing measurable and it hands an attacker a free keyspace reduction. Note this reasoning in the code so nobody "improves" it later.

---

## 5. Read paths — what must be gated

Four routes serve page bodies. **All four**, and the check must happen *before* the R2 fetch in `load_page_view` so a locked page costs no bandwidth:

| Route | `app/viewer/routes.py` | Note |
|---|---|---|
| `GET /p/{slug}` | :333 | Markdown host fetches the body; HTML host does not |
| `GET /p/{slug}/v/{version}` | :304 | Historical — same PIN, since the PIN is page-level |
| `GET /p/{slug}/raw` | :318 | The iframe target *and* a directly shareable URL |
| `GET /p/{slug}/v/{version}/raw` | :282 | Easiest one to forget |

Implementation: a single helper in `app/viewer/routes.py` used by all four, placed after the slug lookup and before content loading. Because `load_page_view` currently does lookup and R2 fetch in one call, add `require_unlocked: bool` to it, or split the slug lookup out — the former is a smaller diff and keeps the "never fetch a body you won't serve" property in one place.

On a locked page with no valid unlock cookie:

- Non-`/raw` routes → **303** to `/p/{slug}/unlock?next=<original path>`.
- `/raw` routes → **403** rendered with `error_raw.html` and `_RAW_HTML_CSP`. **Not** a redirect: `/raw` is loaded inside `sandbox=""`, where a navigation to the unlock form would render a form the sandbox forbids submitting. A 403 inside the frame is honest, and the host shell handles the unlock (see §7).

`GET /v1/pages/{id}` and `GET /v1/pages` are publish-token-authed and stay open, gaining `has_pin: bool`. They must never return `pin_hash`.

---

## 6. Rate limiting — two layers, different jobs

The layers exist for different reasons and neither substitutes for the other.

### Layer 1 — per page (the security bound)

Key: `sha256("pin-unlock-page:" + str(page_id))`, in the existing `publish_rate_limits` table via `consume_publish_rate_limit`. **5 attempts / 15 min**, settings `PIN_UNLOCK_RATE_LIMIT_MAX` / `_WINDOW_SECONDS`.

- **Unspoofable.** Keyed on the page, so no client-controlled input feeds it. This is what still holds when `TRUSTED_PROXY_CIDRS` is wrong, which — per the merged PR — is a realistic state.
- **Reuses the atomic conditional `UPDATE`** rather than adding a second locking pattern (AGENTS.md "do not regress" #4).
- **Accepted trade-off:** an attacker can keep one page unavailable at 5 attempts/15 min. Blast radius is that page only, and Layer 2 means a single IP burns its own budget first, so sustaining it needs a botnet. Documented, tunable, not solved — solving it would require letting a correct PIN bypass the limit, which means evaluating every guess, which removes the limit.

### Layer 2 — per client IP (fairness)

Key: `sha256("pin-unlock-ip:" + client_ip)`. **10 attempts / 15 min**, reusing `resolve_client_ip()` from `app/viewer/session.py`.

**Critical difference from index sign-in:** when `resolve_client_ip` returns `shared_proxy_bucket=True` (trusted proxy, no usable XFF), the PIN limiter must apply a **tighter** ceiling, not a looser one. `index_signin_rate_limit_max_shared = 1000` exists because a 256-bit token cannot be guessed; reusing it here would hand an attacker the whole keyspace in ~2.5 hours (§2). Add:

```python
pin_unlock_rate_limit_max_shared: int = Field(default=5, alias="PIN_UNLOCK_RATE_LIMIT_MAX_SHARED")
```

Fail closed. Put a comment at both settings explaining why one number is 1000 and the other is 5, because they look inconsistent until you know the entropy difference.

### Order of checks

1. Layer 2 (per IP) — cheap, and stops one host consuming the page budget.
2. Layer 1 (per page) — the hard bound.
3. `verify_pin` — the expensive scrypt call, reached only if both budgets allow.

Consume both budgets *before* hashing so scrypt cannot be used as a CPU/memory amplifier.

### `TRUSTED_PROXY_CIDRS` is now genuinely required

Under the index gate this setting was about availability. Now it is part of the security posture, and a missing value silently degrades Layer 2. Add the startup warning that PR #15 review suggested as a follow-up:

> `PUBLIC_BASE_URL` is https (so TLS terminates upstream, i.e. there is a proxy) but `trusted_proxy_cidrs` is empty → log a `WARNING` at import naming the risk.

That is inferable, costs one `if`, and closes the silent-misconfiguration hole for both features.

---

## 7. The unlock flow

### Routes (new, in `app/viewer/unlock.py`)

| Route | Behaviour |
|---|---|
| `GET /p/{slug}/unlock` | PIN form. Open page → 303 to `/p/{slug}`. Unknown slug → normal styled 404. |
| `POST /p/{slug}/unlock` | Correct → set cookie, 303 to validated `next` (default `/p/{slug}`). Wrong → re-render, **401**. Over limit → **429**. |

The form shows the page **title** — it is already public in the ledger, so hiding it buys nothing and costs orientation. It shows no content, no version list, and no PIN hint.

### CSP

The unlock page needs `form-action 'self'`, which today only `/` has. Add `_VIEWER_CSP_UNLOCK` to `app/viewer/shell.py` — a copy of `_VIEWER_CSP` with `form-action 'self'` — and extend `_viewer_headers()` with `allow_forms` reuse or a new flag.

**The artifact and `/raw` CSPs are not touched.** `_RAW_HTML_CSP` keeps `form-action 'none'`, `script-src 'none'`, `font-src 'none'`. AGENTS.md "do not regress" #2 and #9 both apply; add a test asserting the unlock CSP appears on `/p/{slug}/unlock` and nowhere under `/raw`.

### Unlock cookie

```
name  : agent_pages_unlock
value : {expires_at}.{HMAC(pin_hash, b"agent-pages-unlock-v1|" + slug + "|" + expires_at)}
path  : /p/{slug}
flags : HttpOnly; SameSite=Lax; Secure (when PUBLIC_BASE_URL is https); Max-Age=<PIN_UNLOCK_TTL>
```

Reuses the v2 construction from `app/viewer/session.py` — signed absolute expiry so `Max-Age` is not the only clock — with two deliberate differences:

- **HMAC key is the page's own `pin_hash`.** No new server-wide secret in the environment, and changing or removing a PIN invalidates every outstanding unlock for that page for free (the same rotation property the index token has).
- **Slug is bound into the MAC** as defence in depth, so a cookie cannot be replayed against another page even if path scoping is bypassed.

`PIN_UNLOCK_TTL` default **7 days**, shorter than the index session's 30: the credential behind it is 13 bits, so a stolen cookie should not outlive its usefulness for long. No sliding renewal.

**Cookie path subtlety worth a test.** Slugs are `^[a-z0-9]+(?:-[a-z0-9]+)*-[a-z0-9]{8}$`, so `foo-a1b2c3d4` is a legal prefix of the legal slug `foo-a1b2c3d4-e5f6g7h8`. RFC 6265 path matching only treats a prefix as matching when the next character is `/`, so a cookie at `/p/foo-a1b2c3d4` is *not* sent to `/p/foo-a1b2c3d4-e5f6g7h8`. That is the behaviour we want, and it depends on a spec detail rather than on our code — so assert it, and do not "fix" it by appending a trailing slash (which would stop the cookie reaching `/p/{slug}` itself).

### ⚠ Verify before building on it: cookies in the sandboxed iframe

The HTML viewer loads `/p/{slug}/raw` inside `<iframe sandbox="">`, which gives the frame an opaque origin. The unlock cookie must still reach that subresource request or **locked HTML pages render as a blank frame while Markdown works fine** — a failure that no server-side test will catch.

Expectation: the request is same-site, so a `SameSite=Lax` path-scoped cookie *is* sent, and `sandbox` affects the resulting document's origin rather than the request's cookie attachment. **Confirm in a real browser (Chrome + Firefox) as step 1 of implementation**, before the rest of the HTML path is written.

Fallback if it does not hold: the host shell, which has already verified the unlock, mints a single-use short-TTL token and appends it to the iframe `src` (`/raw?u=<token>`), with `/raw` accepting either the cookie or the token. Uglier, and it puts a credential in a URL that could be copied out — so only if measurement forces it.

---

## 8. API and MCP contract

### `POST /v1/pages`

```jsonc
{ "title": "...", "format": "markdown", "content": "...", "pin": "4821" }  // pin optional
```

Response gains `has_pin: bool`. The PIN is **never** echoed back — the caller already knows it, and echoing puts it in logs and agent transcripts twice over.

### `PUT /v1/pages/{id}`

Three-state semantics, because "absent" and "null" must not mean the same thing:

| `pin` field | Effect |
|---|---|
| absent | Unchanged |
| `null` | PIN removed — page becomes open |
| `"4821"` | PIN set or replaced |

Pydantic cannot distinguish absent from `null` on a plain `str | None`, so use a sentinel default and `model_fields_set`, and test all three explicitly. Getting this wrong silently unlocks pages on every content update — it is the highest-risk line in the change.

Changing or removing a PIN invalidates outstanding unlock cookies automatically (§7), which is correct: revoking a PIN should end sessions derived from it.

### MCP (`mcp_server/server.py`)

`publish_page` and `update_page` gain an optional `pin` parameter; `get_page` / `list_pages` surface `has_pin`. The MCP layer stays a thin HTTP client (AGENTS.md "do not regress" #1) — validation lives in `/v1`, not here.

**Operational note for the tool docstrings:** the agent chooses or relays the PIN, so it will appear in the chat transcript. That is inherent to an agent-set credential and worth stating in the tool description so the human is not surprised.

---

## 9. Ledger and index changes

- `PageSummary` gains `has_pin: bool` (`app/schemas.py`), populated in `_page_summary`.
- `_ledger.html` gains a lock marker on locked rows. Follow the existing sigil convention — mono glyph plus a real text label, not an icon alone — and give it an accessible name (`<span class="sr-only">Locked</span>`), since a glyph with `aria-hidden` would make the state invisible to screen readers. Keep it in the format column so the row still has exactly one colour-carrying cell.
- Titles and slugs of locked pages are public **by design** under a public index. Document it: the PIN gates *content*, not the fact that a page exists or what it is called. If a title is itself sensitive, the page should not be published to a public index.
- `X-Robots-Tag: noindex, nofollow, noarchive` stays on every viewer response. Public ≠ crawlable, and there is no reason to change that here.

---

## 10. Test plan

New: `tests/test_pin_publish.py`, `tests/test_pin_unlock.py`, `tests/test_pin_rate_limit.py`. Extend `tests/test_home_index.py` and `tests/test_viewer_security.py`.

**Gating** — for each of the four read paths in §5: locked + no cookie → 303/403; locked + valid cookie → 200; open page → unaffected. Assert R2 was not touched on a gated request (the fake R2 in `tests/conftest.py` records access, so assert on `fake_r2.objects` reads).

**Cookie** — value is not the PIN and not `pin_hash`; forged and truncated values rejected; expired `expires_at` rejected; cookie from page A rejected on page B; changing the PIN invalidates the old cookie; removing the PIN makes the page open.

**Rate limiting** — per-page budget exhausts at 5 and blocks the *correct* PIN too (assert this explicitly: it is intended, unlike the index case); per-IP budget exhausts independently; `shared_proxy_bucket=True` applies the **tighter** ceiling; scrypt is not called once either budget is spent (patch `verify_pin` and assert call count — this is the amplification guard).

**Update semantics** — the three `pin` states from §8, including that a content-only `PUT` leaves the PIN intact.

**CSP** — unlock page carries `form-action 'self'`; `/raw` and artifact responses still carry `form-action 'none'`, `script-src 'none'`, `font-src 'none'`.

**Migration** — build a DB without the new columns, run `init_db()`, assert both columns exist and existing rows read as open; run `init_db()` twice and assert idempotence.

**Slug prefix** — the RFC 6265 case from §7, with two slugs where one is a path prefix of the other.

**Manual, not automatable** — the sandboxed-iframe cookie check (§7), locked HTML *and* Markdown, in Chrome and Firefox, light and dark.

---

## 11. Retiring the index token — opt-in, not a flag day

Making the index public in the same deploy that adds PINs would publish every existing page's link before anyone had a chance to lock anything. Instead, ship the capability and let the operator flip the switch:

```python
index_visibility: Literal["token", "public"] = Field(default="token", alias="AGENT_PAGES_INDEX_VISIBILITY")
```

- **`token` (default, no behaviour change on deploy)** — `/` behaves exactly as it does today. PINs work. The operator locks what needs locking at their own pace.
- **`public`** — `/` serves the ledger to everyone; the sign-in card, `POST /session`, and the sign-out control disappear. `AGENT_PAGES_INDEX_TOKEN` becomes unused.

Deploying is therefore inert, and the exposure is a deliberate one-line change the operator makes when ready. Keep `token` as the default in this PR; a later PR can flip the default or delete the token path once the model has proven itself.

**Pre-flight helper** — `agent-pages index-audit` prints how many pages exist, how many are PIN-locked, and the titles/slugs of those that would become publicly listed. One small CLI command that turns an irreversible decision into an informed one.

Keep `app/viewer/session.py` intact under `public` mode — it is ~200 lines of reviewed, tested code, and deleting it forecloses going back.

---

## 12. Docs to update

- **`AGENTS.md`** — locked-decision rows (Privacy, Index, Index auth), the viewer route table (`/p/{slug}/unlock`), a PIN section covering entropy and the two-layer limiter, and new "do not regress" entries: *(a)* PIN gating covers all four read paths before the R2 fetch; *(b)* the PIN limiter fails closed and never reuses `index_signin_rate_limit_max_shared`; *(c)* PIN is page-level so version history cannot bypass it.
- **`MEMORY.md`** — decisions with reasoning: page-level PIN, scrypt-in-stdlib, two-layer limiter, opt-in visibility switch, 7-day unlock TTL, no weak-PIN blocklist. Note the third hand-rolled migration as the Alembic trigger.
- **`docs/architecture.md`** — trust-boundary diagram: the PIN check sits between the viewer route and R2.
- **`README.md`** / **`.env.example`** — the new settings, the `PIN_LENGTH` trade-off table from §2, and `TRUSTED_PROXY_CIDRS` restated as security-relevant rather than availability-only.

---

## 13. Sequencing

Roughly 700–1000 lines with tests. Order matters — step 1 can invalidate §7:

1. **Browser-verify the sandboxed-iframe cookie question** (§7). Do this first; the fallback design changes if it fails.
2. Model + migration + `app/pins.py` + tests (§3, §4).
3. `/v1` and schema changes, including the three-state `PUT` (§8).
4. Unlock routes, template, CSP, cookie (§7).
5. Gate the four read paths (§5).
6. Two-layer limiter + the startup warning (§6).
7. Ledger lock marker, `has_pin` plumbing (§9).
8. `AGENT_PAGES_INDEX_VISIBILITY` + `index-audit` (§11).
9. MCP parameters (§8).
10. Docs (§12).

Steps 2–7 are the reviewable core; 8–10 are additive and could be split into a second PR if the diff gets uncomfortable.

## 14. Explicitly out of scope

Per-version PINs (§3 — creates a bypass); multiple or named PINs per page; PIN recovery beyond `PUT`; escalating/exponential lockout (fixed window first, measure before adding a second locking pattern); owner notification on lockout; weak-PIN blocklists (§4); making the public index crawlable (§9); replacing the two hand-rolled migrations with Alembic (§3 — but this is the trigger to plan it).