Skip to content

Commit 59ed2e9

Browse files
os-zhuangclaude
andauthored
feat(service-settings): report-only sys_secret orphan classifier, and the reachability fact it falsifies (#8103) (#8261)
* feat(service-settings): report-only sys_secret orphan classifier (#8103) Lands the non-destructive half of #8103: an operator-facing answer to "which sys_secret rows are orphaned, and why", with no deletion and no migration. The sweep's vehicle stays an open maintainer decision. Re-measuring #8063's three reachability facts falsified the middle one: sys_setting.value_enc is NOT the only column holding a sys_secret handle. The store has three producers (SettingsService, the engine's secret-field channel storing `secret:<id>` on arbitrary business rows, and the datasource credential binder storing `sys_secret:<id>` at external.credentialsRef), and the engine's holders are not statically enumerable. So the classifier reports a row it cannot attribute as `unattributable` rather than `orphaned` — the guard that stands between a future sweep and a live datasource credential. Also pins the two directional guards the card names: a rotateKey() re-wrap keeps the handle stable and reads as in-force (rotation metadata never decides a verdict), and a legacy inline value_enc contributes no handle while flagging its (namespace, key) siblings. Tests drive a real ObjectQL over the real SysSetting/SysSecret schemas: the orphan-generating mechanism is reproduced rather than cited, and the fixture carries all four classes so the pins are not vacuous. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARidKDYSCD56LaygrvDPnk * fix(service-settings): write the refKey NUL separator as an escape, not a raw byte check:nul-bytes caught a raw 0x00 in the composite-key separator -- the exact accident the gate documents (an editing tool materialising an escape while the author writes about it). The backslash-u-0000 spelling is byte-identical at run time. Also documents why NUL rather than a printable separator: no namespace or specifier key can contain one, so no two distinct pairs alias into one composite -- and an aliased pair here would silently change a row's verdict. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARidKDYSCD56LaygrvDPnk * test(service-settings): route the context-dropping engine double through assertEngineUpdateDispatch check:engine-double-contract (convention-scoped, so dispatch-gates.mjs cannot derive it) flagged the #8103 harness's SettingsEngine double as declaring an update() that does not route through the shared dispatch predicate. Took the pin rather than a baseline entry: the predicate reads only where / multi / data.id and never looks at context, so it is orthogonal to the defect this double reproduces (dropping context so the engine strips the read-only value_enc). The double stays loose in exactly one dimension and is now conformant in the one that drifts silently -- by-id vs multi. multi:true is passed unconditionally instead of re-deciding the branch, because that IS the settings adapter's contract: a scalar where.id outranks multi in the shared predicate, so one call reproduces both of wrapEngineAsSettingsEngine's branches without mirroring the guard. Imported from @objectstack/objectql (already a devDependency, already aliased to source by vitest.config.ts) rather than @objectstack/metadata-core: the cycle rule prefers metadata-core only when objectql depends on the package being pinned, and objectql does not depend on service-settings. No verdict logic changed; settings-service.ts and crypto-adapter.ts untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARidKDYSCD56LaygrvDPnk --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent aca68eb commit 59ed2e9

4 files changed

Lines changed: 1088 additions & 0 deletions

File tree

.changeset/olive-pots-tease.md

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
'@objectstack/service-settings': minor
3+
---
4+
5+
Add a **report-only** classifier for `sys_secret` orphans (#8103), plus the reachability
6+
measurements a sweep would depend on.
7+
8+
`classifySysSecretRows()` is a pure, read-only function over caller-supplied snapshots: it
9+
never writes, never deletes and never decrypts, and its `SecretRowSnapshot` type
10+
deliberately has no `ciphertext` member. It reports which `sys_secret` rows the settings
11+
subsystem still references (`in_force`), which are unreferenced and attributable to a
12+
declared encrypted specifier (`orphaned`), and which it cannot attribute at all
13+
(`unattributable`).
14+
15+
That third verdict exists because re-measuring #8063's reachability argument **falsified**
16+
one of its three facts: `sys_setting.value_enc` is *not* the only column that holds a
17+
`sys_secret` handle. The store has three producers — `SettingsService`, the engine's
18+
`secret`-field channel (which stores `secret:<id>` on any business row, including
19+
tenant-authored objects), and the datasource credential binder (`sys_secret:<id>` at
20+
`external.credentialsRef`). Two are invisible from this package and the engine's set of
21+
holders is not statically enumerable, so "unreferenced by `sys_setting`" is not
22+
"unreferenced". Rows that cannot be attributed are reported, never classified as orphans,
23+
and the report carries explicit caveats naming its own blind spots.
24+
25+
The classifier also pins the two directional guards the card names: a row re-wrapped in
26+
place by `rotateKey()` keeps its handle and is reported `in_force` (rotation metadata never
27+
decides a verdict), and a legacy inline `value_enc` contributes no handle to the referenced
28+
set while flagging any `sys_secret` row sharing its `(namespace, key)`.
29+
30+
No deletion ships with this change — the vehicle for removing orphans remains an open
31+
maintainer decision.

packages/services/service-settings/src/index.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,25 @@ export {
6666
redactSecretValues,
6767
dropEchoedSecretMasks,
6868
} from './settings-secret-redaction.js';
69+
// #8103 — REPORT-ONLY classification of `sys_secret` rows against the settings
70+
// subsystem's references. Published because the operator-facing vehicle for it
71+
// (admin command / opt-in script) is still an open maintainer decision and will
72+
// live outside this package; the classifier is the part that is safe to settle
73+
// now. ⛔ Contains no deletion and must not grow one — and note the verdict
74+
// vocabulary's third value: `sys_secret` has three producers, so "unreferenced
75+
// by `sys_setting`" is NOT "unreferenced". See the module header.
76+
export {
77+
classifySysSecretRows,
78+
collectEncryptedSpecifierRefs,
79+
isSecretHandle,
80+
SECRET_HANDLE_PREFIX,
81+
type ClassifiedSecretRow,
82+
type EncryptedSpecifierRef,
83+
type SecretRowSnapshot,
84+
type SecretRowVerdict,
85+
type SettingRowSnapshot,
86+
type SysSecretOrphanReport,
87+
} from './sys-secret-orphan-report.js';
6988
export {
7089
settingsObjects,
7190
settingsPluginManifestHeader,

0 commit comments

Comments
 (0)