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:
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 = enforced shape 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:
The constant is redeclared rather than imported because this service is deliberately framework-agnostic: it defines its own minimal SettingsEngine instead of importing IDataEngine, and does not depend on @objectstack/objectql at all. Taking a runtime dependency on the whole data engine to reach one string would undo that.
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 in spec/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/objectql as 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
- The duplication is inert until someone edits one of the two literals — this is a drift-prevention card, not a live defect. Priority accordingly.
packages/services/service-settings/src/settings-secret-redaction.ts already carries a comment stating the mirroring and naming this follow-up, so the next reader does not re-derive the analysis.- Both literals are spelled as the literal
•••••••• 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.
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.