Planning: record M2 closeout decisions
M2's review reported six planning recommendations rather than applying them, three marked before M3. All six are applied here, plus three additions drawn from the same evidence. No implementation file is touched. The endpoint policy was the gap that mattered. It is the most consequential setting in the application — the storyteller sends the player's prose, the context, the memories and the embedding inputs to whatever address it names — and it existed only as a module docstring. It is now ADR 011 and a new §10A in the threat model, which also retires the assumption in §71A that the inherited guard was a starting point. It was not: AI-DnD's SSRF guard blocked private addresses to stop a hosted server reaching its own internal network, which is the exact opposite of what a local storyteller needs. It was removed, not adapted. Both documents state the rule as implemented — an allowlist of explicit local-network CIDRs, every resolved address checked, enforced on save and again before every outbound request, TLS never traded against it — and both state the two residual limits plainly rather than implying they are covered: a hostile host already on the trusted LAN is inside the permitted boundary, and a rebinding interval exists between the policy's resolution and the client's connection. Accepted risks, not M3 work. The CIDRs are spelled out rather than derived from is_private/is_reserved, and the ADR records why: is_private is true of the documentation ranges and 0.0.0.0/8, and is_reserved is true of IPv6 loopback, so a rule built on it refuses an ordinary same-host Ollama on [::1]. TECHNICAL-DESIGN §5.1 items 3 and 4 are marked done, closing all five hardening items. A new §5.2 records the M1/M2 architecture as fact rather than intention, so later milestones inherit what the code does. A new §18.1 carries the lesson of M2's two regressions: when removing a setting, test a real consumer construction path; when adding one, prove it reaches the component that uses it. Both defects hid behind a green suite because the tests at that boundary were mocks. BUILD-MILESTONES records M2 complete, with the capabilities later milestones inherit and the debt carried forward. Two notes go to milestones that would otherwise misread what M2 left them. M5 is told that eight rollback tests now use the world-state engine as instrumentation and not as endorsement — the instrumentation moves when the protocol does, and those tests are reworked rather than deleted. M6 is told that the memory bank died silently under a green suite, so background failure must be observable and at least one real provider-construction path must be tested. The security contract gains what M2 demonstrated. H10 now names the two conditions that were defects during M2: a wildcard origin must be refused at startup, and an unknown /api path must 404 rather than returning the SPA with 200. New H12 covers endpoint enforcement, and its fourth pass condition is the one that matters — a public endpoint written into the database behind the settings API must still be refused at the wire. A build passing the first three and failing that one has configuration validation only. SPECIFICATION.md is deliberately unchanged. M2 altered no product requirement; it removed capability the specification never asked for. The two M2 reports gain appended closeout notes rather than edits. Their original wording about an uncommitted working tree was true when written, and the note records what happened afterwards: the six-file correction is8652fe7,8c65ae9remains the implementation commit, and the two were never squashed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HsZBU8sWRuYTyLgWsu2oQ6
This commit is contained in:
co-authored by
Claude Opus 5
parent
8652fe7cd8
commit
2fdd2547f0
@@ -201,8 +201,8 @@ Production defaults must not require:
|
||||
|
||||
### 5.1 Known AI-DnD hardening work
|
||||
|
||||
Phase 0B identified concrete inherited violations, and M1 added a fifth. Items
|
||||
1, 2 and 5 are **resolved**; items 3 and 4 remain open and belong to M2.
|
||||
Phase 0B identified concrete inherited violations, and M1 added a fifth. **All
|
||||
five are now resolved** — items 1, 2 and 5 in M1, items 3 and 4 in M2.
|
||||
|
||||
1. ~~`tiktoken` attempts to download the `cl100k_base` encoding on first use.~~
|
||||
**Done in M1.** The encoding table is vendored in the tree and loaded
|
||||
@@ -211,21 +211,48 @@ Phase 0B identified concrete inherited violations, and M1 added a fifth. Items
|
||||
2. ~~the SPA requests Google Fonts at runtime.~~
|
||||
**Done in M1.** All three families are self-hosted, and the CSP names no
|
||||
remote origin at all.
|
||||
3. hosted/multi-user/auth/demo/analytics/Postgres/cloud-provider/QuickJS paths are unnecessary.
|
||||
3. ~~hosted/multi-user/auth/demo/analytics/Postgres/cloud-provider/QuickJS paths are unnecessary.~~
|
||||
- remove them rather than merely hide them where practical.
|
||||
- **Open — M2.** M1 removed nothing, so this surface is unchanged from the
|
||||
fork point.
|
||||
4. endpoint validation must reflect this product's threat model.
|
||||
- **Done in M2, in full.** Removed rather than hidden: 52 API routes fell to
|
||||
36, and `/api/auth`, `/api/analytics` and `/api/scripts` are gone entirely
|
||||
rather than gated. See §5.2.
|
||||
4. ~~endpoint validation must reflect this product's threat model.~~
|
||||
- same-host loopback Ollama is the default; an explicitly configured trusted-LAN Ollama endpoint is supported; arbitrary public/Internet model endpoints must be rejected or kept outside normal v1 configuration.
|
||||
- inference endpoint configuration must not change the storyteller's own loopback bind behavior.
|
||||
- **Open — M2.** The trusted-LAN path itself works as of M1; what remains is
|
||||
deciding and enforcing which endpoints normal v1 configuration may name.
|
||||
- **Done in M2.** `backend/app/endpoints.py` applies an address-based
|
||||
allowlist on save and again before every outbound request. Endpoint
|
||||
configuration has no influence on the storyteller's own bind address.
|
||||
ADR 011; `SECURITY-THREAT-MODEL.md` §10A.
|
||||
5. ~~outbound TLS verified only against a bundled public-CA list, so a LAN host
|
||||
with a privately issued certificate was refused.~~
|
||||
**Found and fixed in M1.** Not visible to Phase 0B: every run up to that
|
||||
point used plain HTTP over loopback, where certificate verification never
|
||||
happens. See *Transport to a trusted-LAN endpoint* above.
|
||||
|
||||
### 5.2 Production architecture as established by M1 and M2
|
||||
|
||||
The architecture below is no longer a selection; it is what the code does. It is
|
||||
recorded here so later milestones inherit facts rather than intentions.
|
||||
|
||||
| | |
|
||||
| --- | --- |
|
||||
| Production base | AI-DnD, forked at `d72f7c1` (§2, ADR 009) |
|
||||
| Persistence | SQLite. Postgres, Neon and the Render deployment path are removed |
|
||||
| Inference | Ollama only. No cloud provider code, no API key, no key UI |
|
||||
| Storyteller bind | loopback by default, in every run path including the published Docker port |
|
||||
| Ollama endpoint | same-host loopback by default; an explicitly configured trusted-LAN endpoint is equally supported |
|
||||
| Public endpoints | refused by address, on save and before every request |
|
||||
| Trusted-LAN HTTPS | supported, with full certificate and hostname verification against the machine's CA store; no bypass exists |
|
||||
| Runtime assets | self-contained. Tokenizer table and fonts are vendored; the CSP names no remote origin |
|
||||
|
||||
Removed in M2 rather than hidden: hosted accounts and auth, guest/demo
|
||||
behaviour, hosted analytics, cloud inference providers, API-key storage and its
|
||||
UI, Postgres/Neon/Render support, and QuickJS campaign scripting.
|
||||
|
||||
**A trusted-LAN Ollama endpoint is accepted production behaviour**, not a
|
||||
development convenience. Any statement that the only valid endpoint is literally
|
||||
`127.0.0.1` is stale and should be read against §5 and §10A of the threat model.
|
||||
|
||||
## 6. Browser UI Boundary
|
||||
|
||||
The browser remains a presentation/control layer, not the owner of story authority.
|
||||
@@ -606,6 +633,24 @@ Required categories:
|
||||
|
||||
Acceptance-test IDs in `V1-ACCEPTANCE-TESTS.md` are the black-box release contract.
|
||||
|
||||
### 18.1 Wiring rule, from the M2 regressions
|
||||
|
||||
M2 shipped two defects that a 604-test green suite did not see: a removed
|
||||
`Settings` attribute left two provider factories raising `AttributeError` inside
|
||||
a background task, and a newly added timeout setting was stored, validated,
|
||||
exposed and rendered without ever being passed to the provider that needed it.
|
||||
Both were invisible because the tests at that boundary were mocks.
|
||||
|
||||
> **When removing a setting, attribute or dependency, test at least one real
|
||||
> consumer construction path. When adding a setting, test that the configured
|
||||
> value reaches the component that uses it. A green suite built entirely around
|
||||
> mocks at that boundary is insufficient evidence.**
|
||||
|
||||
The corollary is where to look: subtractive changes and plumbing changes fail in
|
||||
background and fire-and-forget paths, which are exactly the paths that report
|
||||
nothing when they break.
|
||||
|
||||
|
||||
## 19. Removal / Migration Strategy From Upstream
|
||||
|
||||
Production migration should be incremental and test-gated rather than a broad rewrite.
|
||||
|
||||
Reference in New Issue
Block a user