mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-12 06:06:10 +00:00
fix: close four small gaps around the record surface
The privacy dashboard re-implemented the audit-retention resolver with its own default and its own floor. Two implementations of one disclosure, quoted to the same person on two pages and by the purge job; it uses the shared resolver now. The record-content branch in the settings gate matched a classification rather than a page, so a second manage-writable destination would have rendered the allergy and family-history managers under its own heading without anybody choosing that. It resolves through a slug map, and the contract test fails if the map and the classification disagree in either direction — a destination cannot join the kind without somebody deciding what it renders. The managed-profile nav filter assumed a guardian grant is always manage. It is today, which is why the check was easy to leave out; an entry below that level would have listed destinations the gate refuses. Interpolation used a replacement string, and String.replace reads $&, $`, $', $1 and $$ out of one. A display name containing $' rewrote the sentence around it — and display names land in exactly the sentences that say who did what in somebody's health record. A replacer function has no $ grammar at all, which removes the class rather than escaping the members of it somebody thought of.
This commit is contained in:
1 parent
5de488c35c
commit
6ae0cc993f
9 files changed
+272
-14
No files matched your search
@@ -0,0 +1,83 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { getServerTranslator } from "@/lib/i18n/server-translator";
|
||||
|
||||
/**
|
||||
* An interpolated value is substituted literally, whatever is in it.
|
||||
*
|
||||
* ## The defect
|
||||
*
|
||||
* Both translators filled `{param}` with
|
||||
* `value.replace(new RegExp("\\{k\\}", "g"), String(v))`, and a replacement
|
||||
* STRING is not literal text: `String.prototype.replace` reads `$&`, `` $` ``,
|
||||
* `$'`, `$1` and `$$` out of it. So a value containing `$'` splices in
|
||||
* everything that followed the placeholder, `` $` `` splices in everything
|
||||
* before it, and `$&` repeats the placeholder itself. The sentence a person
|
||||
* reads is then not the sentence the bundle holds.
|
||||
*
|
||||
* Every value that reaches this path is user-controlled, and the ones that
|
||||
* matter most are the ones this release added: a display name lands in
|
||||
* "{name} removed a reading" and in "{name} tried to change a dose and was
|
||||
* refused", on the panel where an owner works out what somebody did in their
|
||||
* health record. A name of `$'` turns "Alex removed a reading" into a sentence
|
||||
* with the wrong subject and no way for the reader to tell.
|
||||
*
|
||||
* Not an injection in the XSS sense — the result is React text children, and
|
||||
* this repository ships no markdown renderer for exactly that reason. It is a
|
||||
* correctness and honesty defect in an audit-facing sentence, which on this
|
||||
* surface is the more expensive kind.
|
||||
*
|
||||
* ## The fix, and why the test is written this way
|
||||
*
|
||||
* A replacer FUNCTION. Its return value is used verbatim and has no `$`
|
||||
* grammar at all, which removes the whole class rather than escaping the
|
||||
* members of it somebody thought of.
|
||||
*
|
||||
* The cases below are the five `$` forms plus a plain control, and each
|
||||
* asserts the WHOLE sentence rather than that it contains the value: a
|
||||
* splice adds text without removing the value, so "contains" would pass on
|
||||
* every one of them.
|
||||
*/
|
||||
describe("a translated sentence survives its own parameters", () => {
|
||||
const t = getServerTranslator("en").t;
|
||||
// A real key with one placeholder, and it is one of the ones this release
|
||||
// added — the sharing activity line an owner reads about somebody else.
|
||||
const KEY = "recordSharing.activityVerb.measurementDelete";
|
||||
|
||||
it("has a template with exactly one placeholder", () => {
|
||||
// The pin. A key whose text lost its `{name}` would make every case below
|
||||
// pass by never substituting anything.
|
||||
const raw = t(KEY);
|
||||
expect(raw).toContain("{name}");
|
||||
expect(raw.match(/\{name\}/g)).toHaveLength(1);
|
||||
expect(t(KEY, { name: "Alex" })).toBe("Alex removed a reading");
|
||||
});
|
||||
|
||||
const hostile = [
|
||||
{ label: "the trailing-context form", value: "$'" },
|
||||
{ label: "the leading-context form", value: "$`" },
|
||||
{ label: "the whole-match form", value: "$&" },
|
||||
{ label: "a capture reference", value: "$1" },
|
||||
{ label: "an escaped dollar", value: "$$" },
|
||||
{ label: "a plain name, as the control", value: "Alex" },
|
||||
] as const;
|
||||
|
||||
for (const { label, value } of hostile) {
|
||||
it(`substitutes ${label} verbatim`, () => {
|
||||
expect(t(KEY, { name: value })).toBe(`${value} removed a reading`);
|
||||
});
|
||||
}
|
||||
|
||||
it("substitutes every occurrence, and only the named placeholder", () => {
|
||||
// The `g` flag is load-bearing for keys that name a value twice, and the
|
||||
// replacer must not change that.
|
||||
const both = getServerTranslator("en").t("recordSharing.activity.opened", {
|
||||
name: "$'",
|
||||
count: 3,
|
||||
});
|
||||
expect(both).toContain("$'");
|
||||
expect(both).toContain("3");
|
||||
expect(both).not.toContain("{name}");
|
||||
expect(both).not.toContain("{count}");
|
||||
});
|
||||
});
|
||||
@@ -290,7 +290,16 @@ export function I18nProvider({
|
||||
// Replace {param} placeholders
|
||||
if (params) {
|
||||
for (const [k, v] of Object.entries(params)) {
|
||||
value = value.replace(new RegExp(`\\{${k}\\}`, "g"), String(v));
|
||||
// A REPLACER FUNCTION, not a replacement string. `String.replace`
|
||||
// reads `$&`, `$\``, `$'`, `$1` and `$$` out of a replacement
|
||||
// STRING, so a value carrying one of them rewrites the sentence
|
||||
// around it: a display name of `$\'` splices everything after the
|
||||
// placeholder back in, and `$\`` splices everything before it. Every
|
||||
// interpolated value here is user-controlled — a person's display
|
||||
// name is the common one, and it lands in sentences on the sharing
|
||||
// panel that describe who did what. A function replacement is
|
||||
// substituted verbatim and has no `$` grammar at all.
|
||||
value = value.replace(new RegExp(`\\{${k}\\}`, "g"), () => String(v));
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -24,7 +24,16 @@ export function getServerTranslator(locale: Locale): ServerTranslator {
|
||||
if (value === undefined) return key;
|
||||
if (params) {
|
||||
for (const [k, v] of Object.entries(params)) {
|
||||
value = value.replace(new RegExp(`\\{${k}\\}`, "g"), String(v));
|
||||
// A REPLACER FUNCTION, not a replacement string. `String.replace`
|
||||
// reads `$&`, `$\``, `$'`, `$1` and `$$` out of a replacement
|
||||
// STRING, so a value carrying one of them rewrites the sentence
|
||||
// around it: a display name of `$\'` splices everything after the
|
||||
// placeholder back in, and `$\`` splices everything before it. Every
|
||||
// interpolated value here is user-controlled — a person's display
|
||||
// name is the common one, and it lands in sentences on the sharing
|
||||
// panel that describe who did what. A function replacement is
|
||||
// substituted verbatim and has no `$` grammar at all.
|
||||
value = value.replace(new RegExp(`\\{${k}\\}`, "g"), () => String(v));
|
||||
}
|
||||
}
|
||||
return value;
|
||||
|
||||
Reference in new issue
Block a user