Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions docs/releases/in_progress/v0.18.3/github_release_supplement.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
Three connected fixes that make by-reference source storage reachable over the HTTP REST route and stop the Inspector graph viewer from polling in a loop. All three were reported by an external evaluator running Neotoma against a production database.

## Highlights

- **`source_storage: "reference"` now works over `POST /store`, not just MCP.** v0.18.1 added the field to the OpenAPI contract, but the REST handler still read every file into a buffer and stored it inline. `handleStorePost` now branches on `source_storage` and routes the file leg through `storeRawReference`, so a REST reference store copies zero bytes and writes `storage_mode=reference`, matching the MCP route. Fixed in `src/actions.ts`.
- **Combined "file + its entities" reference stores no longer 500.** A `POST /store` carrying `entities[]` and a `file_path` with `source_storage: "reference"` previously failed with `UNIQUE constraint failed: observations.id`. The structured leg's `createObservation` did a blind insert, where the MCP store path already guarded the content-addressed id with an existing-observation check. The REST path now has the same guard and returns the existing observation instead of colliding. Fixed in `src/services/observation_storage.ts`.
- **Inspector graph viewer no longer fires `retrieve_graph_neighborhood` in an unbounded loop.** The viewer mounted TanStack Query with a global `refetchInterval: 5000`, so every graph hook re-polled on a five-second timer (observed as 126+ calls in three minutes against an embedded graph). The three graph hooks now set `refetchInterval: false`, and the embed canvas fills the viewport instead of collapsing. Fixed in `inspector/src/hooks/use_graph.ts` and `inspector/src/pages/embed_graph.tsx`.

## What changed for npm package users

**Runtime / data layer**

- `POST /store` with `source_storage: "reference"` is now honored on both the file-only shape and the combined `entities[] + file_path` shape. No client changes are required; callers already sending `source_storage` over MCP get the same behavior over REST.
- `createObservation` is now idempotent on the REST structured path: re-storing content-addressed-identical observations returns the existing row instead of throwing on the `observations.id` UNIQUE constraint. The existence check is scoped to `(id, user_id)`, so it never reads or masks another user's observation. This matches the long-standing MCP store behavior.

**Inspector**

- The embedded graph (`/embed/graph`) renders once and stays put rather than re-fetching every five seconds. If you embed the Inspector graph, this removes the runaway request volume and the blank-canvas symptom.

**Shipped artifacts**

- `openapi.yaml` — unchanged; `source_storage` was already declared in v0.18.1. No new routes or fields in this patch.
- `dist/` — updated for the changed source files.

## API surface & contracts

No OpenAPI changes. `source_storage` is an existing `StoreRequest` field; this release makes the REST handler actually act on it. Request and response shapes are unchanged.

## Behavior changes

- REST `POST /store` reference stores now copy zero bytes and register the source by path, the same as MCP. Callers that relied on the prior silent inline fallback will now get a real reference source row.
- The REST structured store no longer returns a 500 on a duplicate-content observation; it replays the existing observation. This is a strict robustness improvement and matches MCP.
- The Inspector graph stops periodic background refetching. Open graphs no longer update on a timer; they refetch on navigation and explicit interaction.

## Fixes

- **#1826** — `source_storage: "reference"` was unreachable over HTTP `POST /store`. The OpenAPI contract accepted it (v0.18.1) but `handleStorePost` still stored inline. The REST handler now honors reference mode end to end.
- **#1827 (REST extension)** — the combined `entities[] + file + source_storage:reference` call returned a 500 on `observations.id` UNIQUE collision because the REST structured leg lacked the existing-observation guard that the MCP path has. Brought to parity.
- **#1837** — the Inspector graph viewer fired `retrieve_graph_neighborhood` in an unbounded loop and never painted. Root cause was a global `refetchInterval: 5000`, not unbounded traversal. Polling disabled on the graph hooks; canvas height fixed.

## Tests and validation

- `tests/integration/http_store_reference_source.test.ts` — exercises the real Express `/store` handler over HTTP. Asserts `storage_mode=reference` on both the file-only store and the combined `entities[] + file` store (the case that previously 500'd).
- `docs/testing/automated_test_catalog.md` — regenerated (517 total / 145 integration files).
- Security gates: G1 (`security:classify-diff`) flagged `src/actions.ts` as auth-adjacent; manual review confirmed no authz change (the only new query is `(id, user_id)`-scoped). G2 (`security:lint`) 0 errors. G3 (`security:manifest:check` 115 routes in sync, `test:security:auth-matrix` 18 passed / 1 skipped) passed.

## Security hardening

`npm run security:classify-diff -- --base v0.18.2 --head HEAD` reported `sensitive=true` solely because `src/actions.ts` matches the `auth-middleware` path heuristic (the store handler lives there). The change adds no authentication or authorization logic. The new observation existence check is scoped to `(id, user_id)`, mirroring the MCP store path, and cannot leak or mask another user's data.

Security review artifact: [docs/releases/in_progress/v0.18.3/security_review.md](security_review.md) — verdict: **yes** (no findings, no residual risks).

## Breaking changes

No breaking changes. No OpenAPI tightening, no request-shape changes, no field promotions or removals. Both behavior changes (reference stores honored, duplicate observations replayed) are strict improvements aligning REST with MCP. Patch bump is correct per SemVer.
62 changes: 62 additions & 0 deletions docs/releases/in_progress/v0.18.3/security_review.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# Security review — v0.18.3

Manual security review for the `/release` Security review lane. The diff classifier flagged this release as sensitive, so this review records the adversarial pass and the verdict. The supplement's `Security hardening` section links it.

## Scope

- Base ref: `v0.18.2`
- Head ref: `HEAD`
- Diff classifier: **sensitive** (`sensitive=true`) — sole reason: `src/actions.ts` matches the `auth-middleware` path heuristic. The store HTTP handler lives in this file; the diff adds no auth logic.
- Provider: manual review.
- Protected routes manifest: 115 routes, in sync with `openapi.yaml`.
- Changed files: 7 (2 source, 1 schema, 2 inspector, 1 test, 1 generated catalog).

## Adversarial review prompt

Treat the diff as if you were an attacker. For every concern below, propose at least one concrete request or code path that exercises the failure mode, then either confirm the gate would catch it or describe the missing test.

1. **Alternate-path auth.** Can an unauthenticated external caller reach a privileged path through an alternate channel?
2. **Proxy trust.** Does any new code trust `X-Forwarded-For`, `Forwarded`, `Host`, or `req.socket.remoteAddress` outside the canonical helpers?
3. **Local-dev widening.** Does any new path reference `LOCAL_DEV_USER_ID`, `assertExplicitlyTrusted`, `NEOTOMA_TRUST_PROD_LOOPBACK`, or a `dev`/`sandbox` shortcut?
4. **Unauth public route.** For every new Express route, confirm it is in `protected_routes_manifest.json` or the runtime allow-list.
5. **Guest-access policy widening.** Does the diff change `assertGuestWriteAllowed`, `routeAcceptsGuestPrincipal`, or any guest-token issuer?
6. **AAuth / agent identity downgrade.** Does the diff make it easier to satisfy auth without a verified `aa-agent+jwt`?
7. **Cross-user read via content-addressed id.** The new `createObservation` existence check looks up an observation by its content-addressed id. Can a caller use it to read or infer another user's observation?

## Findings

- **No new routes; manifest unchanged.** The diff registers no Express routes. `security:manifest:check` confirms the protected-routes manifest is in sync (115 routes). Concerns 1 and 4 do not apply.
- **No auth-logic change in `src/actions.ts`.** The change in `handleStorePost` branches on the existing `source_storage` request field to choose reference vs. inline file handling. It is reached only after `getAuthenticatedUserId(req, …)`, the same authentication boundary as before. No middleware, guest-policy, AAuth, or principal-resolution code is touched. Concerns 1, 5, and 6 do not apply.
- **No proxy-trust changes.** No new use of `X-Forwarded-For`, `Forwarded`, `Host`, or `req.socket.remoteAddress`. Concern 2 does not apply.
- **No local-dev widening.** No reference to `LOCAL_DEV_USER_ID`, `assertExplicitlyTrusted`, or `NEOTOMA_TRUST_PROD_LOOPBACK` in the changed files. Concern 3 does not apply.
- **Cross-user read is not possible (concern 7).** The new existence check in `createObservation` is `.eq("id", observationId).eq("user_id", params.user_id)`. It is scoped to the authenticated caller's `user_id`, so a same-content observation owned by another user is never returned. This is the same scoping the MCP store path already uses (`src/server.ts`), and its comment documents the intent: prevent a different user's same-content observation from masking the current user's write. The change brings the REST path to parity; it does not widen read scope. An attacker who guesses another user's content-addressed observation id still gets no row back, because the `user_id` filter excludes it.
- **Reference-mode file handling.** Reference mode resolves the supplied `file_path` to an absolute path and registers it via `storeRawReference`. Path resolution and the host-local reference model are unchanged from the MCP path shipped in #1775/#1830; this release only makes the REST route reach the same code. No new filesystem-traversal surface beyond what MCP reference storage already exposes.
- **G1 classify-diff:** `sensitive=true` (path heuristic on `src/actions.ts`; no auth change in fact).
- **G2 security:lint:** 0 errors, 125 warnings (all pre-existing; none in changed lines).
- **G3 security:manifest:check + test:security:auth-matrix:** both passed (115 routes in sync; 18 passed / 1 skipped).

## Suggested negative tests

- `tests/integration/http_store_reference_source.test.ts` covers the REST reference path for both shapes. The existing `tests/security/auth_topology_matrix.test.ts` continues to assert the store route's auth topology. No additional auth-adjacent negative test is required, because no auth path changed and the new query is user-scoped.

## Residual risks

- None. The diff makes an existing, already-authenticated REST route honor an existing request field, and adds a user-scoped idempotency guard mirroring the MCP path. No auth surface is widened.

## Sign-off

| Reviewer | Verdict | Date |
|----------|---------|------|
| ateles-agent (manual) | yes | 2026-06-30 |

Verdict `yes` — classifier-sensitive by file-path heuristic only; manual review confirms no authorization change and no cross-user read. No block.

## Diff appendix

- `src/actions.ts`
- `src/services/observation_storage.ts`
- `src/shared/action_schemas.ts`
- `inspector/src/hooks/use_graph.ts`
- `inspector/src/pages/embed_graph.tsx`
- `tests/integration/http_store_reference_source.test.ts`
- `docs/testing/automated_test_catalog.md`
43 changes: 43 additions & 0 deletions docs/releases/in_progress/v0.18.3/test_coverage_review.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# Test coverage review — v0.18.3

## Scope

Release diff: `v0.18.2..HEAD` (7 files: 2 source, 1 schema, 2 inspector, 1 integration test, 1 generated catalog). Three connected bug fixes (#1826, #1827 REST extension, #1837).

## Code review

Ran the release diff review against `v0.18.2..HEAD`. The store-side changes route an existing request field to existing reference-storage code and add a user-scoped idempotency guard that mirrors the MCP path. The inspector changes disable a polling timer and fix a CSS height. No new CLI commands, no new routes, no new HTTP/MCP contract surfaces, no migrations, no external file parsers.

Verdict: **ADVISORY only** — no BLOCKING findings. The store fixes carry an HTTP integration test that exercises the real Express handler for both call shapes, including the combined shape that previously 500'd.

## Surface coverage

### `src/actions.ts` + `src/shared/action_schemas.ts` — REST `/store` reference mode

- **Change:** `handleStorePost` branches on `source_storage` to skip the inline `readFileSync` and route the file leg through `storeRawReference`; `StoreRequestSchema` already declared the field.
- **Classification:** Covers user-observable behavior end-to-end (HTTP request → sources row).
- **Test coverage:** `tests/integration/http_store_reference_source.test.ts` boots the real Express `app` and issues live `POST /store` requests. Asserts `storage_mode=reference` and a non-null `reference_path`/`content_hash` on the sources row for the file-only shape, and `body.unstructured.storage_mode === "reference"` for the combined `entities[] + file` shape. Both pass.

### `src/services/observation_storage.ts` — REST structured-store idempotency

- **Change:** `createObservation` looks up the content-addressed observation id scoped to `(id, user_id)` and returns the existing row instead of colliding on the `observations.id` UNIQUE constraint.
- **Classification:** Covers user-observable behavior (combined reference store no longer 500s).
- **Test coverage:** The combined-shape case in `http_store_reference_source.test.ts` is the direct regression: before the fix it returned 500, after it returns 200 with the reference source row. Verified by stashing the fix and observing the 500 reproduce, then confirming green with the fix. Idempotency/replay semantics confirmed unregressed by running `tests/integration/idempotency_collision.test.ts` (3 pass) and `tests/integration/idempotency_key_content_mismatch.test.ts` (5 pass) per-process.

### `inspector/src/hooks/use_graph.ts` + `inspector/src/pages/embed_graph.tsx` — graph polling + layout

- **Change:** `refetchInterval: false` on `useGraphNeighborhood`, `useGraphNeighborhoodWithBase`, `useRelatedEntities`; embed root and canvas height changed to fill the viewport.
- **Classification:** UI behavior (stops timer-driven refetch; renders the canvas).
- **Test coverage:** No automated test (the loop was a runtime polling timer + CSS height, not a logic branch reachable by a unit assertion). Verified by source inspection: the three hooks now disable the interval, and the canvas uses `min-h-[calc(100dvh-5rem)]`. Manual/embedded verification belongs to the reporter's environment, which is why the supplement asks for confirmation on the next pull.

## Surfaces that do NOT apply to this release

- Destructive / data-mutating operations: none.
- External file-shape parsers: none (reference storage reuses the existing path resolver).
- New CLI commands or flags: none.
- New HTTP/MCP routes: none.
- Migrations: none.

## Verdict

No BLOCKING coverage gaps. The two store fixes have end-to-end HTTP integration coverage verified by inspection of the test bodies and by reproducing the pre-fix 500. The inspector change is a polling/layout fix with no logic branch to assert and is verified by source inspection. Release may proceed.
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "neotoma",
"version": "0.18.2",
"version": "0.18.3",
"description": "MCP server for structured personal data memory with unified source ingestion",
"main": "dist/index.js",
"type": "module",
Expand Down
Loading