fix(storage): route the last two id sanitisers through the single source + scope every record-id fetch to the brain

[RobertSigmundsson]

Hi Toni!

The two follow-ups you tracked on the id-sanitiser consolidation — thank you again for that review. Both are small and neither is reachable with today's inputs, but they each nick a guarantee, so here they are closed together.

1. Two sanitisers still bypassed the single choke point

_ids._to_surreal_id is meant to be the only place a caller-supplied id is folded to [A-Za-z0-9_]. Two copies still lived outside it:

• migrations._sanitize_id — a dash-only .replace("-", "_"). Replaced with _to_surreal_id. Besides restoring the single-source guarantee, this fixes a latent mismatch: neuron records are stored folded (_to_surreal_id), so a legacy v7 endpoint id containing anything other than - (e.g. a .) used to migrate to a neuron:{id} edge endpoint that did not match the stored neuron. For uuid4 / content-hash ids (the only ids in practice) the output is unchanged.
• tool_events.insert_tool_events — an inline event_id.replace("-", "_") on a server-generated uuid4. Routed through _to_surreal_id; output is identical for a uuid4, but the id no longer has a private folding path.

2. Record-id fetches were not brain-scoped

Every fetch-by-record-id did a bare select (neuron:{sid}, synapse:{sid}, …) with no WHERE brain_id, so a caller in brain A could read a record owned by brain B just by knowing (or guessing) its id — the same gap the store-layer guard closed for consolidation.py. You flagged get_neurons_batch / find_neurons_by_ids; the same class was open in get_neuron, get_neuron_state, get_synapse and get_fiber, so I closed the whole class here rather than leave a half-guarantee. All six now add WHERE brain_id = $brain_id (parameterised; brain_id from _get_brain_id() → _safe_brain_id, fail-closed). (device fetch was already brain-keyed in its record id.)

They stay direct record fetches — the ids are still pinned in FROM neuron:{sid}, …, so this is not a table scan and the 2.7.2 dashboard-perf property is preserved; the WHERE only rejects a cross-brain hit on records already pinned by id. The single-record getters also gain the reconnect-on-dropped-transport retry (they route through _query now instead of a bare select).

Type of Change

• Security hardening (defence-in-depth; non-breaking for legitimate callers)

Testing

• tests/unit/test_id_consolidation_and_brain_scope.py — brain-scope query shape + bound param for all six methods; fail-closed brain-id path; single-source routing for migrations (dotted-id fold) and tool_events.
• Updated test_dashboard_perf_queries.py::test_fetches_records_directly_with_omit to pin the scoped-but-still-direct fetch (was asserting WHERE not in sql).
• Full unit suite (rebased on main): 6,090 passed, 33 skipped in ~29 s (-n0), clean exit. ruff + mypy clean.
• Cross-brain isolation slice-tested against a live surrealdb:v3.2.0: for all six methods the owning brain reads the record and a foreign brain gets None.

Checklist

• Style (ruff/mypy). [x] Self-review. [x] Tests prove the fix.
• CHANGELOG.md — happy to add a Security/Fixed line if you'd like.

Callers audited (why the scope is a no-op for legitimate use)

All ids reaching these methods are derived from the current brain's own graph/fibers: get_neuron/get_neuron_state (anchor ids from this brain's fibers), the batch methods (retrieval_context, activation, consolidation, dashboard_api, compression, query_expansion), find_neurons_by_ids (the graph-view route). None cross a brain.

Robert

[RobertSigmundsson]

You flagged the two batch methods; the same bare-select read-by-id class was also open in get_neuron, get_neuron_state, get_synapse and get_fiber, so I scoped all of them here rather than leave a half-guarantee — a caller could otherwise still read a foreign brain's record via the single-record path. brain_id is a bound param from _get_brain_id() (→ _safe_brain_id, fail-closed), and the record stays pinned in FROM, so it's still a direct fetch, not a scan. Routing through _query also gives the single getters the reconnect-on-dropped-transport retry they lacked. (device was already brain-keyed in its record id.)

[RobertSigmundsson]

The ids in FROM already pass _to_surreal_id + an isalnum()/_ filter, so the WHERE only adds the brain check on records already pinned by id — no change to the 2.7.2 dashboard-perf profile (still a pinned-record fetch, not a FROM neuron scan).

[RobertSigmundsson]

Second follow-up: the old _sanitize_id was dash-only, so it both duplicated the fold and under-folded. _to_surreal_id here also quietly corrects endpoints — neuron records are stored folded, so a legacy source_id/target_id with a . (or any non-- char) previously produced a neuron:{id} endpoint that didn't match the stored record. uuid4 / hash ids are unaffected.

[RobertSigmundsson]

Updated this branch — same fix, extended to the concurrent batch paths that were added after this PR was opened.

Why it changed

The original commit (fc88973) was written against an older base and covered the single-record fetch paths. Since then v2.10.5 introduced concurrent batch fetches with exactly the same unscoped-select pattern, so the PR as it stood would have left the newer paths open. This branch is now rebased onto current main (9c9aba5) and extends the same fix to them.

Added on top of the original:

• get_neurons_batch — the bare conn.select inside _fetch_one becomes a scoped self._query. The brain id is resolved once, before the semaphore/gather, so a hostile or unset brain context fails closed before a single fetch goes out, rather than racing N of them.
• get_synapses_batch — new in v2.10.5, had the identical unscoped concurrent-fetch pattern and was absent from fc88973. Same adaptation.
• Tests assert brain-scope over every recorded query (conn.query.call_args_list, N calls), not just the last one — a single-call assertion passes vacuously on a concurrent path.

Unchanged from the original: single-sourcing migrations._sanitize_id and the inline .replace("-", "_") in tool_events through _ids._to_surreal_id, and the WHERE brain_id = $brain_id predicate on get_neuron / get_neuron_state / get_synapse / get_fiber / find_neurons_by_ids. These stay direct record fetches — the ids remain pinned in FROM, so this is not a table scan, and the dashboard-perf property is preserved.

Heads-up on the red Lint check

Lint is failing on this run, and it is not from this diff. The two errors are in files this PR does not touch:

RUF100  src/surreal_memory/mcp/version_check_handler.py:214  Unused `noqa` directive (unused: S310)
RUF036  tests/unit/test_version_check.py:87                  `None` not at the end of the type union

Reproduced against clean main with the same ruff the workflow installs:

$ git switch --detach origin/main
$ ruff check src/ tests/      # ruff 0.16.0
Found 2 errors.               # ← identical two, on unmodified main

$ ruff check <the 5 files in this PR>
All checks passed!

Root cause is that .github/workflows/… runs an unpinned pip install ruff, which now resolves to 0.16.0; RUF036 is newer than the code, and RUF100 fires because S310 is no longer raised at that site. That is why this PR's earlier run was green in July and this one is not — nothing in the diff changed, the linter did. Both are [*] fixable with --fix, but they belong in a separate housekeeping change rather than here, and pinning ruff in the workflow would stop this recurring on every open PR.

Everything else on this run is green: Test (3.11), Test (3.12), Type Check, Docs Freshness, Security Scan.

Local verification

Isolated worktree on origin/main (9c9aba5), Python 3.12.13:

Check	Result
ruff check src/ tests/	All checks passed!
ruff format --check src/ tests/	702 files already formatted
mypy src/ --ignore-missing-imports	Success: no issues found in 369 source files
pytest -m "not stress" -n auto	6547 passed, 84 skipped, 1 xfailed

Baseline on unmodified main is 6535 passed / 84 skipped / 1 xfailed → +12 tests, no regressions.