Repository navigation
The eight-bullet secret read-mask is declared twice with no gate binding them — objectql's SECRET_MASK and service-settings' SETTINGS_SECRET_MASK #7572
Description
Activity
Findings triage: resolved the co-applied
pm:queue+findingdual state →finding(keptdomain:spec). The card's own Notes rule the grade: "inert until someone edits one of the two literals — this is a drift-prevention card, not a live defect." Both masks are byte-identical today and each side is pinned by its own tests; nothing pages anyone until an edit desynchronises them.- Hold restart condition: promote to
pm:queuewhen (a) either literal is touched, or (b) the spec lane picks up Option A (hoist into@objectstack/spec, objectql re-exports) as part of contract-constant work. The in-code breadcrumb atsettings-secret-redaction.tsalready points here. - Routing kept as filed:
domain:specmatches Option A's landing (one definition inpackages/spec, consumers import); the settings-service redeclaration rationale is recorded on-card. - Dup check: sibling QA checklist:
platform-core.settings-hub-roundtrip's secret clause says what must NOT be returned but never what IS — it cannot distinguish masked from omitted #7573 (checklist clause) covers the QA-visible half; no other open card binds these two constants.
本评论来自分诊座位 Routine（#5474 试点），不构成认领。
Generated by Claude Code
- Hold restart condition: promote to
Promoted
finding→pm:queueby the spec-lane seat, executing the maintainer's lane-triage authorization (spec-lane PM session chat, 2026-08-11, verbatim: 「你的车道你可以执行 分类改标」; sessionsession_01JY2Q5Xto1u8YHADgrZDTnk). Grading rationale: restore-invariant class — the eight-bullet secret read-mask is declared twice with no gate binding the copies; the repo already states the invariant (one mask), so the finding carries its own decision. Dispatch shape: bind with a gate/pin rather than hoping the copies stay in sync.
Generated by Claude Code
Claim: PM loop round 2 (spec lane)
Session:session_01JY2Q5Xto1u8YHADgrZDTnk
Branch:claude/issue-7572-secret-mask-single-source
Worktree: cloud dev session (own container)
Domain:domain:spec
File surface:packages/objectql/src/secret-fields.ts(SECRET_MASK,:42) + service-settings'SETTINGS_SECRET_MASK+ the binding (shared constant hoist intopackages/specper the shared-contract rule, or a cross-package pin — dev measures which is the honest fix and states it in the PR). If the fix ends up touching NO spec file, report the deviation — a cross-seat transfer follows, not a silent out-of-lane landing. Stop on breach; explain in the report.
Container & model: M (cross-package invariant binding with a design choice),mode:cloud, model: opus
Serial constraints cleared: no in-flight writer onsecret-fields.ts/ service-settings mask (open PRs + queue checked 12:3xZ). Filter-family cards (#7596 in flight, #7711 queued behind it) share no file with this card.
Generated by Claude Code
OS-DEV-REPORT — #7572
{ "issue": 7572, "status": "done", "branch": "claude/issue-7572-secret-mask-single-source", "pr": "https://github.com/objectstack-ai/objectstack/pull/7759", "premise_still_valid": true, "summary": "Premise confirmed on origin/main before implementing: two byte-identical literals, SECRET_MASK at packages/objectql/src/secret-fields.ts:42 and SETTINGS_SECRET_MASK at packages/services/service-settings/src/settings-secret-redaction.ts:59, with nothing binding them. Implemented Option A: the mask is now declared once in packages/spec/src/data/secret-mask.ts and exported from the @objectstack/spec/data entry point; objectql re-exports it (public API byte-for-byte unchanged — same name, value and literal type, still exported from the package root and from core), and service-settings aliases it as SETTINGS_SECRET_MASK, keeping its published name. Placement was measured, not invented: ADR-0100's entire spec surface already lives in src/data/ (field.zod.ts's secret/password FieldTypes + ackPlaintextMasking, object.zod.ts's author-time diagnostic), and a dedicated leaf module beside them follows the existing default-value-tokens.ts shape. The #7522 framework-agnostic rationale was re-tested rather than overridden: it argued against depending on objectql (the whole data engine) and still holds — no objectql import was added anywhere in service-settings — but it never applied to spec, which is already a dependency AND already in that package's runtime graph (manifest.ts -> @objectstack/platform-objects/system -> @objectstack/spec/data, a value import in the platform-objects object modules). Pins judged individually per the card: spec-side byte + source-spelling pin restates the literal (a pin importing the constant to check the constant is green by construction); each consumer gained an identity pin plus one restated eight-bullet case; the far-side literal pins in plugin-audit and driver-memory, and the route-level assertions in settings-routes.test.ts, were deliberately left alone. Spec keeps the grep-findable literal spelling #7572 asked to preserve, now pinned by a source read.", "tests": "BUILD: pnpm --filter @objectstack/spec build (3m30s, full DTS); pnpm --filter '@objectstack/objectql^...' --filter '@objectstack/service-settings^...' build (5m32s) — dependency closure first, per the stale-artefact rule. TESTS: @objectstack/spec vitest run -> 'Test Files 378 passed (378) / Tests 9885 passed (9885)' (the -- swallowed my path filters, so this is the full spec suite). objectql vitest run src/secret-fields.test.ts -> 'Test Files 1 passed (1) / Tests 21 passed (21)'. service-settings full suite -> 'Test Files 19 passed (19) / Tests 399 passed (399)'. TYPECHECK: pnpm --filter @objectstack/objectql typecheck -> Done; pnpm --filter @objectstack/spec typecheck -> tsc + check:scripts-typecheck + check:test-typecheck all OK (1m07s). service-settings has NO typecheck script; running tsc -p tsconfig.json directly surfaces 5 pre-existing test-file errors (src/manifests/{ai,sms,storage}.manifest.test.ts, src/settings-service.test.ts, src/translations/settings-translation-coverage.test.ts — manifest i18n label typing and SettingsActionHandler), none in files this PR touches; reported, not fixed. GATES: check:generated -> initially '2 of 13 artifact(s) stale: api-surface/, export-origins/', regenerated exactly those two (each gained one line: 'SECRET_MASK (const)' and 'SECRET_MASK': 'src/data/secret-mask.ts#SECRET_MASK (const)'), then '✓ All 13 generated artifacts are up to date'. authorable-surface.base.json was NOT rewritten (no diff), tree never in MERGE state. check:nul-bytes OK (7117 files scanned); check:merge-driver OK; check:adr-anchors OK (22360 citations resolve); check:spec-parsed-alias OK; check:dual-source-exports '✅ no new dual-source exports: 4843 names, 170 re-exported (single declaration), 0 accepted dual-source'; check:exported-any OK. Note for anyone repeating this: those last two REFUSE to run against an OS_SKIP_DTS=1 build (they read the .d.ts and say so loudly, #7122) — my reverse-verification rebuild had used that flag, so a full spec rebuild was needed before they were meaningful. REVERSE VERIFICATION (direction predicted before running; both matched; restore via checkpoint commit + git checkout, never git stash): (1) break the ONE literal (8 bullets -> 7, spec rebuilt so consumers resolve it) -> spec pin RED on both cases (bytes AND spelling); objectql '1 failed | 20 passed' (restated case red, identity case green, every masked-read behaviour assertion green); service-settings '1 failed | 398 passed', same split. The identity cases staying green is correct, not a hole — they compare one declaration with itself; the ~30 green behaviour assertions are exactly why the restated pins exist. (2) re-introduce a drifted local literal in service-settings (the pre-fix world) -> all 4 new pins RED (identity first) while the pre-existing suite stayed at '395 passed', including every route-level mask assertion in settings-routes.test.ts — the card's invisibility claim, measured. Restore verified by spec's build-input hash returning to b1eb6b0ab6859342, a clean git status, and 399/399 green.", "open_questions": [], "out_of_scope_findings": [] }Changeset: one file, three packages, graded separately —
@objectstack/specminor (new public exportSECRET_MASKon thedataentry; additive, evidenced by the single added line in each ofapi-surface/data.jsonandexport-origins/data.json),@objectstack/objectqlpatch (re-export; no public API change —export-originsconfirms it resolves to a single declaration and the package's export list is untouched),@objectstack/service-settingspatch (SETTINGS_SECRET_MASKkeeps name, value and literal type).No
skip-changesetlabel — this PR ships a changeset, so the label does not apply.Sibling scope: #7573 (the QA-checklist half) is untouched.
docs/qa/platform-checklist/areas/platform-core.jsonstill namesSETTINGS_SECRET_MASKandsettings-secret-redaction.ts, which remain accurate after this change.Gate status:
in_progress— reported at draft-PR time per the dispatch contract; CI convergence, the ready-flip and landing are the PM's. Note: the platform auto-subscribed this session to PR #7759 with a stay-resident / drive-to-green posture. Per.claude/agents/os-dev.md(maintainer ruling 2026-08-11, #7512) the dispatch contract outranks it, so I am not idle-polling CI and have armed no self check-ins; a red gate comes back to me as a patch round.
Generated by Claude Code
Step-7 review — ACCEPT (spec-lane PM, session
session_01JY2Q5Xto1u8YHADgrZDTnk). PR: #7759 (draft,Fixes #7572).- Option A executed per the triage note's own restart condition:
SECRET_MASKdeclared once inpackages/spec/src/data/secret-mask.ts(placement measured against ADR-0100's existing spec surface, mirroring thedefault-value-tokens.tsreserved-vocabulary shape); objectql re-exports with an unchanged public API; service-settings aliases, keeping every import working. The [security] GET /api/settings/:namespace returns encrypted setting values as plaintext — no redaction at the REST read boundary #7522 framework-agnostic reasoning is preserved and shown to not apply (spec already in the package's runtime graph — evidenced, not asserted). - Pin-by-pin load-bearing analysis is the standout: restated pins vs identity pins judged individually, far-side literals (plugin-audit, driver-memory) deliberately left restated to keep their distance-coverage, and the reverse verification demonstrates the split exactly (identity green with itself, restated red on the byte change; the drifted-local-literal experiment reproduces the pre-fix blindness: 395 pre-existing tests green while drifted).
- Changeset three-way grading with api-surface/export-origins line-level evidence (spec minor / objectql patch / service-settings patch).
- Honest report: service-settings' 5 pre-existing tsc errors reported, not fixed. Cross-package landing (objectql + service-settings mechanical import swaps) was declared at claim time under the shared-contract rule — noted here as the cross-seat record.
Landing:
api-surface/export-originsare committed generated artifacts ⇒ joins the os-regen relay. Position: after #7756. Sync lap + flip when its slot clears.
Generated by Claude Code
- Option A executed per the triage note's own restart condition:
- added a commit that references this issue
on Aug 17, 2026 - added a commit that references this issue
on Oct 7, 2026
Follow-up from the #7522 security fix (PR #7554), raised by the implementing dev and accepted by the PM as out of scope for that card.
Symptom
The mask a client sees in place of a redacted secret is now declared in two places, byte-identical by convention only:
SECRET_MASKin@objectstack/objectql— the encrypted-field read mask on the generic CRUD path (ADR-0100), exercised by therecords-forms.encrypted-field-behaviorchecklist item;SETTINGS_SECRET_MASKin@objectstack/service-settings— added by [security] GET /api/settings/:namespace returns encrypted setting values as plaintext — no redaction at the REST read boundary #7522 for the settings REST read boundary.Nothing binds them. An edit to either literal silently desynchronises the two masked-read surfaces a client sees, and the failure is invisible on both sides: each package's own tests keep passing, because each asserts against its own constant.
This is the
declared = enforcedshape one level up — the contract is "a masked read looks like this", and there is no single definition of "this".Why it wasn't fixed in #7522
Deliberate, and the reasoning is worth keeping. The dev's own note:
Plus: a cross-package move whose consumer sweep is effectively the whole repo has no business riding on a security fix.
Options
A — hoist into
@objectstack/spec, objectql re-exports for back-compat. Both packages already depend on spec, and ADR-0100 is already documented inspec/src/data/field.zod.ts. One definition, both sides import it. Recommended.B — leave the duplication, add a cross-package pin test in a package that already depends on both (
cli/plugin-email/verify). Binds the constants, but puts a contract pin in a package whose subject is something else.C — add
@objectstack/objectqlas a dependency of service-settings. Rejected: it undoes the framework-agnostic design the settings service is built around.A is the only option that leaves one definition. A mask is a client-facing contract, and "structurally hard to get wrong" here means the console cannot be shown two different masks.
Notes for whoever takes it
packages/services/service-settings/src/settings-secret-redaction.tsalready carries a comment stating the mirroring and naming this follow-up, so the next reader does not re-derive the analysis.••••••••rather than an escape, deliberately, so a grep for the mask finds both declarations. Keep that property in whatever lands.Source
Follow-up from #7522 (PR #7554), which itself came from the QA run #7514.