mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-11 22:02:16 +00:00
fix(ui): close the holes the audit found in this work's own guards
Every item here is a check that could pass while the thing it checks is broken, or a margin the sweep's codemod could not reach. Guards: - The access-payload matcher accepted `.level` on ANY object. A chart's zoom level or a log record's level would have satisfied it and reported a consumer that does not exist. It now names the bindings the entry actually arrives under; a third binding is a deliberate edit rather than a silent widening. Re-falsified: rename both readers, it goes red. - The `pl-7` sweep skipped any line CONTAINING `//`, so a class string sharing a line with a URL was waved through. Only the part after a real line-comment marker is exempt now. Proven by putting `pl-7` on a line with an `https://` comment: caught. - `LEGIT_IDENTICAL` in the English-leak guard had no staleness check, unlike both of its siblings, and carried two dead keys — one from a removal long before this work, one from the duplicated version fields that came off `/admin`. Both pruned, the check added, falsified with a made-up key. - `split-description.mjs` split inside `z. B.`, `d. h.`, `u. a.` and `p. ex.` and produced a head ending mid-abbreviation. Both halves have to be caught, which is why the rule is a shape (single letter + stop) rather than a list, and the merge re-tests its own tail. Exercised against eight cases including the ones that used to break. Margins the codemod could not see, because the child lives in a different component from the card that spaces it: six form roots under managed record settings (one of which also carried the banned `space-y-5` step), the low-stock status line, the avatar card's explainer, and the backups restore report. Each was adding a margin on top of the card's own gap. Two facts the text diet cut too close, both restored inside the guideline: that an excluded injection site wins over a medication that prefers it, and that the privacy card's second button leads to a full health record.
This commit is contained in:
1 parent
23eae2823e
commit
272c426bea
8 files changed
+112
-20
No files matched your search
@@ -21,6 +21,63 @@ import { readFileSync, writeFileSync } from "node:fs";
|
||||
const LOCALES = ["de", "en", "es", "fr", "it", "pl"];
|
||||
const BOUNDARY = /(?<=[.!?])\s+(?=[A-ZÄÖÜÀÂÉÈÊÎÔÙÛÇŁŚŹŻĄĆĘŃÓ0-9„"„«])/u;
|
||||
|
||||
/**
|
||||
* Abbreviations whose full stop is not a sentence end.
|
||||
*
|
||||
* The boundary above demands an upper-case letter after the space, which
|
||||
* already rejects `z. B.` mid-sentence in German (`B.` continues lower-case
|
||||
* more often than not) — but not reliably: "… z. B. Blutdruck." splits after
|
||||
* `z.`, because `B` IS upper-case. The same hole exists for `d. h.`, `u. a.`,
|
||||
* `e.g.` / `i.e.` before a capitalised noun, and French `p. ex.`. A split
|
||||
* there produces a head that ends mid-abbreviation and a tail that starts
|
||||
* with a fragment, in a file nobody re-reads afterwards.
|
||||
*/
|
||||
const ABBREVIATIONS = [
|
||||
"bzw.",
|
||||
"ca.",
|
||||
"vgl.",
|
||||
"usw.",
|
||||
"e.g.",
|
||||
"i.e.",
|
||||
"cf.",
|
||||
"etc.",
|
||||
"ej.",
|
||||
"es.",
|
||||
"np.",
|
||||
"tzn.",
|
||||
"ecc.",
|
||||
"ex.",
|
||||
];
|
||||
|
||||
/**
|
||||
* A single letter followed by a full stop is an abbreviation part, never a
|
||||
* sentence: `z. B.`, `d. h.`, `u. a.`, `p. ex.`. Both halves have to be
|
||||
* caught, which is why this is a shape rather than a list — catching only
|
||||
* `z.` still splits after the `B.`.
|
||||
*/
|
||||
const SINGLE_LETTER = /(^|\s)\p{L}\.$/u;
|
||||
|
||||
/** Split on sentence boundaries, skipping the ones an abbreviation created. */
|
||||
function splitSentences(text) {
|
||||
const rough = text.split(BOUNDARY);
|
||||
const out = [];
|
||||
for (const part of rough) {
|
||||
const previous = out[out.length - 1];
|
||||
const lower = previous?.toLowerCase() ?? "";
|
||||
const endsInAbbreviation =
|
||||
previous !== undefined &&
|
||||
(SINGLE_LETTER.test(previous) ||
|
||||
ABBREVIATIONS.some(
|
||||
(abbr) => lower.endsWith(` ${abbr}`) || lower === abbr,
|
||||
));
|
||||
// Merge, then let the NEXT iteration re-test the merged tail: `z. B.`
|
||||
// needs two merges, and a single pass would stop after the first.
|
||||
if (endsInAbbreviation) out[out.length - 1] = `${previous} ${part}`;
|
||||
else out.push(part);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
function get(bundle, path) {
|
||||
return path
|
||||
.split(".")
|
||||
@@ -56,7 +113,7 @@ for (const locale of LOCALES) {
|
||||
console.log(`${locale} ${key}: MISSING`);
|
||||
continue;
|
||||
}
|
||||
const parts = value.split(BOUNDARY);
|
||||
const parts = splitSentences(value);
|
||||
if (parts.length < 2) {
|
||||
console.log(`${locale} ${key}: single sentence, untouched`);
|
||||
continue;
|
||||
|
||||
@@ -92,6 +92,14 @@ const ACCOUNT_ENTRY_CONSUMERS = [
|
||||
join(SRC, "components", "layout", "account-switcher-menu.tsx"),
|
||||
];
|
||||
const ACTIVE_RECORD_CONSUMER = join(SRC, "hooks", "use-record-capabilities.ts");
|
||||
|
||||
/**
|
||||
* The names an access ENTRY is bound to at the two consumer sites. Pinning
|
||||
* these is what stops the matcher from accepting `.level` on an unrelated
|
||||
* object; a new consumer that binds it under a third name adds that name here,
|
||||
* which is a deliberate edit rather than a silent widening.
|
||||
*/
|
||||
const ENTRY_BINDINGS = ["entry", "account"];
|
||||
const ACCOUNT_ACCESS_ADDITIVE_FIELDS = [
|
||||
"level",
|
||||
"sections",
|
||||
@@ -245,14 +253,18 @@ describe("account payload consumer guard", () => {
|
||||
|
||||
const readerCount = ACCOUNT_ACCESS_ADDITIVE_FIELDS.reduce(
|
||||
(count, field) => {
|
||||
// The reader binds the entry under its own name (`entry`, `account`,
|
||||
// …), so the matcher pins the property access, not the variable. An
|
||||
// empty match set fails rather than passing quietly.
|
||||
const read = new RegExp(`\\w\\.${field}(?![\\w$])`);
|
||||
// The read has to be a read OF THE PAYLOAD ENTRY, so the matcher names
|
||||
// the bindings the entry actually arrives under. `\w\.<field>` would
|
||||
// have accepted `.level` on any object in the file — a chart's zoom
|
||||
// level, a log record's level — and reported a consumer that does not
|
||||
// exist. An empty match set fails rather than passing quietly.
|
||||
const read = new RegExp(
|
||||
`\\b(?:${ENTRY_BINDINGS.join("|")})\\.${field}(?![\\w$])`,
|
||||
);
|
||||
const readers = entrySources.filter((source) => read.test(source));
|
||||
expect(
|
||||
readers.length,
|
||||
`no client reader for account access field \`${field}\``,
|
||||
`no client reader for account access field \`${field}\` bound as ${ENTRY_BINDINGS.join(" / ")}`,
|
||||
).toBeGreaterThan(0);
|
||||
expect(activeSource).toMatch(new RegExp(`active\\.${field}(?![\\w$])`));
|
||||
return count + readers.length + 1;
|
||||
|
||||
@@ -102,7 +102,6 @@ const LEGIT_IDENTICAL = new Set<string>([
|
||||
"admin.overview.snapshotOfflineGeoOff",
|
||||
"admin.overview.snapshotOfflineGeoOn",
|
||||
"admin.webPushVapidTitle",
|
||||
"insights.healthScore.provenance.sources.appleHealth",
|
||||
"measurements.sourceAppleHealth",
|
||||
"onboarding.source.more.appleHealth",
|
||||
"settings.ai.providerChain.types.anthropic",
|
||||
@@ -114,7 +113,6 @@ const LEGIT_IDENTICAL = new Set<string>([
|
||||
"mood.tag.fastFood",
|
||||
// Technical / config labels conventionally kept verbatim.
|
||||
"admin.hostMetrics.diskBusyPercent",
|
||||
"admin.overview.snapshotBuildSha",
|
||||
"admin.servicesGlobal",
|
||||
"admin.webPushVapidPublicKeyPlaceholder",
|
||||
"settings.ai.baseUrlLabel",
|
||||
@@ -231,4 +229,21 @@ describe("i18n English-leak guard", () => {
|
||||
).toEqual([]);
|
||||
},
|
||||
);
|
||||
|
||||
it("carries no stale LEGIT_IDENTICAL entry", () => {
|
||||
// An allowlist that outlives the key it excused is an allowlist nobody
|
||||
// can reason about: the next reader cannot tell which entries are load-
|
||||
// bearing. Two were already dead when this assertion was written — one
|
||||
// from a key removed long before, one from the duplicated version fields
|
||||
// that came off `/admin`. The sibling allowlists in
|
||||
// `card-description-length.test.ts` and `account-payload-consumer-guard`
|
||||
// carry the same check; this one did not.
|
||||
const stale = [...LEGIT_IDENTICAL].filter(
|
||||
(key) => typeof enMap.get(key) !== "string",
|
||||
);
|
||||
expect(
|
||||
stale,
|
||||
"these keys no longer exist in en.json — drop them from LEGIT_IDENTICAL",
|
||||
).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -223,7 +223,7 @@ export function RestoreSkipReport({
|
||||
<div
|
||||
role="status"
|
||||
data-slot="restore-skip-report"
|
||||
className="border-warning/40 bg-warning/10 mt-4 rounded-md border px-3 py-2 text-sm"
|
||||
className="border-warning/40 bg-warning/10 rounded-md border px-3 py-2 text-sm"
|
||||
>
|
||||
<div className="flex items-start justify-between gap-2">
|
||||
<div className="flex items-start gap-2">
|
||||
|
||||
@@ -170,9 +170,17 @@ describe("the abolished body gutter stays abolished", () => {
|
||||
const source = readFileSync(file, "utf8");
|
||||
for (const [i, line] of source.split("\n").entries()) {
|
||||
// Comments may name the class they retired; class strings may not.
|
||||
// Skipping any line that CONTAINS `//` was too coarse — a class
|
||||
// string on the same line as a URL (`https://…`) would have been
|
||||
// waved through. Only the part of the line after a line-comment
|
||||
// marker is exempt, and a marker inside a string is not one.
|
||||
if (line.trimStart().startsWith("*")) continue;
|
||||
if (line.includes("//")) continue;
|
||||
if (/\bpl-7\b/.test(line)) {
|
||||
const marker = line.indexOf("//");
|
||||
const code =
|
||||
marker === -1 || /["'`]/.test(line.slice(0, marker))
|
||||
? line
|
||||
: line.slice(0, marker);
|
||||
if (/\bpl-7\b/.test(code)) {
|
||||
offenders.push(`${file.slice(ROOT.length + 1)}:${i + 1}`);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -114,7 +114,7 @@ export function AvatarSection() {
|
||||
Two strings rather than one merged key: the same wording is the
|
||||
upload's own constraint copy, and a translator keeps the shorter unit
|
||||
either way. */}
|
||||
<p className="text-muted-foreground mb-4 text-sm">
|
||||
<p className="text-muted-foreground text-sm">
|
||||
{t("settings.avatar.description")} {t("settings.avatar.hint")}
|
||||
</p>
|
||||
<div className="flex items-center gap-4">
|
||||
|
||||
@@ -311,8 +311,8 @@ export function LowStockCard({
|
||||
aria-live="polite"
|
||||
className={
|
||||
msgType === "error"
|
||||
? "text-destructive mt-3 text-sm"
|
||||
: "text-muted-foreground mt-3 text-sm"
|
||||
? "text-destructive text-sm"
|
||||
: "text-muted-foreground text-sm"
|
||||
}
|
||||
>
|
||||
{msg}
|
||||
|
||||
@@ -256,7 +256,7 @@ function ProfileSettingsForm({
|
||||
}
|
||||
|
||||
return (
|
||||
<form className="mt-4 grid gap-4 sm:grid-cols-2" onSubmit={submit}>
|
||||
<form className="grid gap-4 sm:grid-cols-2" onSubmit={submit}>
|
||||
<label className="grid gap-1 text-sm" htmlFor="managed-display-name">
|
||||
{t("settings.sharedRecord.managedSettings.profile.displayName")}
|
||||
<input
|
||||
@@ -437,7 +437,7 @@ function ModulesSettingsForm({
|
||||
}
|
||||
|
||||
return (
|
||||
<form className="mt-4 space-y-3" onSubmit={submit}>
|
||||
<form className="space-y-3" onSubmit={submit}>
|
||||
{entries.map(([key, defaultEnabled]) => (
|
||||
<label className="flex items-center gap-3 text-sm" key={key}>
|
||||
<input
|
||||
@@ -488,7 +488,7 @@ function NotificationsSettingsForm({
|
||||
}
|
||||
|
||||
return (
|
||||
<form className="mt-4 grid gap-4 sm:grid-cols-2" onSubmit={submit}>
|
||||
<form className="grid gap-4 sm:grid-cols-2" onSubmit={submit}>
|
||||
<label className="flex items-center gap-3 text-sm sm:col-span-2">
|
||||
<input
|
||||
defaultChecked={settings.moodReminderEnabled === true}
|
||||
@@ -588,7 +588,7 @@ function ThresholdsSettingsForm({
|
||||
}
|
||||
|
||||
return (
|
||||
<form className="mt-4 grid gap-4 sm:grid-cols-3" onSubmit={submit}>
|
||||
<form className="grid gap-4 sm:grid-cols-3" onSubmit={submit}>
|
||||
<label className="grid gap-1 text-sm" htmlFor="managed-threshold-metric">
|
||||
{t("settings.sharedRecord.managedSettings.thresholds.metric")}
|
||||
<select
|
||||
@@ -675,7 +675,7 @@ function CoachSettingsForm({
|
||||
}
|
||||
|
||||
return (
|
||||
<form className="mt-4 space-y-4" onSubmit={submit}>
|
||||
<form className="space-y-4" onSubmit={submit}>
|
||||
<label className="flex items-center gap-3 text-sm">
|
||||
<input
|
||||
defaultChecked={settings.disableCoach === true}
|
||||
@@ -897,7 +897,7 @@ function InsightsSettingsForm({
|
||||
);
|
||||
|
||||
return (
|
||||
<form className="mt-4 space-y-5" onSubmit={submit}>
|
||||
<form className="space-y-4" onSubmit={submit}>
|
||||
{renderItems("section", sections)}
|
||||
{renderItems("tile", tiles)}
|
||||
<SaveButton disabled={disabled} label={saveLabel} />
|
||||
|
||||
Reference in new issue
Block a user