From c92a80ee4c9bd4459b5e836cbc44a9cf2c749f80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 6 Aug 2026 23:30:07 +0200 Subject: [PATCH 1/9] feat: describe managed sharing activity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every action a manage-level route files now renders as its own sentence in the owner's activity view, in all six locales. A write delegate can only add, so the generic "made a change" was at least the right shape of fact; a manager changes and removes entries the owner made themselves, and the generic line withholds the only part that matters — whether a reading was corrected or deleted, whether a dose history was purged. The inventory reads the routes and calls the map rather than matching `case` in its source, so an action that falls through to the fallback fails, and it follows each resolved key into all six bundles. Every sentence interpolates the actor's name and nothing else: the audit table is not encrypted and a value in a verb line would be a health figure in a second store. The revoke dialog's retention disclosure no longer depends on which of two requests answers first. `retentionDays` rides the grant list, so a client holding a row to revoke holds the window it must state, and the branch that omitted the sentence is gone. --- docs/api/openapi.yaml | 9 + messages/de.json | 59 ++- messages/en.json | 59 ++- messages/es.json | 59 ++- messages/fr.json | 59 ++- messages/it.json | 59 ++- messages/pl.json | 59 ++- .../manage-activity-inventory.test.ts | 487 ++++++++++++++++++ src/app/api/account/grants/route.ts | 16 + .../access/__tests__/access-panel.test.tsx | 42 +- .../__tests__/grant-access-level.test.tsx | 48 +- .../settings/access/grants-given-card.tsx | 38 +- src/lib/openapi/routes/account-sharing.ts | 6 + src/lib/queries/use-account-grants.ts | 10 + src/lib/record-activity/activity-verb.ts | 182 ++++++- 15 files changed, 1131 insertions(+), 61 deletions(-) create mode 100644 src/__tests__/manage-activity-inventory.test.ts diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index bbd1d6b42..8fbadf170 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -37000,9 +37000,18 @@ components: type: array items: $ref: "#/components/schemas/AccountGrant" + retentionDays: + type: integer + minimum: -9007199254740991 + maximum: 9007199254740991 + description: How long the record's activity feed can still attribute an entry to the person who made it, in days. + Resolved from the instance's audit retention setting (365 by default, operator-configurable), NOT a + constant. It rides this payload so a client ending an access can state the window in the same breath, + without a second request whose timing would decide whether the owner was told. required: - given - received + - retentionDays additionalProperties: false description: "Both directions at once: `given` are the grants this account has offered on its own record, `received` are the ones offered to it. Capped at 100 rows per direction, newest first." diff --git a/messages/de.json b/messages/de.json index 2e876a92b..50224cb88 100644 --- a/messages/de.json +++ b/messages/de.json @@ -4461,7 +4461,9 @@ "dateMdy": "Monat / Tag / Jahr", "dateYmd": "Jahr / Monat / Tag" }, - "modules": { "title": "Module" }, + "modules": { + "title": "Module" + }, "notifications": { "title": "Benachrichtigungen", "moodReminder": "Stimmungserinnerung aktivieren", @@ -9652,7 +9654,60 @@ "medicationSideEffectCreate": "{name} hat eine Nebenwirkung notiert", "medicationCreate": "{name} hat ein Medikament angelegt", "medicationIntake": "{name} hat eine Dosis eingetragen", - "medicationIntakeRefused": "{name} wollte eine Dosis ändern oder eine Erinnerung verschieben und wurde abgewiesen" + "medicationIntakeRefused": "{name} wollte eine Dosis ändern oder eine Erinnerung verschieben und wurde abgewiesen", + "measurementUpdate": "{name} hat einen Messwert geändert", + "measurementDelete": "{name} hat einen Messwert gelöscht", + "measurementDeleteBulk": "{name} hat mehrere Messwerte auf einmal gelöscht", + "measurementRestore": "{name} hat einen Messwert wiederhergestellt", + "measurementReminderCreate": "{name} hat eine Messerinnerung angelegt", + "measurementReminderUpdate": "{name} hat eine Messerinnerung geändert", + "measurementReminderDelete": "{name} hat eine Messerinnerung gelöscht", + "measurementReminderComplete": "{name} hat eine Messerinnerung erledigt", + "measurementReminderSatisfy": "{name} hat eine Messerinnerung abgehakt", + "labResultUpdate": "{name} hat einen Laborwert geändert", + "labResultDelete": "{name} hat einen Laborwert gelöscht", + "labResultRestore": "{name} hat einen Laborwert wiederhergestellt", + "biomarkerUpdate": "{name} hat einen Biomarker geändert", + "biomarkerDelete": "{name} hat einen Biomarker gelöscht", + "allergyUpdate": "{name} hat eine Allergie geändert", + "allergyDelete": "{name} hat eine Allergie gelöscht", + "familyHistoryUpdate": "{name} hat einen Eintrag zur Familienanamnese geändert", + "familyHistoryDelete": "{name} hat einen Eintrag zur Familienanamnese gelöscht", + "illnessEpisodeUpdate": "{name} hat einen Krankheitseintrag geändert", + "illnessEpisodeDelete": "{name} hat einen Krankheitseintrag gelöscht", + "illnessEpisodeResolve": "{name} hat eine Erkrankung als beendet markiert", + "illnessEpisodeRestore": "{name} hat einen Krankheitseintrag wiederhergestellt", + "illnessDayLogUpsert": "{name} hat einen Krankheitstag erfasst", + "medicationUpdate": "{name} hat ein Medikament geändert", + "medicationDelete": "{name} hat ein Medikament gelöscht", + "medicationGlp1Update": "{name} hat eine GLP-1-Therapie geändert", + "medicationScheduleCreate": "{name} hat einen Einnahmeplan angelegt", + "medicationScheduleUpdate": "{name} hat einen Einnahmeplan geändert", + "medicationScheduleDelete": "{name} hat einen Einnahmeplan gelöscht", + "medicationInventoryCreate": "{name} hat eine Packung angelegt", + "medicationInventoryUpdate": "{name} hat eine Packung geändert", + "medicationInventoryDelete": "{name} hat eine Packung gelöscht", + "medicationSideEffectDelete": "{name} hat eine Nebenwirkung gelöscht", + "medicationIntakeUpdate": "{name} hat eine Einnahme geändert", + "medicationIntakeDelete": "{name} hat eine Einnahme gelöscht", + "medicationIntakeBulkDelete": "{name} hat mehrere Einnahmen auf einmal gelöscht", + "medicationIntakePurge": "{name} hat die gesamte Einnahmehistorie eines Medikaments gelöscht", + "medicationIntakeImport": "{name} hat einen Einnahme-Import für ein Medikament gestartet", + "medicationIntakeHistoryImport": "{name} hat einen Import der Einnahmehistorie gestartet", + "moodEntryCreate": "{name} hat einen Stimmungseintrag angelegt", + "moodEntryUpdate": "{name} hat einen Stimmungseintrag geändert", + "moodEntryDelete": "{name} hat einen Stimmungseintrag gelöscht", + "moodDeleteBulk": "{name} hat mehrere Stimmungseinträge auf einmal gelöscht", + "moodRestore": "{name} hat einen Stimmungseintrag wiederhergestellt", + "mentalHealthCreate": "{name} hat ein Fragebogenergebnis erfasst", + "cycleDelete": "{name} hat einen Zyklus gelöscht", + "cycleDayLogUpsert": "{name} hat einen Zyklustag erfasst", + "cycleDayLogUpdate": "{name} hat einen Zyklustag geändert", + "cycleDayLogDelete": "{name} hat einen Zyklustag gelöscht", + "cyclePeriodBoundary": "{name} hat den Beginn oder das Ende einer Periode geändert", + "cycleSymptomCreate": "{name} hat ein Zyklussymptom angelegt", + "nutrientWaterWrite": "{name} hat die Trinkmenge erfasst", + "healthRecordExport": "{name} hat die Akte exportiert" }, "section": { "measurements": { diff --git a/messages/en.json b/messages/en.json index 704da23e3..255435074 100644 --- a/messages/en.json +++ b/messages/en.json @@ -4461,7 +4461,9 @@ "dateMdy": "Month / day / year", "dateYmd": "Year / month / day" }, - "modules": { "title": "Modules" }, + "modules": { + "title": "Modules" + }, "notifications": { "title": "Notifications", "moodReminder": "Enable mood reminder", @@ -9652,7 +9654,60 @@ "medicationSideEffectCreate": "{name} noted a side effect", "medicationCreate": "{name} added a medication", "medicationIntake": "{name} marked a dose", - "medicationIntakeRefused": "{name} tried to change a dose or defer a reminder, and was refused" + "medicationIntakeRefused": "{name} tried to change a dose or defer a reminder, and was refused", + "measurementUpdate": "{name} changed a reading", + "measurementDelete": "{name} removed a reading", + "measurementDeleteBulk": "{name} removed several readings at once", + "measurementRestore": "{name} restored a reading", + "measurementReminderCreate": "{name} added a measurement reminder", + "measurementReminderUpdate": "{name} changed a measurement reminder", + "measurementReminderDelete": "{name} removed a measurement reminder", + "measurementReminderComplete": "{name} completed a measurement reminder", + "measurementReminderSatisfy": "{name} cleared a measurement reminder", + "labResultUpdate": "{name} changed a lab result", + "labResultDelete": "{name} removed a lab result", + "labResultRestore": "{name} restored a lab result", + "biomarkerUpdate": "{name} changed a biomarker", + "biomarkerDelete": "{name} removed a biomarker", + "allergyUpdate": "{name} changed an allergy", + "allergyDelete": "{name} removed an allergy", + "familyHistoryUpdate": "{name} changed a family history entry", + "familyHistoryDelete": "{name} removed a family history entry", + "illnessEpisodeUpdate": "{name} changed an illness entry", + "illnessEpisodeDelete": "{name} removed an illness entry", + "illnessEpisodeResolve": "{name} marked an illness as over", + "illnessEpisodeRestore": "{name} restored an illness entry", + "illnessDayLogUpsert": "{name} recorded an illness day", + "medicationUpdate": "{name} changed a medication", + "medicationDelete": "{name} removed a medication", + "medicationGlp1Update": "{name} changed a GLP-1 course", + "medicationScheduleCreate": "{name} added a dosing schedule", + "medicationScheduleUpdate": "{name} changed a dosing schedule", + "medicationScheduleDelete": "{name} removed a dosing schedule", + "medicationInventoryCreate": "{name} added a medication package", + "medicationInventoryUpdate": "{name} changed a medication package", + "medicationInventoryDelete": "{name} removed a medication package", + "medicationSideEffectDelete": "{name} removed a side effect", + "medicationIntakeUpdate": "{name} changed a dose", + "medicationIntakeDelete": "{name} removed a dose", + "medicationIntakeBulkDelete": "{name} removed several doses at once", + "medicationIntakePurge": "{name} deleted the whole dose history of a medication", + "medicationIntakeImport": "{name} started a dose import for one medication", + "medicationIntakeHistoryImport": "{name} started a dose history import", + "moodEntryCreate": "{name} added a mood entry", + "moodEntryUpdate": "{name} changed a mood entry", + "moodEntryDelete": "{name} removed a mood entry", + "moodDeleteBulk": "{name} removed several mood entries at once", + "moodRestore": "{name} restored a mood entry", + "mentalHealthCreate": "{name} recorded a questionnaire result", + "cycleDelete": "{name} removed a cycle", + "cycleDayLogUpsert": "{name} recorded a cycle day", + "cycleDayLogUpdate": "{name} changed a cycle day", + "cycleDayLogDelete": "{name} removed a cycle day", + "cyclePeriodBoundary": "{name} changed the start or end of a period", + "cycleSymptomCreate": "{name} added a cycle symptom", + "nutrientWaterWrite": "{name} recorded water intake", + "healthRecordExport": "{name} exported the record" }, "section": { "measurements": { diff --git a/messages/es.json b/messages/es.json index 13cb2d2cb..a1403afd0 100644 --- a/messages/es.json +++ b/messages/es.json @@ -4461,7 +4461,9 @@ "dateMdy": "Mes / día / año", "dateYmd": "Año / mes / día" }, - "modules": { "title": "Módulos" }, + "modules": { + "title": "Módulos" + }, "notifications": { "title": "Notificaciones", "moodReminder": "Activar recordatorio de ánimo", @@ -9652,7 +9654,60 @@ "medicationSideEffectCreate": "{name} anotó un efecto secundario", "medicationCreate": "{name} añadió un medicamento", "medicationIntake": "{name} registró una dosis", - "medicationIntakeRefused": "{name} intentó cambiar una dosis o aplazar un recordatorio, y fue rechazado" + "medicationIntakeRefused": "{name} intentó cambiar una dosis o aplazar un recordatorio, y fue rechazado", + "measurementUpdate": "{name} cambió una medición", + "measurementDelete": "{name} eliminó una medición", + "measurementDeleteBulk": "{name} eliminó varias mediciones a la vez", + "measurementRestore": "{name} restauró una medición", + "measurementReminderCreate": "{name} añadió un recordatorio de medición", + "measurementReminderUpdate": "{name} cambió un recordatorio de medición", + "measurementReminderDelete": "{name} eliminó un recordatorio de medición", + "measurementReminderComplete": "{name} completó un recordatorio de medición", + "measurementReminderSatisfy": "{name} cerró un recordatorio de medición", + "labResultUpdate": "{name} cambió un resultado de laboratorio", + "labResultDelete": "{name} eliminó un resultado de laboratorio", + "labResultRestore": "{name} restauró un resultado de laboratorio", + "biomarkerUpdate": "{name} cambió un biomarcador", + "biomarkerDelete": "{name} eliminó un biomarcador", + "allergyUpdate": "{name} cambió una alergia", + "allergyDelete": "{name} eliminó una alergia", + "familyHistoryUpdate": "{name} cambió un antecedente familiar", + "familyHistoryDelete": "{name} eliminó un antecedente familiar", + "illnessEpisodeUpdate": "{name} cambió una enfermedad", + "illnessEpisodeDelete": "{name} eliminó una enfermedad", + "illnessEpisodeResolve": "{name} marcó una enfermedad como terminada", + "illnessEpisodeRestore": "{name} restauró una enfermedad", + "illnessDayLogUpsert": "{name} registró un día de enfermedad", + "medicationUpdate": "{name} cambió un medicamento", + "medicationDelete": "{name} eliminó un medicamento", + "medicationGlp1Update": "{name} cambió un tratamiento con GLP-1", + "medicationScheduleCreate": "{name} añadió una pauta de dosificación", + "medicationScheduleUpdate": "{name} cambió una pauta de dosificación", + "medicationScheduleDelete": "{name} eliminó una pauta de dosificación", + "medicationInventoryCreate": "{name} añadió un envase de medicamento", + "medicationInventoryUpdate": "{name} cambió un envase de medicamento", + "medicationInventoryDelete": "{name} eliminó un envase de medicamento", + "medicationSideEffectDelete": "{name} eliminó un efecto secundario", + "medicationIntakeUpdate": "{name} cambió una dosis", + "medicationIntakeDelete": "{name} eliminó una dosis", + "medicationIntakeBulkDelete": "{name} eliminó varias dosis a la vez", + "medicationIntakePurge": "{name} borró todo el historial de dosis de un medicamento", + "medicationIntakeImport": "{name} inició una importación de dosis de un medicamento", + "medicationIntakeHistoryImport": "{name} inició una importación del historial de dosis", + "moodEntryCreate": "{name} añadió una entrada de ánimo", + "moodEntryUpdate": "{name} cambió una entrada de ánimo", + "moodEntryDelete": "{name} eliminó una entrada de ánimo", + "moodDeleteBulk": "{name} eliminó varias entradas de ánimo a la vez", + "moodRestore": "{name} restauró una entrada de ánimo", + "mentalHealthCreate": "{name} registró el resultado de un cuestionario", + "cycleDelete": "{name} eliminó un ciclo", + "cycleDayLogUpsert": "{name} registró un día del ciclo", + "cycleDayLogUpdate": "{name} cambió un día del ciclo", + "cycleDayLogDelete": "{name} eliminó un día del ciclo", + "cyclePeriodBoundary": "{name} cambió el inicio o el final de una regla", + "cycleSymptomCreate": "{name} añadió un síntoma del ciclo", + "nutrientWaterWrite": "{name} registró la ingesta de agua", + "healthRecordExport": "{name} exportó el historial" }, "section": { "measurements": { diff --git a/messages/fr.json b/messages/fr.json index 0c97286b6..3df592639 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -4461,7 +4461,9 @@ "dateMdy": "Mois / jour / année", "dateYmd": "Année / mois / jour" }, - "modules": { "title": "Modules" }, + "modules": { + "title": "Modules" + }, "notifications": { "title": "Notifications", "moodReminder": "Activer le rappel d’humeur", @@ -9652,7 +9654,60 @@ "medicationSideEffectCreate": "{name} a noté un effet indésirable", "medicationCreate": "{name} a ajouté un médicament", "medicationIntake": "{name} a enregistré une prise", - "medicationIntakeRefused": "{name} a tenté de modifier une dose ou de reporter un rappel, et a été refusé" + "medicationIntakeRefused": "{name} a tenté de modifier une dose ou de reporter un rappel, et a été refusé", + "measurementUpdate": "{name} a modifié une mesure", + "measurementDelete": "{name} a supprimé une mesure", + "measurementDeleteBulk": "{name} a supprimé plusieurs mesures d'un coup", + "measurementRestore": "{name} a restauré une mesure", + "measurementReminderCreate": "{name} a ajouté un rappel de mesure", + "measurementReminderUpdate": "{name} a modifié un rappel de mesure", + "measurementReminderDelete": "{name} a supprimé un rappel de mesure", + "measurementReminderComplete": "{name} a terminé un rappel de mesure", + "measurementReminderSatisfy": "{name} a clôturé un rappel de mesure", + "labResultUpdate": "{name} a modifié un résultat de laboratoire", + "labResultDelete": "{name} a supprimé un résultat de laboratoire", + "labResultRestore": "{name} a restauré un résultat de laboratoire", + "biomarkerUpdate": "{name} a modifié un biomarqueur", + "biomarkerDelete": "{name} a supprimé un biomarqueur", + "allergyUpdate": "{name} a modifié une allergie", + "allergyDelete": "{name} a supprimé une allergie", + "familyHistoryUpdate": "{name} a modifié un antécédent familial", + "familyHistoryDelete": "{name} a supprimé un antécédent familial", + "illnessEpisodeUpdate": "{name} a modifié une maladie", + "illnessEpisodeDelete": "{name} a supprimé une maladie", + "illnessEpisodeResolve": "{name} a marqué une maladie comme terminée", + "illnessEpisodeRestore": "{name} a restauré une maladie", + "illnessDayLogUpsert": "{name} a enregistré une journée de maladie", + "medicationUpdate": "{name} a modifié un médicament", + "medicationDelete": "{name} a supprimé un médicament", + "medicationGlp1Update": "{name} a modifié un traitement GLP-1", + "medicationScheduleCreate": "{name} a ajouté un schéma de prise", + "medicationScheduleUpdate": "{name} a modifié un schéma de prise", + "medicationScheduleDelete": "{name} a supprimé un schéma de prise", + "medicationInventoryCreate": "{name} a ajouté une boîte de médicament", + "medicationInventoryUpdate": "{name} a modifié une boîte de médicament", + "medicationInventoryDelete": "{name} a supprimé une boîte de médicament", + "medicationSideEffectDelete": "{name} a supprimé un effet indésirable", + "medicationIntakeUpdate": "{name} a modifié une prise", + "medicationIntakeDelete": "{name} a supprimé une prise", + "medicationIntakeBulkDelete": "{name} a supprimé plusieurs prises d'un coup", + "medicationIntakePurge": "{name} a effacé tout l'historique des prises d'un médicament", + "medicationIntakeImport": "{name} a lancé un import des prises d'un médicament", + "medicationIntakeHistoryImport": "{name} a lancé un import de l'historique des prises", + "moodEntryCreate": "{name} a ajouté une entrée d'humeur", + "moodEntryUpdate": "{name} a modifié une entrée d'humeur", + "moodEntryDelete": "{name} a supprimé une entrée d'humeur", + "moodDeleteBulk": "{name} a supprimé plusieurs entrées d'humeur d'un coup", + "moodRestore": "{name} a restauré une entrée d'humeur", + "mentalHealthCreate": "{name} a enregistré le résultat d'un questionnaire", + "cycleDelete": "{name} a supprimé un cycle", + "cycleDayLogUpsert": "{name} a enregistré un jour du cycle", + "cycleDayLogUpdate": "{name} a modifié un jour du cycle", + "cycleDayLogDelete": "{name} a supprimé un jour du cycle", + "cyclePeriodBoundary": "{name} a modifié le début ou la fin de règles", + "cycleSymptomCreate": "{name} a ajouté un symptôme de cycle", + "nutrientWaterWrite": "{name} a enregistré la consommation d'eau", + "healthRecordExport": "{name} a exporté le dossier" }, "section": { "measurements": { diff --git a/messages/it.json b/messages/it.json index 9bb80dbf5..9a5f4506e 100644 --- a/messages/it.json +++ b/messages/it.json @@ -4461,7 +4461,9 @@ "dateMdy": "Mese / giorno / anno", "dateYmd": "Anno / mese / giorno" }, - "modules": { "title": "Moduli" }, + "modules": { + "title": "Moduli" + }, "notifications": { "title": "Notifiche", "moodReminder": "Attiva promemoria dell’umore", @@ -9652,7 +9654,60 @@ "medicationSideEffectCreate": "{name} ha annotato un effetto collaterale", "medicationCreate": "{name} ha aggiunto un farmaco", "medicationIntake": "{name} ha registrato una dose", - "medicationIntakeRefused": "{name} ha provato a modificare una dose o a rimandare un promemoria, ed è stato rifiutato" + "medicationIntakeRefused": "{name} ha provato a modificare una dose o a rimandare un promemoria, ed è stato rifiutato", + "measurementUpdate": "{name} ha modificato una misurazione", + "measurementDelete": "{name} ha eliminato una misurazione", + "measurementDeleteBulk": "{name} ha eliminato più misurazioni in una volta", + "measurementRestore": "{name} ha ripristinato una misurazione", + "measurementReminderCreate": "{name} ha aggiunto un promemoria di misurazione", + "measurementReminderUpdate": "{name} ha modificato un promemoria di misurazione", + "measurementReminderDelete": "{name} ha eliminato un promemoria di misurazione", + "measurementReminderComplete": "{name} ha completato un promemoria di misurazione", + "measurementReminderSatisfy": "{name} ha chiuso un promemoria di misurazione", + "labResultUpdate": "{name} ha modificato un risultato di laboratorio", + "labResultDelete": "{name} ha eliminato un risultato di laboratorio", + "labResultRestore": "{name} ha ripristinato un risultato di laboratorio", + "biomarkerUpdate": "{name} ha modificato un biomarcatore", + "biomarkerDelete": "{name} ha eliminato un biomarcatore", + "allergyUpdate": "{name} ha modificato un'allergia", + "allergyDelete": "{name} ha eliminato un'allergia", + "familyHistoryUpdate": "{name} ha modificato una voce di anamnesi familiare", + "familyHistoryDelete": "{name} ha eliminato una voce di anamnesi familiare", + "illnessEpisodeUpdate": "{name} ha modificato una malattia", + "illnessEpisodeDelete": "{name} ha eliminato una malattia", + "illnessEpisodeResolve": "{name} ha segnato una malattia come conclusa", + "illnessEpisodeRestore": "{name} ha ripristinato una malattia", + "illnessDayLogUpsert": "{name} ha registrato una giornata di malattia", + "medicationUpdate": "{name} ha modificato un farmaco", + "medicationDelete": "{name} ha eliminato un farmaco", + "medicationGlp1Update": "{name} ha modificato una terapia GLP-1", + "medicationScheduleCreate": "{name} ha aggiunto uno schema di assunzione", + "medicationScheduleUpdate": "{name} ha modificato uno schema di assunzione", + "medicationScheduleDelete": "{name} ha eliminato uno schema di assunzione", + "medicationInventoryCreate": "{name} ha aggiunto una confezione di farmaco", + "medicationInventoryUpdate": "{name} ha modificato una confezione di farmaco", + "medicationInventoryDelete": "{name} ha eliminato una confezione di farmaco", + "medicationSideEffectDelete": "{name} ha eliminato un effetto collaterale", + "medicationIntakeUpdate": "{name} ha modificato una dose", + "medicationIntakeDelete": "{name} ha eliminato una dose", + "medicationIntakeBulkDelete": "{name} ha eliminato più dosi in una volta", + "medicationIntakePurge": "{name} ha cancellato tutto lo storico delle dosi di un farmaco", + "medicationIntakeImport": "{name} ha avviato un'importazione delle dosi di un farmaco", + "medicationIntakeHistoryImport": "{name} ha avviato un'importazione dello storico delle dosi", + "moodEntryCreate": "{name} ha aggiunto una voce dell'umore", + "moodEntryUpdate": "{name} ha modificato una voce dell'umore", + "moodEntryDelete": "{name} ha eliminato una voce dell'umore", + "moodDeleteBulk": "{name} ha eliminato più voci dell'umore in una volta", + "moodRestore": "{name} ha ripristinato una voce dell'umore", + "mentalHealthCreate": "{name} ha registrato il risultato di un questionario", + "cycleDelete": "{name} ha eliminato un ciclo", + "cycleDayLogUpsert": "{name} ha registrato un giorno del ciclo", + "cycleDayLogUpdate": "{name} ha modificato un giorno del ciclo", + "cycleDayLogDelete": "{name} ha eliminato un giorno del ciclo", + "cyclePeriodBoundary": "{name} ha modificato l'inizio o la fine di una mestruazione", + "cycleSymptomCreate": "{name} ha aggiunto un sintomo del ciclo", + "nutrientWaterWrite": "{name} ha registrato l'assunzione di acqua", + "healthRecordExport": "{name} ha esportato la cartella" }, "section": { "measurements": { diff --git a/messages/pl.json b/messages/pl.json index f01fa2c7f..aa0cf4af3 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -4461,7 +4461,9 @@ "dateMdy": "Miesiąc / dzień / rok", "dateYmd": "Rok / miesiąc / dzień" }, - "modules": { "title": "Moduły" }, + "modules": { + "title": "Moduły" + }, "notifications": { "title": "Powiadomienia", "moodReminder": "Włącz przypomnienie nastroju", @@ -9652,7 +9654,60 @@ "medicationSideEffectCreate": "{name} zanotował działanie niepożądane", "medicationCreate": "{name} dodał lek", "medicationIntake": "{name} zapisał przyjęcie dawki", - "medicationIntakeRefused": "{name} próbował(a) zmienić dawkę lub odłożyć przypomnienie i został(a) odrzucony(-a)" + "medicationIntakeRefused": "{name} próbował(a) zmienić dawkę lub odłożyć przypomnienie i został(a) odrzucony(-a)", + "measurementUpdate": "{name} zmienił pomiar", + "measurementDelete": "{name} usunął pomiar", + "measurementDeleteBulk": "{name} usunął kilka pomiarów naraz", + "measurementRestore": "{name} przywrócił pomiar", + "measurementReminderCreate": "{name} dodał przypomnienie o pomiarze", + "measurementReminderUpdate": "{name} zmienił przypomnienie o pomiarze", + "measurementReminderDelete": "{name} usunął przypomnienie o pomiarze", + "measurementReminderComplete": "{name} zrealizował przypomnienie o pomiarze", + "measurementReminderSatisfy": "{name} zamknął przypomnienie o pomiarze", + "labResultUpdate": "{name} zmienił wynik badania", + "labResultDelete": "{name} usunął wynik badania", + "labResultRestore": "{name} przywrócił wynik badania", + "biomarkerUpdate": "{name} zmienił biomarker", + "biomarkerDelete": "{name} usunął biomarker", + "allergyUpdate": "{name} zmienił alergię", + "allergyDelete": "{name} usunął alergię", + "familyHistoryUpdate": "{name} zmienił wpis w wywiadzie rodzinnym", + "familyHistoryDelete": "{name} usunął wpis w wywiadzie rodzinnym", + "illnessEpisodeUpdate": "{name} zmienił wpis o chorobie", + "illnessEpisodeDelete": "{name} usunął wpis o chorobie", + "illnessEpisodeResolve": "{name} oznaczył chorobę jako zakończoną", + "illnessEpisodeRestore": "{name} przywrócił wpis o chorobie", + "illnessDayLogUpsert": "{name} zapisał dzień choroby", + "medicationUpdate": "{name} zmienił lek", + "medicationDelete": "{name} usunął lek", + "medicationGlp1Update": "{name} zmienił terapię GLP-1", + "medicationScheduleCreate": "{name} dodał schemat dawkowania", + "medicationScheduleUpdate": "{name} zmienił schemat dawkowania", + "medicationScheduleDelete": "{name} usunął schemat dawkowania", + "medicationInventoryCreate": "{name} dodał opakowanie leku", + "medicationInventoryUpdate": "{name} zmienił opakowanie leku", + "medicationInventoryDelete": "{name} usunął opakowanie leku", + "medicationSideEffectDelete": "{name} usunął działanie niepożądane", + "medicationIntakeUpdate": "{name} zmienił przyjęcie dawki", + "medicationIntakeDelete": "{name} usunął przyjęcie dawki", + "medicationIntakeBulkDelete": "{name} usunął kilka przyjęć dawek naraz", + "medicationIntakePurge": "{name} skasował całą historię przyjmowania leku", + "medicationIntakeImport": "{name} rozpoczął import przyjęć dla jednego leku", + "medicationIntakeHistoryImport": "{name} rozpoczął import historii przyjmowania", + "moodEntryCreate": "{name} dodał wpis nastroju", + "moodEntryUpdate": "{name} zmienił wpis nastroju", + "moodEntryDelete": "{name} usunął wpis nastroju", + "moodDeleteBulk": "{name} usunął kilka wpisów nastroju naraz", + "moodRestore": "{name} przywrócił wpis nastroju", + "mentalHealthCreate": "{name} zapisał wynik kwestionariusza", + "cycleDelete": "{name} usunął cykl", + "cycleDayLogUpsert": "{name} zapisał dzień cyklu", + "cycleDayLogUpdate": "{name} zmienił dzień cyklu", + "cycleDayLogDelete": "{name} usunął dzień cyklu", + "cyclePeriodBoundary": "{name} zmienił początek lub koniec miesiączki", + "cycleSymptomCreate": "{name} dodał objaw cyklu", + "nutrientWaterWrite": "{name} zapisał ilość wypitej wody", + "healthRecordExport": "{name} wyeksportował kartę" }, "section": { "measurements": { diff --git a/src/__tests__/manage-activity-inventory.test.ts b/src/__tests__/manage-activity-inventory.test.ts new file mode 100644 index 000000000..6dace765f --- /dev/null +++ b/src/__tests__/manage-activity-inventory.test.ts @@ -0,0 +1,487 @@ +import { readFileSync, readdirSync } from "node:fs"; +import { join, relative } from "node:path"; + +import ts from "typescript"; +import { describe, expect, it } from "vitest"; + +import { revokeBody } from "@/components/settings/access/grants-given-card"; +import { recordActivityVerbLine } from "@/lib/record-activity/activity-verb"; + +/** + * Every MANAGE-admitted action names itself, in all six locales. + * + * ## What this asks that the sibling guard does not + * + * `src/lib/record-activity/__tests__/verb-map-covers-routes.test.ts` asks the + * same question of the WRITE surface: a delegable write whose audit action has + * no sentence renders as "made a change". MANAGE is where that answer stops + * being merely vague and starts being wrong. A WRITE delegate can only add, + * and "somebody added something" is at least the right shape of fact. A MANAGE + * delegate can change and remove entries the owner made themselves, so the + * generic line would tell an owner that something happened in their record + * while withholding the only part they need — whether a reading was corrected + * or deleted, whether a medication was stopped, whether the dose history was + * purged. The level ships under "reconstructable or refused", and a feed that + * cannot say which verb ran does not reconstruct anything. + * + * So the inventory here is the MANAGE half of the same sweep, and it goes two + * steps further than its sibling: + * + * 1. it resolves the sentence by CALLING the map rather than by matching + * `case "…"` in its source, because a `case` inside a comment matches and + * an action whose arm falls through to the fallback does not; + * 2. it follows the resolved key into all six bundles, because a key that + * exists only in English renders as its own dotted path to everyone else. + * + * ## PHI + * + * Two legs, and both are about the same failure. The activity feed reads out + * of the audit table, the audit table is not encrypted, and the temptation + * whenever a sentence feels thin is to put the value in it — "{name} changed + * your weight from 81 to 78". That is a health figure in a second store with + * its own retention and no rotation story. So: every verb line interpolates + * `name` and nothing else, in every locale, and the feed's own DTO carries no + * `details` field for a future caller to reach for. + * + * ## Why the counts are of MATCHES + * + * The verdict is an empty list of unmapped actions, and an empty match set + * says exactly the same thing. Every leg therefore pins the size of what it + * matched — modules walked, MANAGE modules found, one of them named, actions + * read, sentences resolved — so a matcher that quietly stopped matching fails + * instead of passing. `describe("the matcher sees what it looks for")` proves + * both halves against synthetic modules, including the one shape that silenced + * the sibling guard: a resolver call prettier broke across lines. + */ +const API = join(process.cwd(), "src/app/api"); +const MESSAGES = join(process.cwd(), "messages"); +const ACTIVITY_ROUTE = join( + process.cwd(), + "src/app/api/account/activity/route.ts", +); +const GRANTS_GIVEN_CARD = join( + process.cwd(), + "src/components/settings/access/grants-given-card.tsx", +); + +/** The declaration a MANAGE-level route makes. */ +const RECORD_RESOLVER = "requireRecordAuth"; +/** The level this file is the inventory of. */ +const MANAGE = "manage"; +/** What an action with no sentence of its own renders as. */ +const FALLBACK_KEY = "recordSharing.activity.acted"; +/** The six bundles the product ships. */ +const LOCALES = ["de", "en", "es", "fr", "it", "pl"] as const; + +/** Every `route.ts` under `src/app/api`, walked rather than globbed: a glob's + * `*` skips a leading dot and this tree has dot-directories. */ +function routeFiles(dir: string): string[] { + const out: string[] = []; + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const full = join(dir, entry.name); + if (entry.isDirectory()) out.push(...routeFiles(full)); + else if (entry.name === "route.ts") out.push(full); + } + return out; +} + +interface ModuleFacts { + /** Does the module pass `"manage"` to the record resolver anywhere. */ + managesRecord: boolean; + /** + * The action names of the module's AWAITED `auditLog("…")` calls. + * + * Awaited on purpose: a fire-and-forget breadcrumb on a validation failure + * is not something to narrate to the owner. + */ + awaitedAuditActions: string[]; +} + +/** + * Read both facts off one parse. + * + * AST rather than substrings, for the reason the sibling guard learned by + * going green while matching nothing: prettier moves a call's argument onto + * its own line the moment the call grows a second one, and + * `requireRecordAuth("manage"` stops appearing in the source text without + * anything about the route changing. The import table resolves a local name + * back to its exported one, so an alias and a namespace-qualified call both + * count, and a comment naming either symbol counts for nothing. + */ +export function moduleFacts(source: string, fileName: string): ModuleFacts { + const parsed = ts.createSourceFile( + fileName, + source, + ts.ScriptTarget.Latest, + true, + ts.ScriptKind.TS, + ); + + const aliases = new Map(); + const namespaces = new Set(); + parsed.forEachChild((node) => { + if (!ts.isImportDeclaration(node)) return; + const bindings = node.importClause?.namedBindings; + if (!bindings) return; + if (ts.isNamespaceImport(bindings)) { + namespaces.add(bindings.name.text); + return; + } + for (const spec of bindings.elements) { + aliases.set(spec.name.text, (spec.propertyName ?? spec.name).text); + } + }); + + /** The original exported name behind this callee, or null. */ + const calleeName = (callee: ts.Expression): string | null => { + if (ts.isIdentifier(callee)) return aliases.get(callee.text) ?? callee.text; + if ( + ts.isPropertyAccessExpression(callee) && + ts.isIdentifier(callee.expression) && + namespaces.has(callee.expression.text) + ) { + return callee.name.text; + } + return null; + }; + + /** The first argument when it is a plain string literal. */ + const firstStringArg = (node: ts.CallExpression): string | null => { + const first = node.arguments[0]; + return first && ts.isStringLiteralLike(first) ? first.text : null; + }; + + let managesRecord = false; + const awaitedAuditActions: string[] = []; + + const visit = (node: ts.Node): void => { + if (ts.isCallExpression(node)) { + const name = calleeName(node.expression); + if (name === RECORD_RESOLVER && firstStringArg(node) === MANAGE) { + managesRecord = true; + } + } + if (ts.isAwaitExpression(node) && ts.isCallExpression(node.expression)) { + const call = node.expression; + if (calleeName(call.expression) === "auditLog") { + const action = firstStringArg(call); + if (action !== null) awaitedAuditActions.push(action); + } + } + node.forEachChild(visit); + }; + parsed.forEachChild(visit); + + return { managesRecord, awaitedAuditActions }; +} + +/** The message key one action resolves to, or the fallback. */ +function verbKeyFor(action: string): { key: string; params: string[] } { + let key = ""; + let params: string[] = []; + recordActivityVerbLine( + (resolved, given) => { + key = resolved; + params = Object.keys(given ?? {}); + return resolved; + }, + action, + "Alex", + ); + return { key, params }; +} + +function bundle(locale: string): Record { + return JSON.parse( + readFileSync(join(MESSAGES, `${locale}.json`), "utf8"), + ) as Record; +} + +/** The leaf at a dotted path, or undefined. */ +function leaf(root: Record, key: string): unknown { + let node: unknown = root; + for (const segment of key.split(".")) { + if (typeof node !== "object" || node === null) return undefined; + node = (node as Record)[segment]; + } + return node; +} + +/** The `{placeholders}` a translated sentence interpolates. */ +function placeholders(sentence: string): string[] { + return [...sentence.matchAll(/\{(\w+)\}/g)].map((m) => m[1]).sort(); +} + +describe("the matcher sees what it looks for", () => { + const REFORMATTED = ` + import { requireRecordAuth } from "@/lib/api-handler"; + import { auditLog } from "@/lib/auth/audit"; + + export const DELETE = apiHandler(async () => { + const { user } = await requireRecordAuth( + "manage", + "labs", + ); + await auditLog("thing.delete", { userId: user.id }); + return apiSuccess({ id: user.id }); + }); + `; + + it("matches a resolver call broken across lines", () => { + // The shape that silenced the sibling guard: the argument moves onto its + // own line and a substring test goes quiet with nothing else changed. + expect(REFORMATTED.includes('requireRecordAuth("manage"')).toBe(false); + const facts = moduleFacts(REFORMATTED, "r.ts"); + expect(facts.managesRecord).toBe(true); + expect(facts.awaitedAuditActions).toEqual(["thing.delete"]); + }); + + it("matches an aliased and a namespace-qualified resolver call", () => { + const ALIASED = ` + import { requireRecordAuth as resolveRecord } from "@/lib/api-handler"; + import { auditLog as record } from "@/lib/auth/audit"; + + export const DELETE = apiHandler(async () => { + const { user } = await resolveRecord("manage", "labs"); + await record("thing.aliased"); + return apiSuccess({ id: user.id }); + }); + `; + expect(moduleFacts(ALIASED, "a.ts")).toEqual({ + managesRecord: true, + awaitedAuditActions: ["thing.aliased"], + }); + + const NAMESPACED = ` + import * as auth from "@/lib/api-handler"; + + export const DELETE = auth.apiHandler(async () => { + const { user } = await auth.requireRecordAuth("manage", "labs"); + return apiSuccess({ id: user.id }); + }); + `; + expect(moduleFacts(NAMESPACED, "n.ts").managesRecord).toBe(true); + }); + + it("does not enrol a write route that only talks about managing", () => { + const WRITE_ONLY = ` + /** Delegable for writes. It must never take requireRecordAuth("manage"). */ + import { requireRecordAuth } from "@/lib/api-handler"; + + const LEVEL = "manage"; + + export const POST = apiHandler(async () => { + const { user } = await requireRecordAuth("write", "labs"); + return apiSuccess({ id: user.id, level: LEVEL }); + }); + `; + // The raw text carries the exact substring a naive membership test uses. + expect(WRITE_ONLY.includes('requireRecordAuth("manage")')).toBe(true); + expect(moduleFacts(WRITE_ONLY, "wo.ts").managesRecord).toBe(false); + }); + + it("reads only the awaited audit calls", () => { + const MIXED = ` + import { requireRecordAuth } from "@/lib/api-handler"; + import { auditLog } from "@/lib/auth/audit"; + + export const DELETE = apiHandler(async () => { + const { user } = await requireRecordAuth("manage", "labs"); + void auditLog("thing.breadcrumb", {}); + await auditLog("thing.delete", { userId: user.id }); + return apiSuccess({ id: user.id }); + }); + `; + expect(moduleFacts(MIXED, "m.ts").awaitedAuditActions).toEqual([ + "thing.delete", + ]); + }); + + it("still reports the fallback for an action nobody mapped", () => { + // The positive control for the recording translator below. Without it, a + // `verbKeyFor` that always returned the same string would report every + // action as specific and this file would prove nothing. + expect(verbKeyFor("nothing.anybody.mapped").key).toBe(FALLBACK_KEY); + }); +}); + +describe("every MANAGE action has a sentence of its own", () => { + const routes = routeFiles(API); + const manageModules = routes + .map((file) => ({ file, ...moduleFacts(readFileSync(file, "utf8"), file) })) + .filter((m) => m.managesRecord); + const actions = [ + ...new Set(manageModules.flatMap((m) => m.awaitedAuditActions)), + ].sort(); + + it("matches the MANAGE modules, and not zero of them", () => { + // A walk that finds nothing agrees with an inventory that covers nothing. + expect(routes.length).toBeGreaterThan(300); + expect(manageModules.length).toBeGreaterThan(0); + // Named anchors, so a matcher that collapsed to one stray file fails + // rather than clearing the floor. Deleting a reading and purging a dose + // history are the two acts the level exists for and the two an owner most + // needs named. + const found = manageModules.map((m) => relative(process.cwd(), m.file)); + expect(found).toContain("src/app/api/measurements/[id]/route.ts"); + expect(found).toContain( + "src/app/api/medications/[id]/intake/purge/route.ts", + ); + // And the actions themselves were read, not merely looked for. + expect(actions.length).toBeGreaterThan(40); + }); + + it("gives every one of them a specific verb rather than the fallback", () => { + const generic = actions.filter( + (action) => verbKeyFor(action).key === FALLBACK_KEY, + ); + + expect( + generic, + "a MANAGE action with no verb line renders to the owner as the generic " + + "'made a change'. Add a case to activity-verb.ts and a sentence to " + + "all six bundles.", + ).toEqual([]); + }); + + it("resolves every verb key in all six bundles", () => { + const bundles = new Map(LOCALES.map((l) => [l, bundle(l)] as const)); + const missing: string[] = []; + let resolved = 0; + + for (const action of actions) { + const { key } = verbKeyFor(action); + for (const locale of LOCALES) { + const value = leaf(bundles.get(locale)!, key); + if (typeof value === "string" && value.length > 0) resolved += 1; + else missing.push(`${locale}: ${key}`); + } + } + + expect(missing.sort()).toEqual([]); + // Six locales for every action, and the actions were counted above. + expect(resolved).toBe(actions.length * LOCALES.length); + }); + + it("interpolates the actor's name and nothing else, in every locale", () => { + const bundles = new Map(LOCALES.map((l) => [l, bundle(l)] as const)); + const leaky: string[] = []; + let checked = 0; + + for (const action of actions) { + const { key, params } = verbKeyFor(action); + // The call site's own parameters first: a sentence cannot leak a value + // the map never hands it. + if (params.join(",") !== "name") leaky.push(`${key} passes ${params}`); + for (const locale of LOCALES) { + const value = leaf(bundles.get(locale)!, key); + if (typeof value !== "string") continue; + checked += 1; + const found = placeholders(value); + if (found.length !== 1 || found[0] !== "name") { + leaky.push(`${locale}: ${key} interpolates ${found.join(", ")}`); + } + } + } + + expect(leaky.sort()).toEqual([]); + expect(checked).toBe(actions.length * LOCALES.length); + }); +}); + +describe("the retention disclosure does not depend on a race", () => { + /** + * The sentence that bounds attribution — "who entered what is answerable for + * the next N days" — used to appear only if a SECOND query had already + * answered. The revoke dialog reads the grant list; the number came off the + * activity feed. Two independent requests, and whichever lost decided + * whether the owner was told. An owner who clicked quickly, or whose + * activity read failed, ended a manage grant having read one sentence fewer + * than an owner who clicked slowly, with nothing on screen to indicate that + * a sentence was missing. + * + * The number now rides the same payload as the row: if there is a grant to + * revoke, `retentionDays` is already in hand. Deterministic means there is + * no branch left, which is what both legs below check from opposite sides — + * one that the sentence is always composed, one that the second query is + * gone. + */ + const t = (key: string, params?: Record) => + params ? `${key}(${JSON.stringify(params)})` : key; + + it("states the window on every grant that could have written something", () => { + for (const access of ["WRITE", "MANAGE"] as const) { + const body = revokeBody({ t, access, name: "Alex", retentionDays: 90 }); + expect(body).toContain("recordSharing.given.revokeEntriesStay"); + expect(body).toContain( + 'recordSharing.given.revokeAttribution({"days":90})', + ); + } + // READ keeps neither: a read grant never put anything in the record, so + // saying what happens to what they entered would describe an act that + // cannot have occurred. + const read = revokeBody({ + t, + access: "READ", + name: "Alex", + retentionDays: 90, + }); + expect(read).not.toContain("recordSharing.given.revokeAttribution"); + }); + + it("takes the number from the read that produced the row", () => { + const source = readFileSync(GRANTS_GIVEN_CARD, "utf8"); + // Positive control: the matcher can see a hook name in this file at all. + expect(source.match(/useAccountGrants/g)?.length ?? 0).toBeGreaterThan(0); + // And the second query, whose arrival used to decide the copy, is gone. + expect(source).not.toContain("useRecordActivity"); + // The branch it fed is gone with it. A required number cannot be absent, + // so there is nothing left for a caller to omit — which is the difference + // between "the sentence is usually there" and "there is no path without + // it". Both halves are named: the optional marker and the guard. + expect(source).not.toContain("retentionDays?:"); + expect(source).not.toContain("retentionDays !== undefined"); + expect(source.match(/retentionDays: number/g)?.length ?? 0).toBe(1); + }); +}); + +describe("the activity feed carries no audit detail", () => { + /** + * The DTO the owner's feed publishes, read off the route. + * + * A field-level assertion rather than a sentence in a comment, because the + * pressure to widen it is real and constant: every thin sentence in the feed + * looks like it wants the value that would make it specific, and the audit + * row usually has one. `details` is a free-form JSON column written by sixty + * handlers; publishing it would put whatever any of them decided to file in + * front of the record owner, and some of them file a label. + */ + it("publishes five fields, and `details` is not one of them", () => { + const source = readFileSync(ACTIVITY_ROUTE, "utf8"); + const parsed = ts.createSourceFile( + ACTIVITY_ROUTE, + source, + ts.ScriptTarget.Latest, + true, + ts.ScriptKind.TS, + ); + + let members: string[] | null = null; + parsed.forEachChild((node) => { + if (!ts.isInterfaceDeclaration(node)) return; + if (node.name.text !== "RecordActivityEntry") return; + members = node.members + .map((m) => (m.name && ts.isIdentifier(m.name) ? m.name.text : "")) + .filter((n) => n.length > 0) + .sort(); + }); + + // The interface was found at all. `toEqual` against null would report the + // absence as a mismatch of the list, which reads like the wrong failure. + expect( + members, + "RecordActivityEntry not found in the route", + ).not.toBeNull(); + expect(members).toEqual(["accesses", "action", "actor", "createdAt", "id"]); + }); +}); diff --git a/src/app/api/account/grants/route.ts b/src/app/api/account/grants/route.ts index 0d2dc86f8..1a0184f55 100644 --- a/src/app/api/account/grants/route.ts +++ b/src/app/api/account/grants/route.ts @@ -56,6 +56,7 @@ import { } from "@/lib/api-response"; import { auditLog } from "@/lib/auth/audit"; import { prisma } from "@/lib/db"; +import { getAuditLogRetentionDays } from "@/lib/jobs/audit-log-cleanup"; import { annotate } from "@/lib/logging/context"; import { checkRateLimit } from "@/lib/rate-limit"; import { GRANT_PARTY_SELECT, toGrantView } from "@/lib/sharing/grant-view"; @@ -102,6 +103,21 @@ export const GET = apiHandler(async () => { return apiSuccess({ given: given.map((g) => toGrantView(g, g.grantee, now)), received: received.map((g) => toGrantView(g, g.grantor, now)), + // v1.37.0 — the audit window, on the payload that carries the rows. + // + // The revoke dialog tells the owner how long "who entered what" stays + // answerable after the access ends, and it can only say that with the + // number the operator configured. It used to read that number off the + // activity feed, a second and independent request, so whether the sentence + // appeared depended on which of two reads finished first — and an owner + // who clicked quickly ended a manage grant having been told less than one + // who clicked slowly, with nothing on screen to say a sentence was + // missing. Riding it here removes the race by construction: a client + // holding a grant to revoke is holding the window it must disclose. + // + // Same resolver as `/api/account/activity` and as the purge job, so the + // two surfaces cannot state different numbers. + retentionDays: getAuditLogRetentionDays(), }); }); diff --git a/src/components/settings/access/__tests__/access-panel.test.tsx b/src/components/settings/access/__tests__/access-panel.test.tsx index d1ce4bbf9..42147f829 100644 --- a/src/components/settings/access/__tests__/access-panel.test.tsx +++ b/src/components/settings/access/__tests__/access-panel.test.tsx @@ -19,7 +19,19 @@ import type { RecordActivity, } from "@/lib/queries/use-account-grants"; -const grantsRef: { value: GrantList } = { value: { given: [], received: [] } }; +const grantsRef: { value: GrantList } = { value: grantList() }; +/** + * A grant payload, with the audit window every real one carries. + * + * v1.37.0 — `retentionDays` rides `GET /api/account/grants` so the revoke + * dialog can state how long attribution survives without waiting on a second + * request. A fixture that omitted it would be describing a payload the server + * cannot send. + */ +function grantList(partial: Partial = {}): GrantList { + return { given: [], received: [], retentionDays: 365, ...partial }; +} + const activityRef: { value: RecordActivity | undefined } = { value: undefined }; vi.mock("@/lib/queries/use-account-grants", async (importOriginal) => { @@ -75,7 +87,7 @@ const BASE: GrantRow = { describe("who can open my record", () => { it("says so honestly when nobody can", () => { - grantsRef.value = { given: [], received: [] }; + grantsRef.value = grantList(); const html = render(); expect(html).toContain("Nobody else can open your record"); expect(html).not.toContain('data-slot="grants-given-list"'); @@ -86,7 +98,7 @@ describe("who can open my record", () => { // produced. A panel that compared `expiresAt` to the clock would be a // second opinion about somebody's access to a health record, and the two // would disagree first in the dangerous direction. - grantsRef.value = { + grantsRef.value = grantList({ given: [ { ...BASE, id: "a", state: "ACTIVE" }, { ...BASE, id: "p", state: "PENDING", acceptedAt: null }, @@ -98,7 +110,7 @@ describe("who can open my record", () => { }, ], received: [], - }; + }); const html = render(); expect(html).toContain('data-grant-state="ACTIVE"'); expect(html).toContain('data-grant-state="PENDING"'); @@ -109,7 +121,7 @@ describe("who can open my record", () => { // The row IS the consent record. "Who had access, from when to when, and // who ended it" is the question the panel exists to answer, and a list // that dropped the ended rows could not answer it. - grantsRef.value = { + grantsRef.value = grantList({ given: [ { ...BASE, @@ -119,14 +131,14 @@ describe("who can open my record", () => { }, ], received: [], - }; + }); const html = render(); expect(html).toContain('data-grant-state="REVOKED"'); expect(html).toContain("You ended it"); }); it("distinguishes a withdrawal from a handover", () => { - grantsRef.value = { + grantsRef.value = grantList({ given: [ { ...BASE, @@ -136,7 +148,7 @@ describe("who can open my record", () => { }, ], received: [], - }; + }); const html = render(); // Two facts, one column. A trail that conflated them would misread an // ordinary handover as a falling-out. @@ -145,20 +157,20 @@ describe("who can open my record", () => { }); it("offers no way to end a grant that has already ended", () => { - grantsRef.value = { + grantsRef.value = grantList({ given: [{ ...BASE, state: "REVOKED", revokedAt: "2026-02-01T09:00:00Z" }], received: [], - }; + }); const ended = render(); expect(ended).not.toContain('data-slot="grant-revoke"'); - grantsRef.value = { given: [BASE], received: [] }; + grantsRef.value = grantList({ given: [BASE] }); const live = render(); expect(live).toContain('data-slot="grant-revoke"'); }); it("says a live grant has never been used rather than leaving a blank", () => { - grantsRef.value = { given: [{ ...BASE, lastUsedAt: null }], received: [] }; + grantsRef.value = grantList({ given: [{ ...BASE, lastUsedAt: null }] }); const html = render(); expect(html).toContain("never"); }); @@ -166,10 +178,10 @@ describe("who can open my record", () => { describe("records I can open", () => { it("offers acceptance on an invitation and nothing else", () => { - grantsRef.value = { + grantsRef.value = grantList({ given: [], received: [{ ...BASE, state: "PENDING", acceptedAt: null }], - }; + }); const html = render(); expect(html).toContain('data-slot="grant-accept"'); // Nothing to open and nothing to hand back: an invitation confers nothing. @@ -178,7 +190,7 @@ describe("records I can open", () => { }); it("offers opening and handing back once it is active", () => { - grantsRef.value = { given: [], received: [BASE] }; + grantsRef.value = grantList({ received: [BASE] }); const html = render(); expect(html).toContain('data-slot="grant-open"'); expect(html).toContain('data-account-id="acct-1"'); diff --git a/src/components/settings/access/__tests__/grant-access-level.test.tsx b/src/components/settings/access/__tests__/grant-access-level.test.tsx index 3e52124a6..d8ee6185a 100644 --- a/src/components/settings/access/__tests__/grant-access-level.test.tsx +++ b/src/components/settings/access/__tests__/grant-access-level.test.tsx @@ -21,7 +21,18 @@ import { describe, expect, it, vi } from "vitest"; import { I18nProvider } from "@/lib/i18n/context"; import type { GrantList, GrantRow } from "@/lib/queries/use-account-grants"; -const grantsRef: { value: GrantList } = { value: { given: [], received: [] } }; +const grantsRef: { value: GrantList } = { value: grantList() }; +/** + * A grant payload, with the audit window every real one carries. + * + * v1.37.0 — `retentionDays` rides `GET /api/account/grants` so the revoke + * dialog can state how long attribution survives without waiting on a second + * request. A fixture that omitted it would be describing a payload the server + * cannot send. + */ +function grantList(partial: Partial = {}): GrantList { + return { given: [], received: [], retentionDays: 365, ...partial }; +} vi.mock("@/lib/queries/use-account-grants", async (importOriginal) => { const actual = @@ -154,10 +165,10 @@ describe("offering a level", () => { describe("accepting a level", () => { it("states the level and what accepting it means before the button", () => { - grantsRef.value = { + grantsRef.value = grantList({ given: [], received: [{ ...BASE, access: "WRITE" }], - }; + }); const html = render(); expect(html).toContain('data-grant-access="WRITE"'); @@ -170,7 +181,7 @@ describe("accepting a level", () => { }); it("asks a manage invitation for its own, heavier consent", () => { - grantsRef.value = { given: [], received: [{ ...BASE, access: "MANAGE" }] }; + grantsRef.value = grantList({ received: [{ ...BASE, access: "MANAGE" }] }); const html = render(); expect(html).toContain('data-grant-access="MANAGE"'); @@ -188,7 +199,7 @@ describe("accepting a level", () => { }); it("does not ask a read invitation for a write consent", () => { - grantsRef.value = { given: [], received: [{ ...BASE, access: "READ" }] }; + grantsRef.value = grantList({ received: [{ ...BASE, access: "READ" }] }); const html = render(); expect(html).toContain('data-grant-access="READ"'); expect(html).not.toContain('data-slot="grant-write-consent"'); @@ -196,7 +207,7 @@ describe("accepting a level", () => { it("stops saying it once the invitation has been answered", () => { // A sentence about a decision already taken is nagging, not consent. - grantsRef.value = { + grantsRef.value = grantList({ given: [], received: [ { @@ -206,7 +217,7 @@ describe("accepting a level", () => { acceptedAt: "2026-01-02T11:00:00.000Z", }, ], - }; + }); const html = render(); expect(html).not.toContain('data-slot="grant-write-consent"'); }); @@ -254,11 +265,24 @@ describe("ending an access, and what it says about what was entered", () => { expect(body).toContain('revokeAttribution({"days":90})'); }); - it("omits the window rather than inventing one", () => { - // The number is the operator's setting. A compiled-in 365 here would be - // exactly the invented value the sentence exists to avoid. - const body = revokeBody({ t, access: "WRITE", name: "Jo" }); + it("states the operator's window rather than a compiled-in one", () => { + // The number is the operator's setting, so an instance running 90 days + // says 90 and this file would fail on a hardcoded 365. + // + // v1.37.0 replaced the case that used to live here. It asserted that the + // sentence was OMITTED when the number had not arrived, which was true and + // was the defect: the number came off a second query, so whether an owner + // was told how long attribution survives depended on which of two requests + // landed first. The window rides the grant list now — a caller with a row + // to revoke has it — so there is no absent case left to assert. + const body = revokeBody({ + t, + access: "WRITE", + name: "Jo", + retentionDays: 7, + }); expect(body).toContain("recordSharing.given.revokeEntriesStay"); - expect(body).not.toContain("revokeAttribution"); + expect(body).toContain('revokeAttribution({"days":7})'); + expect(body).not.toContain("365"); }); }); diff --git a/src/components/settings/access/grants-given-card.tsx b/src/components/settings/access/grants-given-card.tsx index ae9487fb2..f32e0c5d1 100644 --- a/src/components/settings/access/grants-given-card.tsx +++ b/src/components/settings/access/grants-given-card.tsx @@ -12,7 +12,6 @@ import { useTranslations } from "@/lib/i18n/context"; import { accountLabel } from "@/lib/sharing/account-access-view"; import { useAccountGrants, - useRecordActivity, useRevokeGrant, type GrantRow, } from "@/lib/queries/use-account-grants"; @@ -39,12 +38,6 @@ export function GrantsGivenCard() { const { t } = useTranslations(); const { data, isLoading, isError, refetch } = useAccountGrants(); const revoke = useRevokeGrant(); - // Read for one number: how long the activity view can still say who entered - // what. The activity card on this same page holds the query, so this is the - // cached answer rather than a second request — and when the server has not - // answered yet, the sentence that would need the number is left out instead - // of being written around a guessed 365. - const activity = useRecordActivity(); const given = data?.given ?? []; @@ -95,6 +88,10 @@ export function GrantsGivenCard() { // Only a live grant can be ended. An already-ended row keeps its history // and loses its button, rather than offering an act that would 409. if (grant.state !== "ACTIVE" && grant.state !== "PENDING") return null; + // The row and the window arrive together. A grant that can be revoked came + // out of a resolved payload, so there is no state in which this control + // exists and the number behind its disclosure does not. + if (!data) return null; const name = accountLabel(grant.account); return ( ) => string; access: GrantRow["access"]; name: string; - /** Undefined until the server has said what the window is. */ - retentionDays?: number; + /** The audit window, off the same payload the row came from. */ + retentionDays: number; }): string { const lines = [t("recordSharing.given.revokeBody", { name })]; if (access !== "READ") { lines.push(t("recordSharing.given.revokeEntriesStay")); - if (retentionDays !== undefined) { - lines.push( - t("recordSharing.given.revokeAttribution", { days: retentionDays }), - ); - } + lines.push( + t("recordSharing.given.revokeAttribution", { days: retentionDays }), + ); } return lines.join(" "); } diff --git a/src/lib/openapi/routes/account-sharing.ts b/src/lib/openapi/routes/account-sharing.ts index cb3decceb..41a4d2fb7 100644 --- a/src/lib/openapi/routes/account-sharing.ts +++ b/src/lib/openapi/routes/account-sharing.ts @@ -111,6 +111,12 @@ const grantListResponse = z .object({ given: z.array(grantView), received: z.array(grantView), + retentionDays: z + .number() + .int() + .describe( + "How long the record's activity feed can still attribute an entry to the person who made it, in days. Resolved from the instance's audit retention setting (365 by default, operator-configurable), NOT a constant. It rides this payload so a client ending an access can state the window in the same breath, without a second request whose timing would decide whether the owner was told.", + ), }) .meta({ id: "AccountGrantList", diff --git a/src/lib/queries/use-account-grants.ts b/src/lib/queries/use-account-grants.ts index 7fff2b502..2d7cd6fb9 100644 --- a/src/lib/queries/use-account-grants.ts +++ b/src/lib/queries/use-account-grants.ts @@ -65,6 +65,16 @@ export interface GrantList { given: GrantRow[]; /** Grants offered to this account on somebody else's. */ received: GrantRow[]; + /** + * How long the activity feed can still say who entered what, in days — + * resolved by the server from the same setting the purge job reads. + * + * On this payload rather than fetched beside it, because the revoke dialog + * has to disclose the window and a client holding a grant to revoke is + * already holding the answer. Reading it off a second request made the + * sentence appear or not depending on which read landed first. + */ + retentionDays: number; } export interface RecordActivityEntry { diff --git a/src/lib/record-activity/activity-verb.ts b/src/lib/record-activity/activity-verb.ts index fac5538f5..d4c78eab0 100644 --- a/src/lib/record-activity/activity-verb.ts +++ b/src/lib/record-activity/activity-verb.ts @@ -16,10 +16,34 @@ type Translator = ( * mark a dose taken — and "made a change" for seven different things is a way * of answering nothing. An owner reading this at breakfast wants the verb. * - * The map keeps a line for allergies, family history and a tracked value even - * though a delegate can no longer add any of the three: rows written while - * they were admitted sit in the same table and still have to render as - * something, which is the same reason the fallback below exists. + * The map keeps a line for a tracked value even though a delegate can no + * longer add one: rows written while it was admitted sit in the same table and + * still have to render as something, which is the same reason the fallback + * below exists. + * + * ## v1.37.0 — the MANAGE half + * + * A WRITE delegate can only add, so the generic line was at least the right + * shape of fact. A MANAGE delegate changes and removes entries the owner made + * themselves, and there "somebody made a change in your record" withholds the + * only part that matters: whether a reading was corrected or deleted, whether + * a medication was stopped, whether a whole dose history was purged. The level + * ships under "reconstructable or refused", and a feed that cannot say which + * verb ran reconstructs nothing. + * + * So every action a MANAGE-declared route files has an arm below, and + * `src/__tests__/manage-activity-inventory.test.ts` reads the routes to prove + * it — by CALLING this function, not by matching `case` in its source, so an + * action that quietly falls through to the fallback fails there. + * + * ## What a line may say + * + * The verb and the kind of thing, never the value. Every sentence here takes + * `name` and nothing else, and the inventory guard holds that across all six + * bundles. The activity feed reads out of the audit table, which is not + * encrypted and has its own retention: "changed your weight from 81 to 78" + * would put a health figure in a second store with no rotation story. What + * changed is a question the record itself answers. * * ## Why unknown actions fall back rather than fail * @@ -82,6 +106,156 @@ export function recordActivityVerbLine( // client sent them together. case "measurement.create.batch": return t("recordSharing.activityVerb.measurementCreate", { name }); + + /* ---------------------------------------------------------------- */ + /* v1.37.0 — MANAGE. Everything below can change or remove something */ + /* the owner put there themselves. */ + /* ---------------------------------------------------------------- */ + + case "measurement.update": + return t("recordSharing.activityVerb.measurementUpdate", { name }); + case "measurement.delete": + return t("recordSharing.activityVerb.measurementDelete", { name }); + // Its own line rather than the singular one. "Removed a reading" for a + // bulk delete is the sentence that would let a month of readings go + // missing while the feed reported a tidy-up. + case "measurement.delete.bulk": + return t("recordSharing.activityVerb.measurementDeleteBulk", { name }); + case "measurement.restore": + return t("recordSharing.activityVerb.measurementRestore", { name }); + case "measurementReminder.create": + return t("recordSharing.activityVerb.measurementReminderCreate", { + name, + }); + case "measurementReminder.update": + return t("recordSharing.activityVerb.measurementReminderUpdate", { + name, + }); + case "measurementReminder.delete": + return t("recordSharing.activityVerb.measurementReminderDelete", { + name, + }); + case "measurementReminder.complete": + return t("recordSharing.activityVerb.measurementReminderComplete", { + name, + }); + case "measurementReminder.satisfy": + return t("recordSharing.activityVerb.measurementReminderSatisfy", { + name, + }); + + case "labResult.update": + return t("recordSharing.activityVerb.labResultUpdate", { name }); + case "labResult.delete": + return t("recordSharing.activityVerb.labResultDelete", { name }); + case "labResult.restore": + return t("recordSharing.activityVerb.labResultRestore", { name }); + case "biomarker.update": + return t("recordSharing.activityVerb.biomarkerUpdate", { name }); + case "biomarker.delete": + return t("recordSharing.activityVerb.biomarkerDelete", { name }); + + case "allergy.update": + return t("recordSharing.activityVerb.allergyUpdate", { name }); + case "allergy.delete": + return t("recordSharing.activityVerb.allergyDelete", { name }); + case "family-history.update": + return t("recordSharing.activityVerb.familyHistoryUpdate", { name }); + case "family-history.delete": + return t("recordSharing.activityVerb.familyHistoryDelete", { name }); + + case "illness.episode.update": + return t("recordSharing.activityVerb.illnessEpisodeUpdate", { name }); + case "illness.episode.delete": + return t("recordSharing.activityVerb.illnessEpisodeDelete", { name }); + case "illness.episode.resolve": + return t("recordSharing.activityVerb.illnessEpisodeResolve", { name }); + case "illness.episode.restore": + return t("recordSharing.activityVerb.illnessEpisodeRestore", { name }); + case "illness.day-log.upsert": + return t("recordSharing.activityVerb.illnessDayLogUpsert", { name }); + + case "medication.update": + return t("recordSharing.activityVerb.medicationUpdate", { name }); + case "medication.delete": + return t("recordSharing.activityVerb.medicationDelete", { name }); + case "medication.glp1.update": + return t("recordSharing.activityVerb.medicationGlp1Update", { name }); + case "medication.schedule_revision.created": + return t("recordSharing.activityVerb.medicationScheduleCreate", { name }); + case "medication.schedule_revision.updated": + return t("recordSharing.activityVerb.medicationScheduleUpdate", { name }); + case "medication.schedule_revision.deleted": + return t("recordSharing.activityVerb.medicationScheduleDelete", { name }); + case "medication.inventory.create": + return t("recordSharing.activityVerb.medicationInventoryCreate", { + name, + }); + case "medication.inventory.update": + return t("recordSharing.activityVerb.medicationInventoryUpdate", { + name, + }); + case "medication.inventory.delete": + return t("recordSharing.activityVerb.medicationInventoryDelete", { + name, + }); + case "medication.sideEffect.delete": + return t("recordSharing.activityVerb.medicationSideEffectDelete", { + name, + }); + case "medication.intake.update": + return t("recordSharing.activityVerb.medicationIntakeUpdate", { name }); + case "medication.intake.delete": + return t("recordSharing.activityVerb.medicationIntakeDelete", { name }); + case "medication.intake.bulk_delete": + return t("recordSharing.activityVerb.medicationIntakeBulkDelete", { + name, + }); + // The one that empties a medication's whole history in one request. It + // says so, because "removed several doses" would read as a correction. + case "medication.intake.purge": + return t("recordSharing.activityVerb.medicationIntakePurge", { name }); + case "medication.intake.import.kickoff": + return t("recordSharing.activityVerb.medicationIntakeImport", { name }); + case "medication.intake.history.import.kickoff": + return t("recordSharing.activityVerb.medicationIntakeHistoryImport", { + name, + }); + + case "moodEntry.create": + return t("recordSharing.activityVerb.moodEntryCreate", { name }); + case "moodEntry.update": + return t("recordSharing.activityVerb.moodEntryUpdate", { name }); + case "moodEntry.delete": + return t("recordSharing.activityVerb.moodEntryDelete", { name }); + case "mood.delete.bulk": + return t("recordSharing.activityVerb.moodDeleteBulk", { name }); + case "mood.restore": + return t("recordSharing.activityVerb.moodRestore", { name }); + case "mental-health.create": + return t("recordSharing.activityVerb.mentalHealthCreate", { name }); + + case "cycle.cycle.delete": + return t("recordSharing.activityVerb.cycleDelete", { name }); + case "cycle.day-log.upsert": + return t("recordSharing.activityVerb.cycleDayLogUpsert", { name }); + case "cycle.day-log.update": + return t("recordSharing.activityVerb.cycleDayLogUpdate", { name }); + case "cycle.day-log.delete": + return t("recordSharing.activityVerb.cycleDayLogDelete", { name }); + case "cycle.period.boundary": + return t("recordSharing.activityVerb.cyclePeriodBoundary", { name }); + case "cycle.symptom.custom.create": + return t("recordSharing.activityVerb.cycleSymptomCreate", { name }); + + case "nutrient.water.write": + return t("recordSharing.activityVerb.nutrientWaterWrite", { name }); + + // Not a change to the record, and the one act on this list that takes a + // copy of all of it out of the instance. Named plainly for that reason. + case "health-record.export": + return t("recordSharing.activityVerb.healthRecordExport", { name }); + default: return t("recordSharing.activity.acted", { name }); } From 71ad8d66b6e5a7bf172e66311991a3f10da3b5eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 6 Aug 2026 23:39:39 +0200 Subject: [PATCH 2/9] fix: preserve sharing failure states MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Accepting, revoking and handing back a grant had no failure rendering at all. The request came back refused, or never left the machine, and the panel did nothing: the confirm dialog had already closed, the list did not change, and the only signal was the absence of one. The two refusals that most need a reader were the two that arrived most quietly — an invitation that had lapsed before it was opened, and the last guardian being told they may not hand back a profile that would then have nobody looking after it. Each refusal now renders on the row it belongs to, announced, in content contrast, beside a control that runs the same act again. The sentences are split by what the person does next rather than by status code, so lapsed and withdrawn read differently and the guardian refusal names the way out. A request that never reached the server says so instead of reporting a refusal nobody issued. The failed state is read off the mutation rather than copied into local state from onError: the copy is what goes stale, and the mutation already knows which row it was for. The invitation form keeps every field it had, and only a successful send clears them. --- messages/de.json | 18 +- messages/en.json | 18 +- messages/es.json | 18 +- messages/fr.json | 18 +- messages/it.json | 18 +- messages/pl.json | 18 +- .../sharing-recoverable-errors.test.tsx | 416 ++++++++++++++++++ .../settings/access/grant-action-error.tsx | 143 ++++++ .../settings/access/grant-invite-card.tsx | 11 +- .../settings/access/grants-given-card.tsx | 27 ++ .../settings/access/grants-received-card.tsx | 70 ++- 11 files changed, 765 insertions(+), 10 deletions(-) create mode 100644 src/__tests__/sharing-recoverable-errors.test.tsx create mode 100644 src/components/settings/access/grant-action-error.tsx diff --git a/messages/de.json b/messages/de.json index 50224cb88..028a6958a 100644 --- a/messages/de.json +++ b/messages/de.json @@ -9585,7 +9585,8 @@ "errorFailed": "Die Einladung konnte nicht gesendet werden. Bitte noch einmal versuchen.", "errorScope": "So lassen sich diese Bereiche nicht teilen. Wähle mindestens einen Bereich aus oder teile die gesamte Akte.", "errorManageBrowserOnly": "Verwaltenden Zugriff kannst du nur im Browser vergeben, weil dabei dein zweiter Faktor abgefragt wird.", - "errorStepUp": "Bestätige deinen zweiten Faktor und versuche es erneut." + "errorStepUp": "Bestätige deinen zweiten Faktor und versuche es erneut.", + "errorOffline": "Das hat den Server nicht erreicht. Deine Einladung steht noch hier — prüfe deine Verbindung und versuche es erneut." }, "given": { "title": "Wer meine Akte öffnen kann", @@ -9742,6 +9743,21 @@ "label": "Dokumente", "detail": "Alles, was im Dokumententresor liegt." } + }, + "actionError": { + "retry": "Erneut versuchen", + "offline": "Das hat den Server nicht erreicht. Es hat sich nichts geändert — prüfe deine Verbindung und versuche es erneut.", + "rateLimited": "Zu viele Versuche hintereinander. Warte eine Minute und versuche es erneut.", + "acceptExpired": "Diese Einladung ist abgelaufen. Bitte um eine neue.", + "acceptWithdrawn": "Diese Einladung wurde zurückgezogen.", + "acceptNotPending": "Diese Einladung wurde bereits angenommen.", + "acceptNotFound": "Diese Einladung gibt es nicht mehr.", + "acceptFailed": "Die Einladung konnte nicht angenommen werden.", + "lastGuardian": "Diese Person kümmert sich als Einzige um dieses Profil, deshalb kann der Zugriff noch nicht enden. Füge zuerst eine weitere betreuende Person hinzu oder lösche das Profil.", + "alreadyEnded": "Dieser Zugriff ist bereits beendet.", + "notFound": "Diesen Zugriff gibt es nicht mehr.", + "revokeFailed": "Der Zugriff konnte nicht beendet werden.", + "renounceFailed": "Der Zugriff konnte nicht zurückgegeben werden." } } } diff --git a/messages/en.json b/messages/en.json index 255435074..ce085b6b8 100644 --- a/messages/en.json +++ b/messages/en.json @@ -9585,7 +9585,8 @@ "errorFailed": "The invitation could not be sent. Please try again.", "errorScope": "Those sections cannot be shared like that. Pick at least one section, or share the entire record.", "errorManageBrowserOnly": "Manage access can only be offered in a browser, because it asks for your second factor.", - "errorStepUp": "Confirm your second factor, then try again." + "errorStepUp": "Confirm your second factor, then try again.", + "errorOffline": "That did not reach the server. Your invitation is still here — check your connection and try again." }, "given": { "title": "Who can open my record", @@ -9742,6 +9743,21 @@ "label": "Documents", "detail": "Everything stored in the document vault." } + }, + "actionError": { + "retry": "Try again", + "offline": "That did not reach the server. Nothing has changed — check your connection and try again.", + "rateLimited": "Too many attempts in a row. Wait a minute and try again.", + "acceptExpired": "That invitation has lapsed. Ask for a new one.", + "acceptWithdrawn": "That invitation was withdrawn.", + "acceptNotPending": "That invitation has already been accepted.", + "acceptNotFound": "That invitation is no longer there.", + "acceptFailed": "The invitation could not be accepted.", + "lastGuardian": "This is the only person looking after that profile, so this access cannot end yet. Add another guardian first, or delete the profile.", + "alreadyEnded": "That access has already ended.", + "notFound": "That access is no longer there.", + "revokeFailed": "The access could not be ended.", + "renounceFailed": "The access could not be handed back." } } } diff --git a/messages/es.json b/messages/es.json index a1403afd0..f48a513ca 100644 --- a/messages/es.json +++ b/messages/es.json @@ -9585,7 +9585,8 @@ "errorFailed": "No se pudo enviar la invitación. Inténtalo de nuevo.", "errorScope": "Esas secciones no se pueden compartir así. Elige al menos una sección o comparte todo el historial.", "errorManageBrowserOnly": "El acceso de gestión solo se puede ofrecer desde un navegador, porque pide tu segundo factor.", - "errorStepUp": "Confirma tu segundo factor y vuelve a intentarlo." + "errorStepUp": "Confirma tu segundo factor y vuelve a intentarlo.", + "errorOffline": "Esto no llegó al servidor. Tu invitación sigue aquí: comprueba tu conexión e inténtalo de nuevo." }, "given": { "title": "Quién puede abrir mi historial", @@ -9742,6 +9743,21 @@ "label": "Documentos", "detail": "Todo lo que está guardado en el archivo de documentos." } + }, + "actionError": { + "retry": "Inténtalo de nuevo", + "offline": "Esto no llegó al servidor. No ha cambiado nada: comprueba tu conexión e inténtalo de nuevo.", + "rateLimited": "Demasiados intentos seguidos. Espera un minuto e inténtalo de nuevo.", + "acceptExpired": "Esa invitación ha caducado. Pide una nueva.", + "acceptWithdrawn": "Esa invitación fue retirada.", + "acceptNotPending": "Esa invitación ya se aceptó.", + "acceptNotFound": "Esa invitación ya no existe.", + "acceptFailed": "No se pudo aceptar la invitación.", + "lastGuardian": "Esta es la única persona que cuida de ese perfil, así que este acceso todavía no puede terminar. Añade otra persona responsable primero, o elimina el perfil.", + "alreadyEnded": "Ese acceso ya ha terminado.", + "notFound": "Ese acceso ya no existe.", + "revokeFailed": "No se pudo terminar el acceso.", + "renounceFailed": "No se pudo devolver el acceso." } } } diff --git a/messages/fr.json b/messages/fr.json index 3df592639..4c8d71687 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -9585,7 +9585,8 @@ "errorFailed": "L'invitation n'a pas pu être envoyée. Veuillez réessayer.", "errorScope": "Ces rubriques ne peuvent pas être partagées ainsi. Choisissez au moins une rubrique, ou partagez le dossier complet.", "errorManageBrowserOnly": "Un accès de gestion ne peut être proposé que depuis un navigateur, car il demande votre deuxième facteur.", - "errorStepUp": "Confirmez votre deuxième facteur, puis réessayez." + "errorStepUp": "Confirmez votre deuxième facteur, puis réessayez.", + "errorOffline": "Cela n'a pas atteint le serveur. Votre invitation est toujours là — vérifiez votre connexion et réessayez." }, "given": { "title": "Qui peut ouvrir mon dossier", @@ -9742,6 +9743,21 @@ "label": "Documents", "detail": "Tout ce qui est conservé dans le coffre à documents." } + }, + "actionError": { + "retry": "Réessayer", + "offline": "Cela n'a pas atteint le serveur. Rien n'a changé — vérifiez votre connexion et réessayez.", + "rateLimited": "Trop de tentatives d'affilée. Attendez une minute et réessayez.", + "acceptExpired": "Cette invitation a expiré. Demandez-en une nouvelle.", + "acceptWithdrawn": "Cette invitation a été retirée.", + "acceptNotPending": "Cette invitation a déjà été acceptée.", + "acceptNotFound": "Cette invitation n'existe plus.", + "acceptFailed": "L'invitation n'a pas pu être acceptée.", + "lastGuardian": "Cette personne est la seule à s'occuper de ce profil, donc cet accès ne peut pas encore prendre fin. Ajoutez d'abord une autre personne responsable, ou supprimez le profil.", + "alreadyEnded": "Cet accès a déjà pris fin.", + "notFound": "Cet accès n'existe plus.", + "revokeFailed": "L'accès n'a pas pu être terminé.", + "renounceFailed": "L'accès n'a pas pu être rendu." } } } diff --git a/messages/it.json b/messages/it.json index 9a5f4506e..28297a6e6 100644 --- a/messages/it.json +++ b/messages/it.json @@ -9585,7 +9585,8 @@ "errorFailed": "Non è stato possibile inviare l'invito. Riprova.", "errorScope": "Queste sezioni non si possono condividere così. Scegli almeno una sezione oppure condividi l'intera cartella.", "errorManageBrowserOnly": "L'accesso di gestione si può offrire solo da un browser, perché richiede il tuo secondo fattore.", - "errorStepUp": "Conferma il tuo secondo fattore e riprova." + "errorStepUp": "Conferma il tuo secondo fattore e riprova.", + "errorOffline": "La richiesta non ha raggiunto il server. Il tuo invito è ancora qui: controlla la connessione e riprova." }, "given": { "title": "Chi può aprire la mia cartella", @@ -9742,6 +9743,21 @@ "label": "Documenti", "detail": "Tutto ciò che è conservato nell'archivio documenti." } + }, + "actionError": { + "retry": "Riprova", + "offline": "La richiesta non ha raggiunto il server. Non è cambiato nulla: controlla la connessione e riprova.", + "rateLimited": "Troppi tentativi di fila. Aspetta un minuto e riprova.", + "acceptExpired": "Questo invito è scaduto. Chiedine uno nuovo.", + "acceptWithdrawn": "Questo invito è stato ritirato.", + "acceptNotPending": "Questo invito è già stato accettato.", + "acceptNotFound": "Questo invito non esiste più.", + "acceptFailed": "Non è stato possibile accettare l'invito.", + "lastGuardian": "Questa è l'unica persona che si occupa di quel profilo, quindi questo accesso non può ancora finire. Aggiungi prima un'altra persona responsabile, oppure elimina il profilo.", + "alreadyEnded": "Questo accesso è già terminato.", + "notFound": "Questo accesso non esiste più.", + "revokeFailed": "Non è stato possibile terminare l'accesso.", + "renounceFailed": "Non è stato possibile restituire l'accesso." } } } diff --git a/messages/pl.json b/messages/pl.json index aa0cf4af3..0577eed2d 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -9585,7 +9585,8 @@ "errorFailed": "Nie udało się wysłać zaproszenia. Spróbuj ponownie.", "errorScope": "Tych sekcji nie da się udostępnić w ten sposób. Wybierz co najmniej jedną sekcję albo udostępnij całą kartotekę.", "errorManageBrowserOnly": "Dostęp zarządzający można przyznać tylko w przeglądarce, ponieważ wymaga drugiego składnika logowania.", - "errorStepUp": "Potwierdź drugi składnik logowania i spróbuj ponownie." + "errorStepUp": "Potwierdź drugi składnik logowania i spróbuj ponownie.", + "errorOffline": "To nie dotarło do serwera. Twoje zaproszenie nadal tu jest — sprawdź połączenie i spróbuj ponownie." }, "given": { "title": "Kto może otworzyć moją kartę", @@ -9742,6 +9743,21 @@ "label": "Dokumenty", "detail": "Wszystko, co jest przechowywane w sejfie dokumentów." } + }, + "actionError": { + "retry": "Spróbuj ponownie", + "offline": "To nie dotarło do serwera. Nic się nie zmieniło — sprawdź połączenie i spróbuj ponownie.", + "rateLimited": "Zbyt wiele prób pod rząd. Odczekaj minutę i spróbuj ponownie.", + "acceptExpired": "To zaproszenie wygasło. Poproś o nowe.", + "acceptWithdrawn": "To zaproszenie zostało wycofane.", + "acceptNotPending": "To zaproszenie zostało już przyjęte.", + "acceptNotFound": "Tego zaproszenia już nie ma.", + "acceptFailed": "Nie udało się przyjąć zaproszenia.", + "lastGuardian": "Ta osoba jako jedyna opiekuje się tym profilem, więc ten dostęp nie może się jeszcze zakończyć. Dodaj najpierw kolejną osobę opiekującą się profilem albo usuń profil.", + "alreadyEnded": "Ten dostęp już się zakończył.", + "notFound": "Tego dostępu już nie ma.", + "revokeFailed": "Nie udało się zakończyć dostępu.", + "renounceFailed": "Nie udało się zwrócić dostępu." } } } diff --git a/src/__tests__/sharing-recoverable-errors.test.tsx b/src/__tests__/sharing-recoverable-errors.test.tsx new file mode 100644 index 000000000..881a13555 --- /dev/null +++ b/src/__tests__/sharing-recoverable-errors.test.tsx @@ -0,0 +1,416 @@ +/** + * A refused sharing action says what happened, stays on screen, and can be + * tried again. + * + * ## The state this file exists to prevent + * + * Three of the four mutations on the shared-access panel had no failure + * rendering at all. `accept`, `revoke` and `renounce` fired, the request came + * back 409 or 410 or never left the machine, and the panel did nothing: the + * dialog had already closed, the list did not change, and the only signal was + * the absence of one. The two refusals that matter most were exactly the two + * that arrived silently — an invitation that had lapsed, and the last Guardian + * being told they may not walk away from a profile that would then have nobody + * looking after it. Both are recoverable, and neither was recoverable if + * nobody knew it had happened. + * + * ## Why the failure lives on the mutation and not in a `useState` + * + * A local `useState` set from `onError` is a second copy of a fact TanStack + * already holds, and the copy is the thing that goes stale: a re-render from + * an unrelated invalidation clears it, or a second failure overwrites the + * first row's message onto another row. `isError` / `error` / `variables` + * survive on the mutation until something explicitly resets it, and + * `variables` is what says WHICH row failed. That is also what makes this file + * possible — the panel's failure states are reachable from a static render, + * with no click, because they are props rather than the result of an + * interaction. + * + * ## What a static render can and cannot prove + * + * It proves the alert exists, is announced, is addressed to the right row, + * carries content contrast rather than muted meta, sits beside a retry control + * and leaves the row's own affordances in place. It does NOT prove the retry + * button re-fires the mutation with the same argument — there is no click + * here. That claim is the browser journey's, and the pure resolver below is + * where the mapping from a refusal to a sentence is actually pinned. + */ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +import { renderToStaticMarkup } from "react-dom/server"; +import { describe, expect, it, vi } from "vitest"; + +import { I18nProvider } from "@/lib/i18n/context"; +import { ApiError } from "@/lib/api/api-fetch"; +import type { GrantList, GrantRow } from "@/lib/queries/use-account-grants"; + +const grantsRef: { value: GrantList } = { value: grantList() }; + +function grantList(partial: Partial = {}): GrantList { + return { given: [], received: [], retentionDays: 365, ...partial }; +} + +/** A mutation as the panel sees one: idle unless a case says otherwise. */ +interface FakeMutation { + mutate: ReturnType; + isPending: boolean; + isError: boolean; + error: unknown; + variables: string | undefined; +} + +function idle(): FakeMutation { + return { + mutate: vi.fn(), + isPending: false, + isError: false, + error: null, + variables: undefined, + }; +} + +function failed(error: unknown, grantId: string): FakeMutation { + return { ...idle(), isError: true, error, variables: grantId }; +} + +const acceptRef = { value: idle() }; +const revokeRef = { value: idle() }; +const renounceRef = { value: idle() }; + +vi.mock("@/lib/queries/use-account-grants", async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + useAccountGrants: () => ({ + data: grantsRef.value, + isLoading: false, + isError: false, + refetch: vi.fn(), + }), + useAcceptGrant: () => acceptRef.value, + useRevokeGrant: () => revokeRef.value, + useRenounceGrant: () => renounceRef.value, + }; +}); + +vi.mock("@/hooks/use-account-switch", () => ({ + useAccountSwitch: () => ({ mutate: vi.fn(), isPending: false }), +})); + +import { + grantActionErrorKey, + type GrantAction, +} from "@/components/settings/access/grant-action-error"; +import { inviteErrorKey } from "@/components/settings/access/grant-invite-card"; +import { GrantsGivenCard } from "@/components/settings/access/grants-given-card"; +import { GrantsReceivedCard } from "@/components/settings/access/grants-received-card"; + +function render(node: React.ReactNode): string { + return renderToStaticMarkup( + {node}, + ); +} + +const BASE: GrantRow = { + id: "g1", + account: { id: "acct-1", username: "housemate", displayName: "Jo" }, + access: "MANAGE", + scope: null, + state: "ACTIVE", + invitedAt: "2026-01-02T10:00:00.000Z", + acceptedAt: "2026-01-02T11:00:00.000Z", + expiresAt: null, + lastUsedAt: null, + revokedAt: null, + revokedBy: null, +}; + +const PENDING: GrantRow = { ...BASE, state: "PENDING", acceptedAt: null }; + +function apiError(status: number, errorCode?: string): ApiError { + return new ApiError("refused", status, errorCode ? { errorCode } : undefined); +} + +function reset() { + acceptRef.value = idle(); + revokeRef.value = idle(); + renounceRef.value = idle(); + grantsRef.value = grantList(); +} + +describe("what a refused sharing action says", () => { + const cases: Array<{ + action: GrantAction; + err: unknown; + key: string; + why: string; + }> = [ + { + action: "accept", + err: apiError(410, "sharing.accept.expired"), + key: "recordSharing.actionError.acceptExpired", + why: "the invitation lapsed — the next step is to ask for a new one", + }, + { + action: "accept", + err: apiError(410, "sharing.accept.revoked"), + key: "recordSharing.actionError.acceptWithdrawn", + why: "withdrawn is a different fact from lapsed, and has no next step", + }, + { + action: "accept", + err: apiError(409, "sharing.accept.not_pending"), + key: "recordSharing.actionError.acceptNotPending", + why: "already accepted, most likely in another tab", + }, + { + action: "accept", + err: apiError(404, "sharing.accept.not_found"), + key: "recordSharing.actionError.acceptNotFound", + why: "the row is gone", + }, + { + action: "accept", + err: apiError(429), + key: "recordSharing.actionError.rateLimited", + why: "waiting is the recovery, so it must not read as a failure", + }, + { + action: "revoke", + err: apiError(409, "managed_profile.guardian.required"), + key: "recordSharing.actionError.lastGuardian", + why: "the refusal a managed profile depends on, and it names a way out", + }, + { + action: "renounce", + err: apiError(409, "managed_profile.guardian.required"), + key: "recordSharing.actionError.lastGuardian", + why: "the same refusal reached from the delegate's side", + }, + { + action: "revoke", + err: apiError(409, "sharing.revoke.already_ended"), + key: "recordSharing.actionError.alreadyEnded", + why: "nothing to recover — the access is already closed", + }, + { + action: "renounce", + err: apiError(409, "sharing.renounce.already_ended"), + key: "recordSharing.actionError.alreadyEnded", + why: "same, from the other side", + }, + { + action: "revoke", + err: apiError(404, "sharing.revoke.not_found"), + key: "recordSharing.actionError.notFound", + why: "the row is gone", + }, + { + action: "accept", + err: new TypeError("Failed to fetch"), + key: "recordSharing.actionError.offline", + why: "never reached the server — retrying is the whole recovery", + }, + { + action: "revoke", + err: new TypeError("Failed to fetch"), + key: "recordSharing.actionError.offline", + why: "same, and the row is unchanged because nothing was sent", + }, + { + action: "renounce", + err: new TypeError("Failed to fetch"), + key: "recordSharing.actionError.offline", + why: "same", + }, + { + action: "accept", + err: apiError(500), + key: "recordSharing.actionError.acceptFailed", + why: "an unclassified failure still says which act failed", + }, + { + action: "revoke", + err: apiError(500), + key: "recordSharing.actionError.revokeFailed", + why: "same", + }, + { + action: "renounce", + err: apiError(500), + key: "recordSharing.actionError.renounceFailed", + why: "same", + }, + ]; + + for (const { action, err, key, why } of cases) { + it(`${action}: ${why}`, () => { + expect(grantActionErrorKey(err, action)).toBe(key); + }); + } + + it("tells the refusals apart rather than collapsing them", () => { + // The failure this table is written against is one message for + // everything. Distinct codes must produce distinct sentences, and the + // count is asserted so a resolver that returned one key for all of them + // fails here rather than passing every row above by accident. + const distinct = new Set( + cases.map(({ err, action }) => grantActionErrorKey(err, action)), + ); + expect(distinct.size).toBeGreaterThan(8); + }); +}); + +describe("a refused invitation keeps what was typed", () => { + const INVITE_CARD = readFileSync( + join(process.cwd(), "src/components/settings/access/grant-invite-card.tsx"), + "utf8", + ); + + it("separates a request that never arrived from one the server refused", () => { + // "The invitation could not be sent" invites somebody to change the form, + // and after a dropped connection the form is fine. + expect(inviteErrorKey(new TypeError("Failed to fetch"))).toBe( + "recordSharing.invite.errorOffline", + ); + expect(inviteErrorKey(apiError(404))).toBe( + "recordSharing.invite.errorNoAccount", + ); + expect(inviteErrorKey(apiError(409, "sharing.invite.duplicate"))).toBe( + "recordSharing.invite.errorDuplicate", + ); + }); + + it("clears the form only where the invitation actually went", () => { + // A static render cannot press the button, so the claim is made about the + // code path instead: every field reset lives inside `onSuccess`. A reset + // that migrated into `onError` — or out of the callbacks entirely — would + // wipe a typed identifier and eight ticked sections on a failure the + // person is being asked to retry. + const start = INVITE_CARD.indexOf("onSuccess:"); + const split = INVITE_CARD.indexOf("onError:", start); + // The options object ends at the first ` },` after the error arm, so + // the slice stops at the mutation rather than running into the JSX below, + // where a `setNarrowed(false)` on a radio would satisfy the matcher for + // the wrong reason. + const end = INVITE_CARD.indexOf("\n },", split); + const onSuccess = INVITE_CARD.slice(start, split); + const onError = INVITE_CARD.slice(split, end); + + // Positive controls: both slices really contain the callback they name. + expect(onSuccess).toContain("setInvited"); + expect(onError).toContain("inviteErrorKey"); + + for (const reset of [ + 'setIdentifier("")', + 'setExpiresOn("")', + 'setAccess("READ")', + "setNarrowed(false)", + "setSections([])", + ]) { + expect(onSuccess, reset).toContain(reset); + expect(onError, reset).not.toContain(reset); + } + }); +}); + +describe("the refusal stays on screen, addressed to its own row", () => { + it("announces a refused acceptance and leaves the invitation acceptable", () => { + reset(); + grantsRef.value = grantList({ received: [PENDING] }); + acceptRef.value = failed(apiError(410, "sharing.accept.expired"), "g1"); + + const html = render(); + + // Announced, not merely printed. + expect(html).toContain('data-slot="grant-action-error"'); + expect(html).toContain('role="alert"'); + expect(html).toContain("That invitation has lapsed"); + // Addressed to the row it belongs to, so a second invitation below it + // does not wear somebody else's failure. + expect(html).toContain('data-grant-error-for="g1"'); + // The state survives: the row is still there and still offers the act. + expect(html).toContain('data-grant-id="g1"'); + expect(html).toContain('data-slot="grant-accept"'); + // And there is a way to try again. + expect(html).toContain('data-slot="grant-action-retry"'); + }); + + it("does not paint the failure onto a row that did not fail", () => { + reset(); + grantsRef.value = grantList({ + received: [PENDING, { ...PENDING, id: "g2" }], + }); + acceptRef.value = failed(apiError(410, "sharing.accept.expired"), "g1"); + + const html = render(); + + expect(html.match(/data-slot="grant-action-error"/g)).toHaveLength(1); + expect(html).toContain('data-grant-error-for="g1"'); + expect(html).not.toContain('data-grant-error-for="g2"'); + }); + + it("names the last-Guardian refusal and the way out of it", () => { + reset(); + grantsRef.value = grantList({ given: [BASE] }); + revokeRef.value = failed( + apiError(409, "managed_profile.guardian.required"), + "g1", + ); + + const html = render(); + + expect(html).toContain('data-slot="grant-action-error"'); + // The sentence has to carry the recovery. "Could not end access" is true + // and useless; the owner needs to know a second guardian is the fix. + expect(html).toContain("Add another guardian first"); + // The row keeps its control, so the owner can act on the advice and + // return to the same button. + expect(html).toContain('data-slot="grant-revoke"'); + }); + + it("reaches the same refusal from the delegate's side", () => { + reset(); + grantsRef.value = grantList({ received: [BASE] }); + renounceRef.value = failed( + apiError(409, "managed_profile.guardian.required"), + "g1", + ); + + const html = render(); + + expect(html).toContain('data-slot="grant-action-error"'); + expect(html).toContain("Add another guardian first"); + expect(html).toContain('data-slot="grant-renounce"'); + }); + + it("keeps material copy in content contrast, not muted meta", () => { + reset(); + grantsRef.value = grantList({ received: [PENDING] }); + acceptRef.value = failed(new TypeError("Failed to fetch"), "g1"); + + const html = render(); + const alert = /<[^>]*data-slot="grant-action-error"[^>]*>/.exec(html)?.[0]; + + expect(alert).toBeDefined(); + // A refusal somebody has to act on is content. UI-STANDARDS §3 reserves + // muted for meta, and a sentence nobody reads is a sentence nobody acts + // on. + expect(alert).toContain("text-destructive"); + expect(alert).not.toContain("text-muted-foreground"); + }); + + it("shows nothing at all while every mutation is idle", () => { + reset(); + grantsRef.value = grantList({ given: [BASE], received: [PENDING] }); + + expect(render()).not.toContain( + 'data-slot="grant-action-error"', + ); + expect(render()).not.toContain( + 'data-slot="grant-action-error"', + ); + }); +}); diff --git a/src/components/settings/access/grant-action-error.tsx b/src/components/settings/access/grant-action-error.tsx new file mode 100644 index 000000000..62e95682f --- /dev/null +++ b/src/components/settings/access/grant-action-error.tsx @@ -0,0 +1,143 @@ +"use client"; + +import { Button } from "@/components/ui/button"; +import { ApiError } from "@/lib/api/api-fetch"; +import { useTranslations } from "@/lib/i18n/context"; + +/** + * v1.37.0 — what a refused accept, revoke or renounce says, and where it says + * it. + * + * ## The state this replaces + * + * These three mutations had no failure rendering at all. They fired, the + * server answered 409 or 410, or the request never left the machine, and the + * panel did nothing — the confirm dialog had already closed, the list did not + * change, and the only signal was the absence of one. The two refusals that + * matter most were the two that arrived most quietly: an invitation that had + * lapsed, and the last Guardian being refused a renunciation that would leave + * a managed profile with nobody looking after it. Both are recoverable, and + * neither is recoverable by somebody who was not told. + * + * ## Every arm names its own recovery + * + * A refusal is only worth rendering if it changes what the person does next, + * so the sentences are split by what the next step is rather than by status + * code. "Lapsed" leads to asking for a new invitation; "withdrawn" leads + * nowhere and says so; the last-Guardian refusal names the two ways out the + * lifecycle actually offers. The offline arm is the one where retrying IS the + * recovery, so it is separated from a server refusal even though both surface + * as a failed mutation: a request that never left the machine changed nothing, + * and telling somebody their access "could not be ended" when it was never + * asked is a different and worse sentence. + * + * `ApiError` carries the server's stable `meta.errorCode`; anything that is + * not an `ApiError` never reached a response at all, which is what makes the + * offline test a type check rather than a status check. + */ +export type GrantAction = "accept" | "revoke" | "renounce"; + +/** The generic sentence for each act, when nothing more specific applies. */ +const FAILED_KEY: Record = { + accept: "recordSharing.actionError.acceptFailed", + revoke: "recordSharing.actionError.revokeFailed", + renounce: "recordSharing.actionError.renounceFailed", +}; + +/** + * The message key for a refused grant action. + * + * Pure and exported so the mapping can be pinned without a click: the panel's + * error states are otherwise only reachable by driving a browser, and a + * mapping nobody can test is a mapping that drifts. + */ +export function grantActionErrorKey(err: unknown, action: GrantAction): string { + // Not an `ApiError` means no response came back — a dropped connection, a + // sleeping laptop, a server that is not there. Nothing was decided, so the + // sentence says so and points at the retry rather than reporting a refusal + // the server never issued. + if (!(err instanceof ApiError)) return "recordSharing.actionError.offline"; + + const code = + typeof err.meta?.errorCode === "string" ? err.meta.errorCode : null; + + switch (code) { + // The one refusal that protects a person rather than a row. It reaches + // both sides — the owner withdrawing the last Guardian's access and the + // last Guardian handing it back — and it is the same fact either way, so + // it is the same sentence. + case "managed_profile.guardian.required": + return "recordSharing.actionError.lastGuardian"; + case "sharing.accept.expired": + return "recordSharing.actionError.acceptExpired"; + case "sharing.accept.revoked": + return "recordSharing.actionError.acceptWithdrawn"; + case "sharing.accept.not_pending": + return "recordSharing.actionError.acceptNotPending"; + case "sharing.accept.not_found": + return "recordSharing.actionError.acceptNotFound"; + case "sharing.revoke.already_ended": + case "sharing.renounce.already_ended": + return "recordSharing.actionError.alreadyEnded"; + case "sharing.revoke.not_found": + case "sharing.renounce.not_found": + return "recordSharing.actionError.notFound"; + default: + break; + } + + // Waiting is the recovery, so it must not read as a failure. The accept + // route rate-limits without an error code, hence the status. + if (err.status === 429) return "recordSharing.actionError.rateLimited"; + + return FAILED_KEY[action]; +} + +/** + * The refusal, on the row it belongs to. + * + * `role="alert"` because it appears after the act rather than with the page: + * a person who pressed a button and got nothing has already looked away, and + * a screen reader that does not announce it is the same silence the panel had + * before. `text-destructive` because this is content somebody has to act on — + * UI-STANDARDS §3 reserves muted for meta, and a refusal rendered as meta is a + * refusal nobody reads. + * + * `data-grant-error-for` carries the row id so a panel holding two failed rows + * cannot show one row's message against another's, and so a browser journey + * can assert which row was refused rather than that something somewhere went + * red. + */ +export function GrantActionAlert({ + grantId, + message, + onRetry, + retrying = false, +}: { + grantId: string; + message: string; + onRetry: () => void; + retrying?: boolean; +}) { + const { t } = useTranslations(); + return ( +
+ {message} + +
+ ); +} diff --git a/src/components/settings/access/grant-invite-card.tsx b/src/components/settings/access/grant-invite-card.tsx index be3a92285..9c34a61b3 100644 --- a/src/components/settings/access/grant-invite-card.tsx +++ b/src/components/settings/access/grant-invite-card.tsx @@ -654,10 +654,15 @@ function useInvitationOwnerTimezone(): string { * field" and "you already have your own record". */ export function inviteErrorKey(err: unknown): string { + // Nothing that is not an `ApiError` ever reached a response, so no + // invitation was refused — the request did not arrive. Said as its own case + // because "the invitation could not be sent" invites somebody to change the + // form, and the form is fine. The typed identifier, the level and the ticked + // sections all survive a failed mutation, so the retry is the same button. + if (!(err instanceof ApiError)) return "recordSharing.invite.errorOffline"; + const code = - err instanceof ApiError && typeof err.meta?.errorCode === "string" - ? err.meta.errorCode - : null; + typeof err.meta?.errorCode === "string" ? err.meta.errorCode : null; switch (code) { case "sharing.invite.self": return "recordSharing.invite.errorSelf"; diff --git a/src/components/settings/access/grants-given-card.tsx b/src/components/settings/access/grants-given-card.tsx index f32e0c5d1..f90b87db0 100644 --- a/src/components/settings/access/grants-given-card.tsx +++ b/src/components/settings/access/grants-given-card.tsx @@ -2,6 +2,10 @@ import { Loader2, Users } from "lucide-react"; +import { + GrantActionAlert, + grantActionErrorKey, +} from "@/components/settings/access/grant-action-error"; import { GrantRowItem } from "@/components/settings/access/grant-row"; import { SettingsCard } from "@/components/settings/settings-card"; import { SettingsCardHeader } from "@/components/settings/_card-header"; @@ -75,6 +79,7 @@ export function GrantsGivenCard() { key={grant.id} grant={grant} side="given" + notice={renderFailure(grant)} actions={renderRevoke(grant)} /> ))} @@ -84,6 +89,28 @@ export function GrantsGivenCard() { ); + /** + * A refused revoke, on the row it was refused for. + * + * The confirm dialog has already closed by the time the server answers, so + * without this a refusal is invisible: the list looks the same, and the one + * refusal that matters — a managed profile that would be left with nobody + * looking after it — arrived as silence. Read off the mutation rather than + * copied into local state, so `variables` is what decides which row wears + * it. + */ + function renderFailure(grant: GrantRow) { + if (!revoke.isError || revoke.variables !== grant.id) return null; + return ( + revoke.mutate(grant.id)} + /> + ); + } + function renderRevoke(grant: GrantRow) { // Only a live grant can be ended. An already-ended row keeps its history // and loses its button, rather than offering an act that would 409. diff --git a/src/components/settings/access/grants-received-card.tsx b/src/components/settings/access/grants-received-card.tsx index 4ed9eb633..dd7f581b5 100644 --- a/src/components/settings/access/grants-received-card.tsx +++ b/src/components/settings/access/grants-received-card.tsx @@ -2,6 +2,11 @@ import { FolderOpen, Loader2 } from "lucide-react"; +import { + GrantActionAlert, + grantActionErrorKey, + type GrantAction, +} from "@/components/settings/access/grant-action-error"; import { GrantRowItem } from "@/components/settings/access/grant-row"; import { SettingsCard } from "@/components/settings/settings-card"; import { SettingsCardHeader } from "@/components/settings/_card-header"; @@ -34,6 +39,18 @@ import { * at the row and should not have to go hunting for the switcher. * * **Hand back.** The delegate's own way out, attributed as theirs so the * owner's record can tell "I withdrew it" from "he gave it back". + * + * v1.37.0 — both acts can now be refused in ways the person can do something + * about, and both used to fail in silence. An invitation that lapsed before it + * was opened, and the last Guardian being told they may not hand back a + * profile that would then have nobody looking after it. The refusal renders on + * its own row, announced, with the row's control still there. + * + * The failed state is read off the mutation rather than copied into a + * `useState` from `onError`. TanStack keeps `isError` / `error` / `variables` + * until something resets it, and `variables` is what says which row failed — a + * local copy would be a second version of that fact, and it is the copy that + * goes stale when an unrelated invalidation re-renders the panel. */ export function GrantsReceivedCard() { const { t } = useTranslations(); @@ -78,7 +95,12 @@ export function GrantsReceivedCard() { key={grant.id} grant={grant} side="received" - notice={renderConsent(grant)} + notice={ + <> + {renderConsent(grant)} + {renderFailure(grant)} + + } actions={renderActions(grant)} /> ))} @@ -88,6 +110,52 @@ export function GrantsReceivedCard() { ); + /** + * The refusal this row is carrying, if it is the row that was refused. + * + * Guarded on `variables` rather than rendered under the list, because a + * panel with two pending invitations would otherwise show one row's failure + * with no way to tell which act it belonged to. + */ + function renderFailure(grant: GrantRow) { + const failure = failedAction(grant.id); + if (!failure) return null; + return ( + + ); + } + + /** Which of the two acts failed on this row, and how to run it again. */ + function failedAction(grantId: string): { + action: GrantAction; + error: unknown; + pending: boolean; + retry: () => void; + } | null { + if (accept.isError && accept.variables === grantId) { + return { + action: "accept", + error: accept.error, + pending: accept.isPending, + retry: () => accept.mutate(grantId), + }; + } + if (renounce.isError && renounce.variables === grantId) { + return { + action: "renounce", + error: renounce.error, + pending: renounce.isPending, + retry: () => renounce.mutate(grantId), + }; + } + return null; + } + /** * The delegate's half of the consent, on the screen where they give it. * From 0cdfb97a451fd14047b83d1e560600e6200a0921 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 6 Aug 2026 23:46:43 +0200 Subject: [PATCH 3/9] test: cover sharing accessibility states MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shared-record accessibility suite had four tests for five states: success and empty shared one, and neither of them ever entered an empty state. A browser run reports what it ran and never what it did not, so a state nobody wrote a test for produced exactly the same output as a state that passed. The five states are enumerated in a fixture, each test names its state, and a fast guard reads the spec back against the list — a state that loses its test, its anchor, or one of its proofs now fails in the ordinary gate rather than at the point somebody notices the spinner is silent. Every state runs four proofs: an axe scan, the document reading order, either a keyboard reach with a visible focus indicator or a live-region announcement for the one state with nothing to press, and, where the copy is something a person has to act on, that it renders as content rather than as muted meta and clears the contrast floor. The retry control on the read-failure card gained a stable slot so the keyboard proof targets the affordance rather than its wording. The eight release journeys are enumerated in the same fixture. --- ...v137-sharing-managed-profiles-a11y.spec.ts | 280 +++++++++++++++++- .../sharing-accessibility-states.test.tsx | 230 ++++++++++++++ src/components/ui/query-error-card.tsx | 11 +- tests/fixtures/v137/e2e-journeys.ts | 176 +++++++++++ 4 files changed, 686 insertions(+), 11 deletions(-) create mode 100644 src/__tests__/sharing-accessibility-states.test.tsx create mode 100644 tests/fixtures/v137/e2e-journeys.ts diff --git a/e2e/v137-sharing-managed-profiles-a11y.spec.ts b/e2e/v137-sharing-managed-profiles-a11y.spec.ts index 6396eba29..87c3e3128 100644 --- a/e2e/v137-sharing-managed-profiles-a11y.spec.ts +++ b/e2e/v137-sharing-managed-profiles-a11y.spec.ts @@ -4,13 +4,190 @@ import type { Page } from "@playwright/test"; import { expect, test } from "./setup/test"; import { SCOPE_A11Y_STORAGE_STATE_PATH } from "./setup/test-helpers"; -async function scan(page: Page) { +/** + * The five states of a shared record, each proved usable rather than merely + * reachable. + * + * Accessibility work concentrates on the state where everything worked, + * because that is the state the person building the screen is looking at. The + * other four are where somebody who cannot see the screen is most stranded: a + * spinner nobody announces, an error card that reads as an empty page, a + * refusal whose only way out is a control no keyboard reaches. This file had + * four tests for five states, success and empty shared one, and neither of + * them entered an empty state at all. + * + * Each test now names its state in brackets, enters it, and runs four proofs: + * an axe scan, the reading order, either a keyboard reach or an announcement, + * and — where the state has copy somebody has to act on — that the copy is + * content rather than muted meta. + * + * `src/__tests__/sharing-accessibility-states.test.tsx` reads this file back + * against `tests/fixtures/v137/e2e-journeys.ts` and fails if a state loses its + * test or one of its proofs. A browser suite reports what it ran, never what + * it did not, so the enumeration lives where a fast test can check it. + */ + +/** WCAG 2.1 AA, as the rest of the suite scans it. */ +async function scanForViolations(page: Page) { const result = await new AxeBuilder({ page }) .withTags(["wcag2a", "wcag2aa", "wcag21a", "wcag21aa"]) .analyze(); expect(result.violations).toEqual([]); } +/** + * The named elements appear in this order in the document. + * + * Reading order is what a screen reader follows and what a keyboard walks, and + * CSS can put a control visually above an explanation while the DOM keeps it + * below. Every selector must resolve: an absent element would otherwise make + * an order of one trivially correct, which is the shape of check that passes + * for the whole life of a regression. + */ +async function expectReaderOrder(page: Page, selectors: string[]) { + expect(selectors.length).toBeGreaterThan(1); + const positions = await page.evaluate((list) => { + const all = Array.from(document.querySelectorAll("*")); + return list.map((selector) => { + const element = document.querySelector(selector); + return element ? all.indexOf(element) : -1; + }); + }, selectors); + + const missing = selectors.filter((_, i) => positions[i] === -1); + expect(missing, `not on the page: ${missing.join(", ")}`).toEqual([]); + expect(positions, `reading order is ${selectors.join(" then ")}`).toEqual( + [...positions].sort((a, b) => a - b), + ); +} + +/** + * Tab from the top of the document until focus lands on the target. + * + * Real Tab presses rather than `element.focus()`, for two reasons: a control + * that is present but not in the tab order fails here, which is the defect + * this proves the absence of; and `:focus-visible` only applies to keyboard + * focus, so the ring can be asserted at the same time. The invitation form's + * own radios shipped `sr-only` inputs whose focus ring landed on a zero-size + * box — the whole class of defect is invisible to `element.focus()`. + */ +async function expectKeyboardReach(page: Page, selector: string) { + await expect(page.locator(selector).first()).toBeVisible(); + await page.evaluate(() => { + (document.activeElement as HTMLElement | null)?.blur(); + document.body.focus(); + }); + + const MAX_TABS = 120; + let reached = false; + for (let i = 0; i < MAX_TABS && !reached; i += 1) { + await page.keyboard.press("Tab"); + reached = await page.evaluate( + (sel) => document.activeElement?.matches(sel) ?? false, + selector, + ); + } + expect(reached, `${selector} is not reachable by Tab`).toBe(true); + + // And the person can see where they are. A focusable control with no + // visible indicator is WCAG 2.4.7 and is exactly what an `sr-only` input + // produces. + const outlined = await page.evaluate((sel) => { + const element = document.querySelector(sel); + if (!element) return false; + if (element.matches(":focus-visible")) return true; + // Some rings are painted by an ancestor via `has-[:focus-visible]`. + return Boolean(element.closest(":has(:focus-visible)")); + }, selector); + expect(outlined, `${selector} takes focus without showing it`).toBe(true); +} + +/** + * The state says out loud that it is happening. + * + * For the one state with nothing to press. A spinner is a picture; without a + * live region and a label it is a blank page to a screen reader, and the hold + * before the active record resolves is precisely when somebody must not be + * told nothing. + */ +async function expectAnnounced(page: Page, selector: string) { + const region = page.locator(selector).first(); + await expect(region).toBeVisible(); + await expect(region).toHaveAttribute("role", /status|alert/); + const label = await region.evaluate((node) => + (node.textContent ?? "").trim(), + ); + expect(label.length, `${selector} announces nothing`).toBeGreaterThan(0); +} + +/** + * Copy somebody has to act on is content, not meta. + * + * Two halves, and both are needed. The class check is the design standard — + * UI-STANDARDS §3 reserves `text-muted-foreground` for meta, and consent or + * safety text rendered as meta is text people skip. The ratio check is the + * measurable one: it walks up for an opaque background and asserts the AA + * floor for body text, so a future token change that keeps the class and + * loses the contrast still fails. + */ +async function expectContentContrast(page: Page, selector: string) { + const result = await page.evaluate((sel) => { + const element = document.querySelector(sel); + if (!element) return null; + + const parse = (value: string): [number, number, number, number] | null => { + const nums = value.match(/[\d.]+/g); + if (!nums || nums.length < 3) return null; + return [ + Number(nums[0]), + Number(nums[1]), + Number(nums[2]), + nums.length > 3 ? Number(nums[3]) : 1, + ]; + }; + + const luminance = ([r, g, b]: [number, number, number, number]): number => { + const channel = (c: number) => { + const v = c / 255; + return v <= 0.03928 ? v / 12.92 : ((v + 0.055) / 1.055) ** 2.4; + }; + return 0.2126 * channel(r) + 0.7152 * channel(g) + 0.0722 * channel(b); + }; + + const foreground = parse(getComputedStyle(element).color); + let node: Element | null = element; + let background: [number, number, number, number] | null = null; + while (node && !background) { + const parsed = parse(getComputedStyle(node).backgroundColor); + if (parsed && parsed[3] > 0) background = parsed; + node = node.parentElement; + } + if (!foreground || !background) return null; + + const light = Math.max(luminance(foreground), luminance(background)); + const dark = Math.min(luminance(foreground), luminance(background)); + + // The class list of the element and of any descendant carrying the text: + // the copy is sometimes a wrapper's child. + const classes = [element, ...Array.from(element.querySelectorAll("*"))] + .map((n) => n.className) + .filter((c): c is string => typeof c === "string") + .join(" "); + + return { ratio: (light + 0.05) / (dark + 0.05), classes }; + }, selector); + + expect(result, `${selector} is not on the page`).not.toBeNull(); + expect( + result!.classes, + `${selector} renders material copy in the muted token`, + ).not.toContain("text-muted-foreground"); + expect( + result!.ratio, + `${selector} is below the AA floor for body text`, + ).toBeGreaterThanOrEqual(4.5); +} + async function openProfileRecord(page: Page) { await page.goto("/"); await page.getByRole("button", { name: "User menu" }).first().click(); @@ -34,30 +211,81 @@ test.describe("shared record accessibility states", () => { test.describe.configure({ mode: "serial" }); test.use({ storageState: SCOPE_A11Y_STORAGE_STATE_PATH }); - test("success and empty shared profile states have no WCAG violations", async ({ + test("[success] the shared profile is scannable, ordered and escapable", async ({ page, }) => { await openProfileRecord(page); await expect(page.locator('[data-slot="profile-summary"]')).toBeVisible(); - await scan(page); + + await scanForViolations(page); + // Whose record this is comes before what is in it. A reader who meets the + // record before the banner has already read somebody else's health data + // believing it is their own. + await expectReaderOrder(page, [ + '[data-slot="shared-record-banner"]', + '[data-slot="shared-record-banner-context"]', + '[data-slot="profile-summary"]', + ]); + await expectContentContrast( + page, + '[data-slot="shared-record-banner-context"]', + ); + await expectKeyboardReach(page, '[data-slot="shared-record-banner-exit"]'); + await leaveRecord(page); }); - test("the capability hydration loading state has no WCAG violations", async ({ + test("[empty] nothing shared yet reads as an answer, not a failure", async ({ + page, + }) => { + // The delegate's own shared-access panel. They hold grants and have given + // none, so the "who can open my record" card is genuinely empty — which is + // a state the suite claimed to cover and never entered. + await page.goto("/settings/access"); + await expect( + page.locator('[data-slot="grants-given-card"] [data-slot="empty-state"]'), + ).toBeVisible(); + + await scanForViolations(page); + await expectReaderOrder(page, [ + '[data-slot="grant-invite-card"]', + '[data-slot="grants-given-card"]', + '[data-slot="grants-given-card"] [data-slot="empty-state"]', + ]); + await expectContentContrast( + page, + '[data-slot="grants-given-card"] [data-slot="empty-state"] p', + ); + // The way out of an empty state is the act that fills it. + await expectKeyboardReach(page, '[data-slot="grant-invite-identifier"]'); + }); + + test("[loading] the hold before the active record is known is announced", async ({ page, }) => { await page.route("**/api/auth/me", async (route) => { - await new Promise((resolve) => setTimeout(resolve, 250)); + await new Promise((resolve) => setTimeout(resolve, 1500)); await route.continue(); }); await page.goto("/measurements"); await expect( page.locator('[data-slot="record-scope-hydration-gate"]'), ).toBeVisible(); - await scan(page); + + await scanForViolations(page); + await expectAnnounced(page, '[data-slot="record-scope-hydration-gate"]'); + // The label lives inside the region, so a reader that reaches the region + // reaches the sentence. A label rendered as a sibling would be announced + // separately or not at all. + await expectReaderOrder(page, [ + '[data-slot="record-scope-hydration-gate"]', + '[data-slot="record-scope-hydration-gate"] .sr-only', + ]); + + await page.unroute("**/api/auth/me"); }); - test("the shared profile read error state has no WCAG violations", async ({ + test("[error] a failed read says so and hands the retry to a keyboard", async ({ page, }) => { await page.route("**/api/profile/summary**", (route) => @@ -65,11 +293,23 @@ test.describe("shared record accessibility states", () => { ); await openProfileRecord(page); await expect(page.locator('[data-slot="query-error-card"]')).toBeVisible(); - await scan(page); + + await scanForViolations(page); + await expectReaderOrder(page, [ + '[data-slot="shared-record-banner"]', + '[data-slot="query-error-card"]', + '[data-slot="query-error-retry"]', + ]); + // The sentence that says the read failed. Muted here would put "this did + // not load" in the same register as a timestamp. + await expectContentContrast(page, '[data-slot="query-error-card"] p'); + await expectKeyboardReach(page, '[data-slot="query-error-retry"]'); + + await page.unroute("**/api/profile/summary**"); await leaveRecord(page); }); - test("the shared-record refusal state has no WCAG violations", async ({ + test("[refusal] a surface sharing does not cover keeps a way back", async ({ page, }) => { await openProfileRecord(page); @@ -77,7 +317,27 @@ test.describe("shared record accessibility states", () => { await expect( page.locator('[data-slot="shared-record-unavailable"]'), ).toBeVisible(); - await scan(page); + + await scanForViolations(page); + // The explanation before the button. A reader who meets "leave this + // record" first is being offered an act before being told why. + await expectReaderOrder(page, [ + '[data-slot="shared-record-unavailable"]', + '[data-slot="shared-record-unavailable"] p', + '[data-slot="shared-record-unavailable-leave"]', + ]); + await expectContentContrast( + page, + '[data-slot="shared-record-unavailable"] p', + ); + await expectKeyboardReach( + page, + '[data-slot="shared-record-unavailable-leave"]', + ); + await page.locator('[data-slot="shared-record-unavailable-leave"]').click(); + await expect( + page.locator('[data-slot="shared-record-banner"]'), + ).toHaveCount(0); }); }); diff --git a/src/__tests__/sharing-accessibility-states.test.tsx b/src/__tests__/sharing-accessibility-states.test.tsx new file mode 100644 index 000000000..907bd1a0a --- /dev/null +++ b/src/__tests__/sharing-accessibility-states.test.tsx @@ -0,0 +1,230 @@ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +import { describe, expect, it } from "vitest"; + +import { + ACCESSIBILITY_STATES, + SHARING_ACCESSIBILITY_STATES, + V137_E2E_JOURNEYS, +} from "../../tests/fixtures/v137/e2e-journeys"; + +/** + * All five states are claimed by a browser test that really examines them. + * + * ## Why a fast test guards a browser suite + * + * A Playwright run reports what it ran, never what it did not. "The + * accessibility suite is green" and "the accessibility suite covers the error + * state" are different sentences, and only the first one is ever checked — + * a state nobody wrote a test for produces exactly the same output as a state + * that passes. Before this file the suite had four tests for five states, with + * success and empty sharing one and neither of them actually entering an empty + * one. + * + * So the states are enumerated in `tests/fixtures/v137/e2e-journeys.ts` and + * this file reads the spec back against the list. It cannot tell whether the + * assertions inside a test are any good. It can tell that each state has a + * test, that the test enters the state (there is an anchor, and the anchor is + * a slot the component tree really renders), and that the test performs each + * proof the state's own record says it needs — scan, reader order, and either + * a keyboard reach or an announcement. + * + * ## The trap this file is written around + * + * Every assertion here is over a MATCHED set, and an empty match set satisfies + * most of them silently: a spec path that no longer exists reads as a spec + * with no offending tests. Each leg therefore pins the size of what it + * matched — five states, five titles, eight journeys, a non-zero count of + * helper calls — and the parse is proved against a synthetic spec at the + * bottom, so the day the extractor stops seeing `test("…")` is the day this + * file fails rather than the day it goes quiet. + */ +const ROOT = process.cwd(); +const A11Y_SPEC = join(ROOT, "e2e/v137-sharing-managed-profiles-a11y.spec.ts"); + +/** The helper each state's test has to call, by what the state offers. */ +const SCAN = "scanForViolations"; +const READER_ORDER = "expectReaderOrder"; +const KEYBOARD = "expectKeyboardReach"; +const ANNOUNCED = "expectAnnounced"; +const CONTRAST = "expectContentContrast"; + +/** + * Split a spec into `title -> body`. + * + * Bodies are delimited by the next `test(` rather than by brace matching: a + * body containing a string with a brace in it is common, a body containing the + * literal `test(` is not. Exported and exercised against a synthetic spec + * below, because a splitter that returned an empty map would make every leg + * here pass by matching nothing. + */ +export function testBodies(source: string): Map { + const bodies = new Map(); + // `test.only(` and `test.skip(` are tests; `test.describe(` is not, and the + // synthetic spec below is what caught the first version treating it as one. + const opener = /\btest(?:\.(?!describe)\w+)*\(\s*"([^"]+)"/g; + const starts: Array<{ title: string; from: number }> = []; + let match: RegExpExecArray | null; + while ((match = opener.exec(source)) !== null) { + starts.push({ title: match[1], from: match.index }); + } + for (let i = 0; i < starts.length; i += 1) { + const to = i + 1 < starts.length ? starts[i + 1].from : source.length; + bodies.set(starts[i].title, source.slice(starts[i].from, to)); + } + return bodies; +} + +describe("the state parser sees what it looks for", () => { + const SYNTHETIC = ` + import { expect, test } from "./setup/test"; + + test.describe("a group", () => { + test("first — success", async ({ page }) => { + await scanForViolations(page); + }); + + test("second — refusal", async ({ page }) => { + await expectKeyboardReach(page, '[data-slot="x"]'); + }); + }); + `; + + it("splits a spec into its test bodies", () => { + const bodies = testBodies(SYNTHETIC); + expect([...bodies.keys()]).toEqual(["first — success", "second — refusal"]); + expect(bodies.get("first — success")).toContain(SCAN); + // And no body bleeds into the next one, which is what would make a single + // well-instrumented test satisfy the checks for every state. + expect(bodies.get("first — success")).not.toContain(KEYBOARD); + }); + + it("does not treat a describe block as a test", () => { + expect(testBodies(SYNTHETIC).has("a group")).toBe(false); + }); +}); + +describe("the five states are enumerated once each", () => { + it("lists exactly the five, with no state named twice", () => { + expect(ACCESSIBILITY_STATES).toHaveLength(5); + expect(SHARING_ACCESSIBILITY_STATES).toHaveLength(5); + expect(SHARING_ACCESSIBILITY_STATES.map((c) => c.state).sort()).toEqual( + [...ACCESSIBILITY_STATES].sort(), + ); + }); + + it("anchors every state on a slot the tree really renders", () => { + // A state can otherwise be added to the list with an anchor nothing emits, + // and the browser test would then wait for an element that cannot appear — + // which fails, but fails as a timeout that reads like flake rather than + // like a missing surface. + for (const c of SHARING_ACCESSIBILITY_STATES) { + const source = readFileSync(join(ROOT, c.rendersIn), "utf8"); + // The last `[data-slot="…"]` in a compound selector is the one the + // module has to emit; the prefix is context from an ancestor. + const slots = [...c.anchor.matchAll(/data-slot="([^"]+)"/g)].map( + (m) => m[1], + ); + expect(slots.length, c.anchor).toBeGreaterThan(0); + expect(source, `${c.rendersIn} does not emit ${c.anchor}`).toContain( + `data-slot="${slots[slots.length - 1]}"`, + ); + } + }); + + it("gives every state a way forward or says it has none", () => { + // Exactly one state may be without a control, and it is the one nobody can + // act in. A second `null` here would mean a dead end shipped as a state. + const dead = SHARING_ACCESSIBILITY_STATES.filter( + (c) => c.keyboardTarget === null, + ); + expect(dead.map((c) => c.state)).toEqual(["loading"]); + }); +}); + +describe("every state is claimed by a browser test that examines it", () => { + const spec = readFileSync(A11Y_SPEC, "utf8"); + const bodies = testBodies(spec); + + it("parses a non-zero number of tests out of the spec", () => { + // The pin the legs below rest on: an empty map satisfies "no state is + // missing a helper call" perfectly. + expect(bodies.size).toBeGreaterThanOrEqual( + SHARING_ACCESSIBILITY_STATES.length, + ); + }); + + for (const c of SHARING_ACCESSIBILITY_STATES) { + describe(c.state, () => { + const body = [...bodies.entries()].find(([title]) => + title.includes(`[${c.state}]`), + )?.[1]; + + it("has a test that names the state", () => { + expect( + body, + `no test title carries [${c.state}] in ${A11Y_SPEC}`, + ).toBeDefined(); + }); + + it("enters the state before it asserts anything about it", () => { + expect(body ?? "", c.anchor).toContain(c.anchor); + }); + + it("scans it and proves the reading order", () => { + expect(body ?? "").toContain(SCAN); + expect(body ?? "").toContain(READER_ORDER); + }); + + it( + c.keyboardTarget === null + ? "announces itself, having nothing to press" + : "puts its way forward within reach of a keyboard", + () => { + if (c.keyboardTarget === null) { + expect(body ?? "").toContain(ANNOUNCED); + expect(body ?? "").not.toContain(KEYBOARD); + } else { + expect(body ?? "").toContain(KEYBOARD); + expect(body ?? "").toContain(c.keyboardTarget); + } + }, + ); + + it( + c.contrastTarget === null + ? "has no visible copy to check" + : "renders its material copy as content, not as muted meta", + () => { + if (c.contrastTarget === null) { + expect(body ?? "").not.toContain(CONTRAST); + } else { + expect(body ?? "").toContain(CONTRAST); + expect(body ?? "").toContain(c.contrastTarget); + } + }, + ); + }); + } + + it("defines every helper it demands", () => { + // Otherwise the checks above are satisfied by a call to a function that + // does not exist, which typechecks nowhere this guard can see. + for (const helper of [SCAN, READER_ORDER, KEYBOARD, ANNOUNCED, CONTRAST]) { + expect(spec, helper).toMatch( + new RegExp(`(async\\s+function|const)\\s+${helper}\\b`), + ); + } + }); +}); + +describe("the eight release journeys are enumerated", () => { + it("names exactly eight, with distinct ids", () => { + expect(V137_E2E_JOURNEYS).toHaveLength(8); + expect(new Set(V137_E2E_JOURNEYS.map((j) => j.id)).size).toBe(8); + for (const journey of V137_E2E_JOURNEYS) { + expect(journey.claim.length, journey.id).toBeGreaterThan(40); + } + }); +}); diff --git a/src/components/ui/query-error-card.tsx b/src/components/ui/query-error-card.tsx index facb42628..0104b260b 100644 --- a/src/components/ui/query-error-card.tsx +++ b/src/components/ui/query-error-card.tsx @@ -48,7 +48,16 @@ export function QueryErrorCard({

{description}

) : null} {onRetry ? ( - diff --git a/tests/fixtures/v137/e2e-journeys.ts b/tests/fixtures/v137/e2e-journeys.ts new file mode 100644 index 000000000..37f4c118a --- /dev/null +++ b/tests/fixtures/v137/e2e-journeys.ts @@ -0,0 +1,176 @@ +/** + * What a browser has to prove about v1.37 sharing, enumerated. + * + * Two lists, and both exist for the same reason: a browser suite reports what + * it ran, never what it did not. A state nobody wrote a test for and a journey + * somebody quietly dropped both look like a green run. Naming them in one + * place that a fast test reads back turns "we cover the error state" from a + * claim in a summary into something that fails. + * + * Nothing here drives a browser. The enumeration is the contract; the spec is + * the implementation, and `src/__tests__/sharing-accessibility-states.test.ts` + * is what holds the two together. + */ + +/* -------------------------------------------------------------------------- */ +/* The five states */ +/* -------------------------------------------------------------------------- */ + +/** + * Every state a shared-record surface can be in, and all five have to be + * usable — not merely reachable. + * + * Accessibility work concentrates on the state where everything worked, + * because that is the state somebody building the screen is looking at. The + * other four are where a person who cannot see the screen is most stranded: a + * spinner nobody announces, an error card that reads as an empty page, a + * refusal whose only way out is a button no keyboard reaches. + */ +export const ACCESSIBILITY_STATES = [ + "success", + "empty", + "loading", + "error", + "refusal", +] as const; + +export type AccessibilityState = (typeof ACCESSIBILITY_STATES)[number]; + +export interface AccessibilityStateCase { + readonly state: AccessibilityState; + /** + * The `data-slot` that proves the state is actually on screen. + * + * A scan of a page that never entered the state passes trivially, so every + * case names the element that says it arrived, and the fast guard checks the + * component tree really renders that slot. + */ + readonly anchor: string; + /** The module that renders {@link anchor}, so a state cannot be invented. */ + readonly rendersIn: string; + /** + * The control keyboard focus must reach, or `null` where the state offers + * none at all. A loading state has nothing to press; every other state here + * has a way forward, and a way forward a keyboard cannot reach is not one. + */ + readonly keyboardTarget: string | null; + /** + * Copy the person has to read and act on, which must render as content + * rather than as muted meta (UI-STANDARDS §3). `null` where the state's only + * text is a screen-reader label, which has no visual contrast to check. + */ + readonly contrastTarget: string | null; + /** Why this state is on the list, in one line. */ + readonly claim: string; +} + +export const SHARING_ACCESSIBILITY_STATES = [ + { + state: "success", + anchor: '[data-slot="profile-summary"]', + rendersIn: "src/components/records/profile-summary.tsx", + keyboardTarget: '[data-slot="shared-record-banner-exit"]', + contrastTarget: '[data-slot="shared-record-banner-context"]', + claim: + "inside somebody else's record, the line saying whose it is and the way out are both usable", + }, + { + state: "empty", + anchor: '[data-slot="grants-given-card"] [data-slot="empty-state"]', + rendersIn: "src/components/ui/empty-state.tsx", + keyboardTarget: '[data-slot="grant-invite-identifier"]', + contrastTarget: + '[data-slot="grants-given-card"] [data-slot="empty-state"] p', + claim: + "nothing shared yet reads as an answer rather than as a page that failed to load", + }, + { + state: "loading", + anchor: '[data-slot="record-scope-hydration-gate"]', + rendersIn: "src/components/layout/record-scope-hydration-gate.tsx", + keyboardTarget: null, + contrastTarget: null, + claim: + "the hold before the active record is known is announced, not a silent blank", + }, + { + state: "error", + anchor: '[data-slot="query-error-card"]', + rendersIn: "src/components/ui/query-error-card.tsx", + keyboardTarget: '[data-slot="query-error-retry"]', + contrastTarget: '[data-slot="query-error-card"] p', + claim: "a failed read says so and offers the retry to a keyboard", + }, + { + state: "refusal", + anchor: '[data-slot="shared-record-unavailable"]', + rendersIn: "src/components/layout/shared-record-unavailable.tsx", + keyboardTarget: '[data-slot="shared-record-unavailable-leave"]', + contrastTarget: '[data-slot="shared-record-unavailable"] p', + claim: + "a surface sharing does not cover explains itself and keeps a way back to one's own record", + }, +] as const satisfies readonly AccessibilityStateCase[]; + +/* -------------------------------------------------------------------------- */ +/* The eight journeys */ +/* -------------------------------------------------------------------------- */ + +export interface JourneyClaim { + /** Stable id, quoted in the spec's test title so the two can be matched. */ + readonly id: string; + /** What the journey proves, in the release's own words. */ + readonly claim: string; +} + +/** + * The eight end-to-end proofs the release is required to run, transcribed from + * the phase context rather than restated: a paraphrase is where a requirement + * quietly narrows. + * + * The list is here rather than in a spec file because more than one spec runs + * parts of it, and because the integration plan reconciles the whole set at + * the end. An id that no test title mentions is a journey nobody ran. + */ +export const V137_E2E_JOURNEYS = [ + { + id: "J1-adult-levels-lifecycle", + claim: + "Adult full READ, scoped READ, WRITE and MANAGE through invite, accept, switch, expiry, revoke and direct URL.", + }, + { + id: "J2-manage-edits-without-settings", + claim: + "Adult MANAGE edits and deletes with specific activity, and zero Guardian-only settings or integration reach.", + }, + { + id: "J3-managed-profile-lifecycle", + claim: + "Truthful managed-profile creation, two Guardians, concurrent last-Guardian protection, record settings and deletion.", + }, + { + id: "J4-guardian-notification-matrix", + claim: + "Two Guardians by two profiles through APNs, Web Push, Telegram and ntfy with recipient locale and preferences, non-interactive payloads and subject/recipient attempts.", + }, + { + id: "J5-revoked-idempotent-replay", + claim: + "Revoked MANAGE idempotency replay returns 403, no cached DTO and no second mutation.", + }, + { + id: "J6-no-delegated-provider-egress", + claim: + "An invited MANAGE mutation with a configured provider produces no provider job or call; the owner positive control enqueues.", + }, + { + id: "J7-durable-import-actor", + claim: + "Asynchronous import completion retains record, actor and job identity.", + }, + { + id: "J8-old-client-compatibility", + claim: + "Old-client compatibility preserves legacy decode, scoped fail-closed behaviour and MANAGE decode until the canonical client is ready.", + }, +] as const satisfies readonly JourneyClaim[]; From ef242f33bbaa0d5b2157362204f6807417f1e3ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 7 Aug 2026 00:01:27 +0200 Subject: [PATCH 4/9] feat: open the anamnesis record surface to managers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /api/allergies and POST /api/family-history have resolved requireRecordAuth("manage", "profile") since the level shipped, while the only form in the product that posts to either sat behind a Settings destination classified unavailable. The write was admitted, audited and owner-scoped, and nobody could reach it — the one-ended change this repository keeps rediscovering, with the permission shipped and the surface left as a follow-up. The destination now carries a classification that means what the routes mean: reachable by everybody they already admit, which is a guardian inside the profile they administer and an adult delegate holding manage. The four places that decided visibility on the guardian-only literal were each updated deliberately, and the shell that refused every settings path in an adult share now admits exactly the destinations classified as record content. Record configuration — modules, thresholds, notification routing — stays with the owner at every level. The page inside a record is its own component holding the two managers whose routes answer for a delegate. Mounting the personal section would have put three more forms on screen that refuse on submit. The family-history card says whose statement the row is, which the route comment could not. Both routes carried a long paragraph arguing the write was deliberately unreachable. It is gone rather than left to send the next reader back to close what this opens. The integration coverage moves from the undeclared-mode refusal to a manage contract: a manage delegate and a guardian each write and the row lands under the owner with the delegate named as actor, while read and write delegates get the insufficient-access refusal. Nothing exercised either route at manage before. --- messages/de.json | 3 + messages/en.json | 3 + messages/es.json | 3 + messages/fr.json | 3 + messages/it.json | 3 + messages/pl.json | 3 + .../managed-record-navigation.test.tsx | 73 +++++- .../record-settings-contract.test.ts | 50 ++++ src/app/api/allergies/route.ts | 44 ++-- src/app/api/family-history/route.ts | 27 +- src/components/layout/auth-shell.tsx | 33 ++- .../settings/record-anamnesis-section.tsx | 78 ++++++ .../settings/record-settings-section-gate.tsx | 18 +- src/components/settings/settings-shell.tsx | 33 ++- src/lib/record-settings/classification.ts | 62 ++++- src/lib/record-settings/index.ts | 1 + .../sharing-delegable-writes.test.ts | 232 ++++++++++++++---- 17 files changed, 550 insertions(+), 119 deletions(-) create mode 100644 src/components/settings/record-anamnesis-section.tsx diff --git a/messages/de.json b/messages/de.json index 028a6958a..30d655577 100644 --- a/messages/de.json +++ b/messages/de.json @@ -9758,6 +9758,9 @@ "notFound": "Diesen Zugriff gibt es nicht mehr.", "revokeFailed": "Der Zugriff konnte nicht beendet werden.", "renounceFailed": "Der Zugriff konnte nicht zurückgegeben werden." + }, + "anamnesis": { + "relativeNote": "Ein Eintrag zur Familienanamnese beschreibt die Angehörigen dieser Person. Trage ihn nur ein, wenn sie das hier festgehalten haben möchte." } } } diff --git a/messages/en.json b/messages/en.json index ce085b6b8..d82410d75 100644 --- a/messages/en.json +++ b/messages/en.json @@ -9758,6 +9758,9 @@ "notFound": "That access is no longer there.", "revokeFailed": "The access could not be ended.", "renounceFailed": "The access could not be handed back." + }, + "anamnesis": { + "relativeNote": "A family history entry describes this person's relatives. Add it only when they would want it recorded here." } } } diff --git a/messages/es.json b/messages/es.json index f48a513ca..089f1ca4f 100644 --- a/messages/es.json +++ b/messages/es.json @@ -9758,6 +9758,9 @@ "notFound": "Ese acceso ya no existe.", "revokeFailed": "No se pudo terminar el acceso.", "renounceFailed": "No se pudo devolver el acceso." + }, + "anamnesis": { + "relativeNote": "Una entrada de antecedentes familiares describe a los familiares de esta persona. Añádela solo si querría que quedara registrada aquí." } } } diff --git a/messages/fr.json b/messages/fr.json index 4c8d71687..53f77823a 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -9758,6 +9758,9 @@ "notFound": "Cet accès n'existe plus.", "revokeFailed": "L'accès n'a pas pu être terminé.", "renounceFailed": "L'accès n'a pas pu être rendu." + }, + "anamnesis": { + "relativeNote": "Un antécédent familial décrit les proches de cette personne. Ne l'ajoutez que si elle voudrait le voir consigné ici." } } } diff --git a/messages/it.json b/messages/it.json index 28297a6e6..91cfcacba 100644 --- a/messages/it.json +++ b/messages/it.json @@ -9758,6 +9758,9 @@ "notFound": "Questo accesso non esiste più.", "revokeFailed": "Non è stato possibile terminare l'accesso.", "renounceFailed": "Non è stato possibile restituire l'accesso." + }, + "anamnesis": { + "relativeNote": "Una voce di anamnesi familiare descrive i parenti di questa persona. Aggiungila solo se vorrebbe vederla registrata qui." } } } diff --git a/messages/pl.json b/messages/pl.json index 0577eed2d..e8eb37c86 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -9758,6 +9758,9 @@ "notFound": "Tego dostępu już nie ma.", "revokeFailed": "Nie udało się zakończyć dostępu.", "renounceFailed": "Nie udało się zwrócić dostępu." + }, + "anamnesis": { + "relativeNote": "Wpis w wywiadzie rodzinnym opisuje bliskich tej osoby. Dodaj go tylko wtedy, gdy chciałaby, żeby został tu zapisany." } } } diff --git a/src/__tests__/managed-record-navigation.test.tsx b/src/__tests__/managed-record-navigation.test.tsx index 09dec936a..1f0c56634 100644 --- a/src/__tests__/managed-record-navigation.test.tsx +++ b/src/__tests__/managed-record-navigation.test.tsx @@ -6,6 +6,10 @@ import { classifySettingsDestination } from "@/lib/record-settings"; const pathnameRef = { value: "/settings/security" }; const activeRecordRef = { value: true }; +/** The record on screen. `managed` unless a case says otherwise. */ +const recordRef: { value: { kind: "managed" | "shared"; level: string } } = { + value: { kind: "managed", level: "manage" }, +}; vi.mock("next/navigation", () => ({ usePathname: () => pathnameRef.value, @@ -23,8 +27,8 @@ vi.mock("@/hooks/use-auth", () => ({ username: "managed-record", displayName: "Managed record", access: "write", - level: "manage", - recordKind: "managed", + level: recordRef.value.level, + recordKind: recordRef.value.kind, sections: null, canWrite: true, }, @@ -63,18 +67,36 @@ function renderSettings(pathname = "/settings/security") { ); } +/** + * The destinations a record offers, by what kind of record it is. + * + * v1.37.0 — a managed profile offers guardian configuration AND the record + * content a MANAGE holder may write, because a Guardian holds MANAGE. The + * `manage-writable` kind is what carries the second half; before it existed + * the anamnesis form had no page a Guardian could open, while the routes + * behind it had admitted the write since the level shipped. + */ +const MANAGED_KINDS = new Set(["managed-guardian", "manage-writable"]); + describe("managed record navigation", () => { it("shows only managed settings destinations while retaining an unavailable deep-link heading", () => { + recordRef.value = { kind: "managed", level: "manage" }; const html = renderSettings(); - const managedDestinations = SETTINGS_SECTIONS.filter( - (section) => - classifySettingsDestination(section.slug).kind === "managed-guardian", + const managedDestinations = SETTINGS_SECTIONS.filter((section) => + MANAGED_KINDS.has(classifySettingsDestination(section.slug).kind), ); const unavailableDestinations = SETTINGS_SECTIONS.filter( (section) => - classifySettingsDestination(section.slug).kind !== "managed-guardian", + !MANAGED_KINDS.has(classifySettingsDestination(section.slug).kind), ); + // The partition is non-empty on both sides. A classification change that + // emptied either would make one of the loops below prove nothing. + expect(managedDestinations.length).toBeGreaterThan(1); + expect(unavailableDestinations.length).toBeGreaterThan(1); + // And the destination this release opened is really on the offered side. + expect(managedDestinations.map((s) => s.slug)).toContain("anamnesis"); + expect(html).toContain('data-active-record-id="managed-record"'); expect(html).toContain('id="settings-section-security-title"'); expect(html).not.toMatch(/href="[^"]*managed-record/); @@ -91,7 +113,46 @@ describe("managed record navigation", () => { } }); + it("offers an adult manager the record content and none of the configuration", () => { + // The other half of the same classification, and the half that is easiest + // to get wrong in the generous direction: a delegate manages somebody's + // health record, not their account. Anamnese is offered; modules, + // thresholds and notification routing are not, even though a Guardian + // reaches all three. + recordRef.value = { kind: "shared", level: "manage" }; + const html = renderSettings(); + + expect(html).toContain('href="/settings/anamnesis"'); + for (const guardianOnly of [ + "account", + "modules", + "notifications", + "thresholds", + "coach", + ]) { + expect(html, guardianOnly).not.toContain( + `href="/settings/${guardianOnly}"`, + ); + } + }); + + it("offers a WRITE delegate no settings destination at all", () => { + // The refusal side. `manage-writable` is admitted by LEVEL, not by being + // in a shared record: a WRITE grant reaches no Settings page, and a + // destination it could see would be one the route below refuses. + recordRef.value = { kind: "shared", level: "write" }; + const html = renderSettings(); + + expect(html).not.toContain('href="/settings/anamnesis"'); + for (const section of SETTINGS_SECTIONS) { + expect(html, section.slug).not.toContain( + `href="/settings/${section.slug}"`, + ); + } + }); + it("keeps the exit control present while administering a managed record", () => { + recordRef.value = { kind: "managed", level: "manage" }; const html = renderToStaticMarkup( diff --git a/src/__tests__/record-settings-contract.test.ts b/src/__tests__/record-settings-contract.test.ts index 5173a575e..50d3781ab 100644 --- a/src/__tests__/record-settings-contract.test.ts +++ b/src/__tests__/record-settings-contract.test.ts @@ -7,6 +7,7 @@ import { import { classifySettingsDestination, isGuardianSettingsWriteAllowed, + isManageDelegateSettingsDestination, SETTINGS_DESTINATION_INVENTORY, } from "@/lib/record-settings/classification"; import { toRecordSettingsDto } from "@/lib/record-settings/dto"; @@ -74,6 +75,55 @@ describe("record settings contract", () => { expect(isGuardianSettingsWriteAllowed("ai")).toBe(false); }); + /** + * v1.37.0 — the anamnesis destination, which is the one piece of record + * CONTENT under `/settings`. + * + * `POST /api/allergies` and `POST /api/family-history` have resolved + * `requireRecordAuth("manage", "profile")` since MANAGE shipped, while the + * only form that posts to either was classified unavailable — an admitted + * write with no reachable caller. These three legs are the classification + * half of closing that; the integration file drives the routes themselves. + */ + it("opens the anamnesis destination to everybody the routes already admit", () => { + expect(classifySettingsDestination("anamnesis")).toEqual({ + kind: "manage-writable", + guardianWritable: true, + }); + // The Guardian half. Before this the destination existed and was + // read-only for the one person the profile exists for. + expect(isGuardianSettingsWriteAllowed("anamnesis")).toBe(true); + // The adult-delegate half, and its boundary in the same breath: a MANAGE + // grant opens the record's health background and nothing else under + // Settings. Record CONFIGURATION — modules, thresholds, notification + // routing — stays with the owner even though a Guardian may change it. + expect(isManageDelegateSettingsDestination("anamnesis")).toBe(true); + for (const guardianOnly of [ + "account", + "modules", + "notifications", + "thresholds", + "insights", + "coach", + "integrations", + ]) { + expect( + isManageDelegateSettingsDestination(guardianOnly), + guardianOnly, + ).toBe(false); + } + }); + + it("leaves exactly one destination on the manage-writable list", () => { + // The list is short on purpose and a second entry is a consent decision, + // not a classification tidy-up. Pinning the count means a destination + // cannot join it as a side effect of somebody reclassifying a neighbour. + const manageWritable = Object.entries(SETTINGS_DESTINATION_INVENTORY) + .filter(([, value]) => value.kind === "manage-writable") + .map(([slug]) => slug); + expect(manageWritable).toEqual(["anamnesis"]); + }); + it("does not report a synthetic ledger row as a managed connection", () => { const connected = { withings: false, diff --git a/src/app/api/allergies/route.ts b/src/app/api/allergies/route.ts index 4617cdcd6..f4e69e40c 100644 --- a/src/app/api/allergies/route.ts +++ b/src/app/api/allergies/route.ts @@ -67,39 +67,23 @@ export const GET = apiHandler(async (request: NextRequest) => { export const POST = apiHandler(withIdempotency<[NextRequest]>(postAllergy)); async function postAllergy(request: NextRequest): Promise { - // v1.36.x — the GET above is delegable and this is not, which is the - // opposite of where the classification landed and worth the paragraph. + // v1.37.0 — MANAGE, and reachable. // - // Nothing about the row changed: it is still a plain statement about the - // record's own body, still the single most useful thing a caregiver could - // contribute, and the argument for admitting it still holds. What it does - // not have is a caller. The only place in the product that posts here is the - // allergy manager in Settings → Anamnese, and `/settings/*` is not a - // shared-record destination — the shell shows the "not part of what was - // shared" panel there, so no delegate can reach the form at any level. + // Recording an allergy for somebody whose record you manage is the case the + // level exists for; on a managed profile it is the guardian's ordinary act. + // Additive, audited, and the row belongs to the record owner. // - // An admitted write with no reachable surface is the one-ended change this - // repository keeps rediscovering (CLAUDE.md, "A two-ended change carries - // both ends"): the permission ships, the consumer is the follow-up, and - // nothing in the gate notices because every other check proves the other - // end. The frozen list is built the other way round on purpose — its own - // actor-surface note says the rest "arrive as their own diffs; naming them - // before they exist would freeze a guess." + // The surface is Settings → Anamnese inside the record + // (`src/components/settings/record-anamnesis-section.tsx`, reached through + // `record-settings-section-gate.tsx` for any destination classified + // `manage-writable`). This arm carried a long paragraph through v1.36.x + // arguing that the write was deliberately unreachable and waiting for that + // surface; the surface exists now, so the paragraph is gone rather than left + // to send the next reader back to close what it opens. // - // So this arm waits for the surface that would exercise it, and the two - // land together. Choosing that surface is design work, not a fix: allergies - // and family history have exactly one home today, that home is a personal - // account surface a switch rightly closes, and bolting a second copy onto a - // shared page would split one concept across two places. Re-admitting is - // one line here plus one entry in `sharing-surface-guard.test.ts` plus the - // paragraph that argues it — which is exactly the reviewed diff that guard - // exists to force. - // - // The half that was always the point is untouched: a caregiver can still - // READ the allergy list inside the record. - // v1.37.0 — MANAGE. Recording an allergy for somebody whose record you - // manage is the case the level exists for; on a managed profile it is the - // guardian's ordinary act. Additive, audited, and the row is the record's. + // A READ or WRITE delegate reaches neither: the destination is not on their + // Settings list, and this line refuses them with `sharing.access.denied` + // regardless. const { user } = await requireRecordAuth("manage", "profile"); const { data: rawBody, error: jsonError } = await safeJson(request, { diff --git a/src/app/api/family-history/route.ts b/src/app/api/family-history/route.ts index 3bf1435df..661907833 100644 --- a/src/app/api/family-history/route.ts +++ b/src/app/api/family-history/route.ts @@ -62,23 +62,18 @@ export const POST = apiHandler( ); async function postFamilyHistory(request: NextRequest): Promise { - // v1.36.x — not a delegated write, though the GET above is delegable. The - // reasoning is written out at the sibling arm in `api/allergies/route.ts` - // and is the same here: the only surface that posts to either route is the - // manager in Settings → Anamnese, and `/settings/*` is closed inside a - // shared record, so no delegate can reach the form at any level. An - // admitted write with no reachable caller is a permission frozen ahead of - // the surface that would exercise it. + // v1.37.0 — MANAGE, and reachable. Third-party data by design, admitted on + // the same reasoning the read arm was: it is the record's own health + // background, and the surface is Settings → Anamnese inside the record + // (`src/components/settings/record-anamnesis-section.tsx`). // - // This arm carried an extra reason to wait. The row describes the record's - // RELATIVES, so it is the one admitted write where third-party health - // information is present by design — and frequently the delegate is that - // relative, asserting a condition about themselves into somebody else's - // record. The classification admitted it and named the discomfort. Landing - // it together with the surface that offers it means the copy on that surface - // can say whose statement the row is, which no route comment can. - // v1.37.0 — MANAGE. Third-party data by design, admitted on the same - // reasoning the read arm was: it is the record's own health background. + // This arm carried an extra reason to wait, and that reason is now answered + // where it can be. The row describes the record subject's RELATIVES, so it + // is the one admitted write where third-party health information is present + // by design — and frequently the delegate IS that relative, asserting a + // condition about themselves into somebody else's record. No route comment + // can say that to the person filling the form; the card above the form says + // it (`recordSharing.anamnesis.relativeNote`). const { user } = await requireRecordAuth("manage", "profile"); const { data: rawBody, error: jsonError } = await safeJson(request, { diff --git a/src/components/layout/auth-shell.tsx b/src/components/layout/auth-shell.tsx index aeb9bddba..1849f4e61 100644 --- a/src/components/layout/auth-shell.tsx +++ b/src/components/layout/auth-shell.tsx @@ -14,6 +14,7 @@ import { useRecordCapabilities } from "@/hooks/use-record-capabilities"; import { useRecordSessionTransition } from "@/hooks/use-record-session-transition"; import { useTranslations } from "@/lib/i18n/context"; import { CoachLaunchProvider } from "@/lib/insights/coach-launch-context"; +import { isManageDelegateSettingsDestination } from "@/lib/record-settings"; import { isDestinationInSharedRecord } from "./nav-model"; import { BottomNav } from "./bottom-nav"; import { DemoBanner } from "./demo-banner"; @@ -84,15 +85,35 @@ export function AuthShell({ const { inSharedRecord, recordKind, + level, sections, accessRefused, recordSessionPending, } = useRecordCapabilities(); - // A managed profile has a deliberately small Settings surface of its own. - // Let that route family reach its record-aware gate; adult shared records - // stay in the generic refusal branch even at MANAGE. - const isManagedRecordSettingsPath = - recordKind === "managed" && pathname.startsWith("/settings"); + // A shared record has a deliberately small Settings surface of its own, and + // which routes belong to it depends on the record. + // + // A managed profile keeps the whole `/settings` family reaching its + // record-aware gate: guardian configuration lives there and the gate refuses + // per destination. + // + // v1.37.0 — an ordinary shared record at MANAGE reaches exactly the + // destinations classified `manage-writable`, which today is Settings → + // Anamnese and nothing else. That is not a widening of what a delegate may + // do: `POST /api/allergies` and `POST /api/family-history` already resolve + // `requireRecordAuth("manage", "profile")` and have done since the level + // shipped. What was missing was any page from which to call them, and an + // admitted write with no reachable caller is the half-wired state this + // repository keeps rediscovering. Every other `/settings` path in an adult + // share stays in the generic refusal branch, at every level. + const settingsSlug = pathname.startsWith("/settings") + ? (pathname.split("/")[2] ?? "") + : null; + const isRecordSettingsPath = + settingsSlug !== null && + (recordKind === "managed" || + (level === "manage" && + isManageDelegateSettingsDestination(settingsSlug))); // v1.36.0 — a page reached while acting on somebody else's record that // sharing does not cover. The nav already drops these entries, so arriving // here means a bookmark or a browser back button; the panel below explains @@ -101,7 +122,7 @@ export function AuthShell({ const outsideSharedRecord = inSharedRecord && !isDestinationInSharedRecord(pathname, sections) && - !isManagedRecordSettingsPath; + !isRecordSettingsPath; const isOnboardingPage = pathname === "/onboarding"; const showUnlockNotifier = isAuthenticated && !isPublicPage && !!user?.id; diff --git a/src/components/settings/record-anamnesis-section.tsx b/src/components/settings/record-anamnesis-section.tsx new file mode 100644 index 000000000..f6cce2663 --- /dev/null +++ b/src/components/settings/record-anamnesis-section.tsx @@ -0,0 +1,78 @@ +"use client"; + +import { ShieldAlert, Users } from "lucide-react"; + +import { SettingsCard } from "@/components/settings/settings-card"; +import { SettingsCardHeader } from "@/components/settings/_card-header"; +import { AllergyManager } from "@/components/records/allergy-manager"; +import { FamilyHistoryManager } from "@/components/records/family-history-manager"; +import { useTranslations } from "@/lib/i18n/context"; + +/** + * v1.37.0 — Settings → Anamnese, inside somebody else's record. + * + * ## Why a second component instead of the personal one + * + * `AnamnesisSection` renders five managers, and only two of them belong to the + * record. The other three write through `/api/coach/about-me` and + * `/api/anamnesis/facts`, both of which resolve a bare `requireAuth()` and + * therefore refuse outright under a switch. Mounting the personal section here + * would put three forms on screen that answer 403 on submit — the precise + * failure `useRecordCapabilities` was written to end ("a control that appeared + * before its route answered"). So this surface is the intersection of the + * section and what the record routes admit, and it is a separate component so + * that intersection is visible rather than expressed as four conditionals + * inside the personal one. + * + * ## What is here + * + * Allergies and family history. Both post to routes that resolve + * `requireRecordAuth("manage", "profile")`, so both land under the record + * owner with the delegate named as actor, and a READ or WRITE delegate is + * refused by the route as well as never reaching this page. + * + * Family history is the one admitted write where third-party health + * information is present by design — the row describes the record subject's + * relatives, and often enough the delegate IS that relative, asserting a + * condition about themselves into somebody else's record. The route's own + * comment named that discomfort and left it for the surface to say out loud, + * because no route comment can. The sentence under the card is where it gets + * said. + * + * The cache is not a concern here and the reason is worth naming: switching + * records clears the whole query client, so the allergy and family-history + * lists cannot carry one record's rows into another's view. + */ +export function RecordAnamnesisSection() { + const { t } = useTranslations(); + + return ( +
+ + + + + + + + +

+ {t("recordSharing.anamnesis.relativeNote")} +

+ + +

{t("records.disclaimer")}

+
+ ); +} diff --git a/src/components/settings/record-settings-section-gate.tsx b/src/components/settings/record-settings-section-gate.tsx index 2bfac5803..449c55bf6 100644 --- a/src/components/settings/record-settings-section-gate.tsx +++ b/src/components/settings/record-settings-section-gate.tsx @@ -10,6 +10,7 @@ import { classifySettingsDestination } from "@/lib/record-settings"; import { SettingsCardHeader } from "@/components/settings/_card-header"; import { ManagedIntegrationStatus } from "@/components/settings/managed-integration-status"; import { ManagedRecordSettingsSection } from "@/components/settings/managed-record-settings-section"; +import { RecordAnamnesisSection } from "@/components/settings/record-anamnesis-section"; import { SettingsCard } from "@/components/settings/settings-card"; import type { SettingsSectionSlug } from "@/components/settings/section-slugs"; @@ -35,7 +36,7 @@ export function RecordSettingsSectionGate({ children: ReactNode; }) { const { isLoading } = useAuth(); - const { inSharedRecord, recordKind } = useRecordCapabilities(); + const { inSharedRecord, recordKind, level } = useRecordCapabilities(); const { t } = useTranslations(); // A shared session must never begin by mounting the actor's Settings @@ -53,6 +54,21 @@ export function RecordSettingsSectionGate({ if (!inSharedRecord) return children; const destination = classifySettingsDestination(section); + + // v1.37.0 — the one destination that is record CONTENT rather than record + // configuration, and therefore the one every MANAGE holder reaches: a + // Guardian inside the profile they administer, and an adult delegate whose + // grant is MANAGE. Both post to routes that already resolve + // `requireRecordAuth("manage", "profile")`; before this the forms had no + // reachable page and the permission had no caller. + // + // Checked before the managed branches so a Guardian gets the same record + // surface an adult manager does, rather than the guardian configuration + // panel, which has no family for it. + if (destination.kind === "manage-writable" && level === "manage") { + return ; + } + if ( recordKind === "managed" && destination.kind === "managed-guardian" && diff --git a/src/components/settings/settings-shell.tsx b/src/components/settings/settings-shell.tsx index 810b527d5..7d1a03655 100644 --- a/src/components/settings/settings-shell.tsx +++ b/src/components/settings/settings-shell.tsx @@ -439,19 +439,32 @@ export function SettingsShell({ modules?.[section.moduleGate] !== false, ); - // The server has already resolved which record is active. In a managed - // profile, only guardian settings are a navigation destination; the section - // gate remains responsible for refusing any direct URL that is not here. + // The server has already resolved which record is active, and which + // destinations that record offers depends on what kind of record it is. The + // section gate remains responsible for refusing any direct URL that is not + // on the list; this filter only stops the nav from offering one. + // // Keep `allVisibleSections` for the active heading so a direct unavailable // link identifies itself before the gate explains why it cannot open. + // + // * A managed profile shows its guardian configuration AND the record + // content a MANAGE holder may write — the guardian holds MANAGE, so the + // second set is theirs as well. + // * An ordinary shared record at MANAGE shows only the record content. A + // delegate manages somebody's health record, not their account: modules, + // thresholds and notification routing stay with the owner. + // * Every other shared record shows nothing here, because it reaches no + // Settings destination at all. const visibleSections = - activeRecord?.recordKind === "managed" - ? allVisibleSections.filter( - (section) => - classifySettingsDestination(section.slug).kind === - "managed-guardian", - ) - : allVisibleSections; + activeRecord === null + ? allVisibleSections + : allVisibleSections.filter((section) => { + const kind = classifySettingsDestination(section.slug).kind; + if (activeRecord.recordKind === "managed") { + return kind === "managed-guardian" || kind === "manage-writable"; + } + return kind === "manage-writable" && activeRecord.level === "manage"; + }); const activeSection = allVisibleSections.find( (section) => section.slug === activeSlug, ); diff --git a/src/lib/record-settings/classification.ts b/src/lib/record-settings/classification.ts index 9413fd128..2c600d42d 100644 --- a/src/lib/record-settings/classification.ts +++ b/src/lib/record-settings/classification.ts @@ -1,7 +1,11 @@ import type { SettingsSectionSlug } from "@/components/settings/section-slugs"; export type SettingsDestinationKind = - "personal" | "managed-guardian" | "adult-shared-unavailable" | "unavailable"; + | "personal" + | "managed-guardian" + | "manage-writable" + | "adult-shared-unavailable" + | "unavailable"; export interface SettingsDestinationClassification { kind: SettingsDestinationKind; @@ -23,6 +27,32 @@ const MANAGED_GUARDIAN_STATUS: SettingsDestinationClassification = { guardianWritable: false, }; +/** + * v1.37.0 — reachable by everybody the record routes behind it already admit. + * + * The other four kinds each answer for one population. `MANAGED_GUARDIAN` is + * guardian-only; `ADULT_SHARED_UNAVAILABLE` is closed to adult shares and open + * to nobody else; neither can express "the same set the route admits", which + * for the anamnesis surface is a Guardian on a managed profile AND an adult + * delegate holding MANAGE. + * + * That gap had a consequence rather than being a taxonomy nicety. `POST + * /api/allergies` and `POST /api/family-history` both resolve + * `requireRecordAuth("manage", "profile")` — the write is admitted, audited + * and owner-scoped — while the only form in the product that posts to either + * lives under Settings → Anamnese, which was classified unavailable. An + * admitted write with no reachable caller is the one-ended change this + * repository keeps rediscovering: the permission ships, the surface is "the + * follow-up", and every other check proves the other end. + * + * `guardianWritable` is true because a Guardian writes here too; the kind is + * what says the destination is not guardian-ONLY. + */ +const MANAGE_WRITABLE: SettingsDestinationClassification = { + kind: "manage-writable", + guardianWritable: true, +}; + const ADULT_SHARED_UNAVAILABLE: SettingsDestinationClassification = { kind: "adult-shared-unavailable", guardianWritable: false, @@ -55,7 +85,7 @@ export const SETTINGS_DESTINATION_INVENTORY = { labs: ADULT_SHARED_UNAVAILABLE, illness: ADULT_SHARED_UNAVAILABLE, environment: UNAVAILABLE, - anamnesis: ADULT_SHARED_UNAVAILABLE, + anamnesis: MANAGE_WRITABLE, score: ADULT_SHARED_UNAVAILABLE, vorsorge: ADULT_SHARED_UNAVAILABLE, thresholds: MANAGED_GUARDIAN, @@ -83,10 +113,36 @@ export function classifySettingsDestination( ); } +/** + * May a Guardian change this destination inside the profile they administer. + * + * Two kinds qualify, and the second is not a widening of the first: a + * `manage-writable` destination is one the record routes already admit at + * MANAGE, and a Guardian holds MANAGE by construction. Naming only + * `managed-guardian` here would have left the anamnesis destination visible + * and read-only for the one person the record exists for. + */ export function isGuardianSettingsWriteAllowed(destination: string): boolean { const classification = classifySettingsDestination(destination); return ( - classification.kind === "managed-guardian" && + (classification.kind === "managed-guardian" || + classification.kind === "manage-writable") && classification.guardianWritable ); } + +/** + * Is this destination part of the record surface an adult MANAGE grant opens. + * + * Deliberately narrower than {@link isGuardianSettingsWriteAllowed}: a + * Guardian administers a profile and reaches the record's configuration — + * modules, thresholds, notification routing — while an adult delegate reaches + * only the health record itself. `manage-writable` is the one kind that is + * record content rather than record configuration, so it is the only kind an + * ordinary shared record opens under `/settings`. + */ +export function isManageDelegateSettingsDestination( + destination: string, +): boolean { + return classifySettingsDestination(destination).kind === "manage-writable"; +} diff --git a/src/lib/record-settings/index.ts b/src/lib/record-settings/index.ts index c349f9355..293842925 100644 --- a/src/lib/record-settings/index.ts +++ b/src/lib/record-settings/index.ts @@ -8,6 +8,7 @@ export { resolveManagedIntegrationState } from "./integrations"; export { classifySettingsDestination, isGuardianSettingsWriteAllowed, + isManageDelegateSettingsDestination, SETTINGS_DESTINATION_INVENTORY, type SettingsDestinationClassification, type SettingsDestinationKind, diff --git a/tests/integration/sharing-delegable-writes.test.ts b/tests/integration/sharing-delegable-writes.test.ts index c3ac2452c..735a99f86 100644 --- a/tests/integration/sharing-delegable-writes.test.ts +++ b/tests/integration/sharing-delegable-writes.test.ts @@ -117,7 +117,7 @@ async function signIn(userId: string) { async function switchInto( ownerId: string, delegateId: string, - access: "READ" | "WRITE", + access: "READ" | "WRITE" | "MANAGE", ) { const { inviteGrant, acceptGrant } = await import("@/lib/sharing/grants"); const invited = await inviteGrant({ @@ -421,82 +421,155 @@ writeContract("POST /api/biomarkers", { }); /* -------------------------------------------------------------------------- */ -/* 4 + 5 — allergies and family history, the two that are NOT delegable */ +/* 4 + 5 — allergies and family history, admitted at MANAGE */ /* -------------------------------------------------------------------------- */ /** - * The opposite contract, and it earns its place beside the admitted ones. + * The MANAGE contract, and why the WRITE one will not do. * - * Both of these verbs were admitted and then withdrawn, and the argument for - * admitting them was never wrong: an allergy is a plain statement about the - * record's own body and the single most useful thing a caregiver could - * contribute. What they lack is a caller. The only surface in the product that - * posts to either lives in Settings, and a switch closes `/settings` — so no - * delegate can reach the form at any grant level, and admitting the write - * would freeze a permission ahead of anything that exercises it. + * `writeContract` above asserts that a WRITE grant writes. A route that + * declares `requireRecordAuth("manage", …)` must REFUSE a WRITE grant, so the + * two contracts differ in their first leg and agree on every other: the row + * lands under the owner and under nobody else, the audit row is filed under the + * owner with the delegate named as actor, the refusals are the stable + * `sharing.access.denied`, revocation takes effect on the very next request, + * and an unswitched owner is unaffected. * - * Both READ arms stay delegable; the read suite pins them. What is pinned here - * is the pair that has to move together: the route refuses, and the owner's own - * unswitched write still lands. Withdrawing a delegated write by breaking the - * ordinary one would be the worse bug, and no other test in this file would - * have noticed. + * Two legs are new coverage rather than a rewrite, and they are the reason this + * block exists. Nothing in the repository drove a MANAGE delegate against + * either of these routes before, and nothing drove a Guardian against them at + * all — the routes had admitted the write since the level shipped while the + * only form that posts to them was classified unavailable, so the permission + * had no reachable caller. Plan 13 opened the surface; these are the legs that + * say the write it leads to actually lands. * - * Re-admitting means flipping `requireAuth()` back to `requireRecordAuth`, - * re-listing the route in `sharing-surface-guard.test.ts`, and replacing this - * block with a `writeContract` — in the same diff as the caregiver-reachable - * surface that made it worth doing. + * What this block replaces is a `refusedWriteContract` whose legs asserted + * `sharing.not_permitted` — the refusal a route that declares NO sharing mode + * returns. These routes declare one, so a READ or WRITE delegate gets + * `sharing.access.denied` instead, and those four assertions had been failing + * against the shipped routes for a release. */ -function refusedWriteContract( - name: string, - c: { - /** Seed whatever the write needs, owned by the RECORD. Mirrors `writeContract`. */ - prepare?: (recordOwnerId: string) => Promise; - call: (ctx: C) => Promise; - ok: number; - count: (userId: string) => Promise; - }, -) { +function manageWriteContract(name: string, c: WriteCase) { const prepare = c.prepare ?? (async () => undefined as C); describe(name, () => { + it("writes into the owner's record under a live MANAGE grant", async () => { + const owner = await makeUser("owner"); + const delegate = await makeUser("delegate"); + const ctx = await prepare(owner.id); + + await switchInto(owner.id, delegate.id, "MANAGE"); + const response = await c.call(ctx); + + expect(response.status).toBe(c.ok); + // Under the owner, and under nobody else. + expect(await c.count(owner.id)).toBe(1); + expect(await c.count(delegate.id)).toBe(0); + }); + + it("writes into a managed profile for its Guardian", async () => { + // The population the surface exists for, driven through the shipped + // creation service rather than a hand-planted grant: a Guardian is a + // MANAGE holder on a credential-less record, and this is the only place + // in the suite that puts one against these routes. + const guardian = await makeUser("guardian"); + const { createManagedProfile } = + await import("@/lib/managed-profiles/create"); + const { profile } = await createManagedProfile({ + creatorId: guardian.id, + displayName: "Managed profile", + dateOfBirth: null, + locale: "de", + timezone: "Europe/Berlin", + }); + const ctx = await prepare(profile.id); + + const session = await signIn(guardian.id); + await switchSessionTo(session.id, profile.id); + + expect((await c.call(ctx)).status).toBe(c.ok); + expect(await c.count(profile.id)).toBe(1); + expect(await c.count(guardian.id)).toBe(0); + }); + + it("files the audit row under the owner and names the delegate as actor", async () => { + const owner = await makeUser("owner"); + const delegate = await makeUser("delegate"); + const ctx = await prepare(owner.id); + + await switchInto(owner.id, delegate.id, "MANAGE"); + expect((await c.call(ctx)).status).toBe(c.ok); + + const row = await getPrismaClient().auditLog.findFirst({ + where: { action: c.auditAction }, + orderBy: { createdAt: "desc" }, + }); + expect(row, `no ${c.auditAction} row was written`).not.toBeNull(); + expect(row?.userId).toBe(owner.id); + expect(row?.actorUserId).toBe(delegate.id); + }); + for (const access of ["READ", "WRITE"] as const) { - it(`refuses a delegate holding a ${access} grant, and writes nothing`, async () => { + it(`refuses a ${access} delegate with sharing.access.denied, and writes nothing`, async () => { const owner = await makeUser("owner"); const delegate = await makeUser("delegate"); const ctx = await prepare(owner.id); await switchInto(owner.id, delegate.id, access); - const response = await c.call(ctx); + // The insufficient-level refusal, not the undeclared-mode one. The + // route names a mode; what the delegate lacks is the level. + await expectDenied(await c.call(ctx)); - // The undeclared-mode refusal, not the no-grant one: the route names - // no sharing mode at all, so the carrier is refused before any grant - // is consulted. A WRITE grant makes no difference, which is the point. - expect(response.status).toBe(403); - expect((await payload(response)).meta?.errorCode).toBe( - "sharing.not_permitted", - ); - - // Neither account, not just not the owner's — a handler that fell - // back to the caller would have written a row somewhere. expect(await c.count(owner.id)).toBe(0); expect(await c.count(delegate.id)).toBe(0); }); } - it("still lets the owner write it themselves", async () => { + it("refuses a caller who names a record they were never granted", async () => { const owner = await makeUser("owner"); + const stranger = await makeUser("stranger"); const ctx = await prepare(owner.id); - await signIn(owner.id); + + await claimWithoutGrant(owner.id, stranger.id); + await expectDenied(await c.call(ctx)); + + expect(await c.count(owner.id)).toBe(0); + expect(await c.count(stranger.id)).toBe(0); + }); + + it("refuses the next request after the grant is revoked", async () => { + const owner = await makeUser("owner"); + const delegate = await makeUser("delegate"); + const ctx = await prepare(owner.id); + + const { grant } = await switchInto(owner.id, delegate.id, "MANAGE"); + expect((await c.call(ctx)).status).toBe(c.ok); + + await revoke(grant.id, owner.id); + await expectDenied(await c.call(ctx)); + expect(await c.count(owner.id)).toBe(1); + }); + + it("is unchanged for a caller who has not switched", async () => { + const plain = await makeUser("plain"); + const ctx = await prepare(plain.id); + await signIn(plain.id); const response = await c.call(ctx); - expect(response.status).toBe(c.ok); - expect(await c.count(owner.id)).toBe(1); + expect(await c.count(plain.id)).toBe(1); + + const row = await getPrismaClient().auditLog.findFirst({ + where: { action: c.auditAction }, + orderBy: { createdAt: "desc" }, + }); + expect(row?.userId).toBe(plain.id); + expect(row?.actorUserId).toBeNull(); }); }); } -refusedWriteContract("POST /api/allergies", { +manageWriteContract("POST /api/allergies", { call: async () => { const { POST } = await import("@/app/api/allergies/route"); return post(POST as Handler, "/api/allergies", { @@ -506,10 +579,11 @@ refusedWriteContract("POST /api/allergies", { }); }, ok: 201, + auditAction: "allergy.create", count: (userId) => getPrismaClient().allergy.count({ where: { userId } }), }); -refusedWriteContract("POST /api/family-history", { +manageWriteContract("POST /api/family-history", { call: async () => { const { POST } = await import("@/app/api/family-history/route"); return post(POST as Handler, "/api/family-history", { @@ -518,6 +592,7 @@ refusedWriteContract("POST /api/family-history", { }); }, ok: 201, + auditAction: "family-history.create", count: (userId) => getPrismaClient().familyHistoryEntry.count({ where: { userId } }), }); @@ -578,6 +653,69 @@ describe("POST /api/illness/episodes — the module gate", () => { /* 7 — custom-metric entries, the third that is NOT delegable */ /* -------------------------------------------------------------------------- */ +/** + * The contract for a route that declares NO sharing mode at all. + * + * Its legs assert `sharing.not_permitted` — the refusal the carrier collects + * before any grant is consulted — which is a different fact from + * `sharing.access.denied` and is what makes the pair worth keeping apart. A + * route that names a mode and refuses a level answers the second; a route that + * names nothing answers the first, and a WRITE grant makes no difference to it. + * + * v1.37.0 — allergies and family history left this helper for + * `manageWriteContract` above, in the same diff as the surface that made their + * admitted writes reachable. One caller remains. + */ +function refusedWriteContract( + name: string, + c: { + /** Seed whatever the write needs, owned by the RECORD. Mirrors `writeContract`. */ + prepare?: (recordOwnerId: string) => Promise; + call: (ctx: C) => Promise; + ok: number; + count: (userId: string) => Promise; + }, +) { + const prepare = c.prepare ?? (async () => undefined as C); + + describe(name, () => { + for (const access of ["READ", "WRITE"] as const) { + it(`refuses a delegate holding a ${access} grant, and writes nothing`, async () => { + const owner = await makeUser("owner"); + const delegate = await makeUser("delegate"); + const ctx = await prepare(owner.id); + + await switchInto(owner.id, delegate.id, access); + const response = await c.call(ctx); + + // The undeclared-mode refusal, not the no-grant one: the route names + // no sharing mode at all, so the carrier is refused before any grant + // is consulted. A WRITE grant makes no difference, which is the point. + expect(response.status).toBe(403); + expect((await payload(response)).meta?.errorCode).toBe( + "sharing.not_permitted", + ); + + // Neither account, not just not the owner's — a handler that fell + // back to the caller would have written a row somewhere. + expect(await c.count(owner.id)).toBe(0); + expect(await c.count(delegate.id)).toBe(0); + }); + } + + it("still lets the owner write it themselves", async () => { + const owner = await makeUser("owner"); + const ctx = await prepare(owner.id); + await signIn(owner.id); + + const response = await c.call(ctx); + + expect(response.status).toBe(c.ok); + expect(await c.count(owner.id)).toBe(1); + }); + }); +} + /** * The third withdrawal, one release after the other two, and the delay is what * makes it worth a paragraph: the rule that removed allergies and family From f91f701e139d78d4c2272ad0aa43e2e32beff9c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 7 Aug 2026 00:40:40 +0200 Subject: [PATCH 5/9] fix: say what a refused request actually was MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every delegable route files a breadcrumb when a request does not parse, and it files it through auditLog on purpose: that helper is what stamps the actor, and without the stamp a delegate's malformed request would read as the owner's own. The rows are meant to be attributable. None of them had a sentence, so all of them rendered in the owner's activity view as "made a change in your record" — including one a view-only delegate can trigger by sending a query the route cannot parse. Nothing had changed and nothing could have. Thirty-three actions now have honest lines. A refusal says it was an attempt and says it was refused; it never borrows the language of something that happened. Two of them are filed by library modules the routes import — a background import's completion row and a medication lifecycle reconciliation — and a walk over route files alone could not see either. The inventory guard was asking a different question from the feed. It kept only awaited audit calls while the feed shows fire-and-forget rows identically, and it read route files while the feed shows whatever the request reached. Both criteria now match the feed's, the three rows the feed structurally cannot show are excluded by name with the reason written down, and the exclusion list is pinned at three. A leg asserts the shape of the sentence for every refusal, so a future arm cannot map an attempt onto an ordinary verb and pass on coverage. The whole round trip is covered against real Postgres: a view-only delegate sends a malformed query, the row lands attributed to them under the owner, and the sentence the owner reads is an attempt. --- messages/de.json | 30 +- messages/en.json | 30 +- messages/es.json | 30 +- messages/fr.json | 30 +- messages/it.json | 30 +- messages/pl.json | 30 +- .../manage-activity-inventory.test.ts | 280 ++++++++++++++---- src/lib/record-activity/activity-verb.ts | 105 +++++++ tests/integration/sharing-audit-actor.test.ts | 74 +++++ 9 files changed, 574 insertions(+), 65 deletions(-) diff --git a/messages/de.json b/messages/de.json index 30d655577..823ce93a1 100644 --- a/messages/de.json +++ b/messages/de.json @@ -9708,7 +9708,35 @@ "cyclePeriodBoundary": "{name} hat den Beginn oder das Ende einer Periode geändert", "cycleSymptomCreate": "{name} hat ein Zyklussymptom angelegt", "nutrientWaterWrite": "{name} hat die Trinkmenge erfasst", - "healthRecordExport": "{name} hat die Akte exportiert" + "healthRecordExport": "{name} hat die Akte exportiert", + "rejectedMeasurementQuery": "{name} hat eine Messwert-Suche gesendet, die diese Akte nicht lesen konnte", + "rejectedDoseQuery": "{name} hat eine Einnahme-Suche gesendet, die diese Akte nicht lesen konnte", + "rejectedMoodQuery": "{name} hat eine Stimmungs-Suche gesendet, die diese Akte nicht lesen konnte", + "rejectedMeasurementCreate": "{name} wollte einen Messwert eintragen und wurde abgewiesen", + "rejectedMeasurementBatch": "{name} wollte mehrere Messwerte eintragen und wurde abgewiesen", + "rejectedMeasurementUpdate": "{name} wollte einen Messwert ändern und wurde abgewiesen", + "rejectedLabCreate": "{name} wollte einen Laborwert eintragen und wurde abgewiesen", + "rejectedLabUpdate": "{name} wollte einen Laborwert ändern und wurde abgewiesen", + "rejectedBiomarkerCreate": "{name} wollte einen Biomarker anlegen und wurde abgewiesen", + "rejectedBiomarkerUpdate": "{name} wollte einen Biomarker ändern und wurde abgewiesen", + "rejectedReminderCreate": "{name} wollte eine Messerinnerung anlegen und wurde abgewiesen", + "rejectedReminderUpdate": "{name} wollte eine Messerinnerung ändern und wurde abgewiesen", + "rejectedMoodCreate": "{name} wollte einen Stimmungseintrag anlegen und wurde abgewiesen", + "rejectedMoodUpdate": "{name} wollte einen Stimmungseintrag ändern und wurde abgewiesen", + "rejectedDoseCreate": "{name} wollte eine Einnahme eintragen und wurde abgewiesen", + "rejectedDoseUpdate": "{name} wollte eine Einnahme ändern und wurde abgewiesen", + "rejectedDoseImport": "{name} wollte Einnahmen importieren und wurde abgewiesen", + "rejectedTrackedValue": "{name} wollte einen eigenen Messwert eintragen und wurde abgewiesen", + "deniedDoseImport": "Ein von {name} gestarteter Einnahme-Import konnte nicht eingereiht werden", + "anamnesisFactCreate": "{name} hat einen Fakt zur Vorgeschichte eingetragen", + "customMetricCreate": "{name} hat eine eigene Messgröße angelegt", + "customMetricUpdate": "{name} hat eine eigene Messgröße geändert", + "customMetricDelete": "{name} hat eine eigene Messgröße gelöscht", + "documentStore": "{name} hat ein Dokument hinzugefügt", + "documentUpdate": "{name} hat ein Dokument geändert", + "documentDiscard": "{name} hat ein Dokument verworfen", + "medicationIntakeImportFinished": "Der von {name} gestartete Einnahme-Import ist fertig", + "medicationOneShotReconciled": "Ein einmaliges Medikament wurde nach einer Einnahme von {name} geöffnet oder geschlossen" }, "section": { "measurements": { diff --git a/messages/en.json b/messages/en.json index d82410d75..2225ac156 100644 --- a/messages/en.json +++ b/messages/en.json @@ -9708,7 +9708,35 @@ "cyclePeriodBoundary": "{name} changed the start or end of a period", "cycleSymptomCreate": "{name} added a cycle symptom", "nutrientWaterWrite": "{name} recorded water intake", - "healthRecordExport": "{name} exported the record" + "healthRecordExport": "{name} exported the record", + "rejectedMeasurementQuery": "{name} sent a reading search this record could not read", + "rejectedDoseQuery": "{name} sent a dose search this record could not read", + "rejectedMoodQuery": "{name} sent a mood search this record could not read", + "rejectedMeasurementCreate": "{name} tried to add a reading and was refused", + "rejectedMeasurementBatch": "{name} tried to add several readings and was refused", + "rejectedMeasurementUpdate": "{name} tried to change a reading and was refused", + "rejectedLabCreate": "{name} tried to add a lab result and was refused", + "rejectedLabUpdate": "{name} tried to change a lab result and was refused", + "rejectedBiomarkerCreate": "{name} tried to add a biomarker and was refused", + "rejectedBiomarkerUpdate": "{name} tried to change a biomarker and was refused", + "rejectedReminderCreate": "{name} tried to add a measurement reminder and was refused", + "rejectedReminderUpdate": "{name} tried to change a measurement reminder and was refused", + "rejectedMoodCreate": "{name} tried to add a mood entry and was refused", + "rejectedMoodUpdate": "{name} tried to change a mood entry and was refused", + "rejectedDoseCreate": "{name} tried to record a dose and was refused", + "rejectedDoseUpdate": "{name} tried to change a dose and was refused", + "rejectedDoseImport": "{name} tried to import doses and was refused", + "rejectedTrackedValue": "{name} tried to log a tracked value and was refused", + "deniedDoseImport": "A dose import {name} started could not be queued", + "anamnesisFactCreate": "{name} added a health background fact", + "customMetricCreate": "{name} added a tracked value type", + "customMetricUpdate": "{name} changed a tracked value type", + "customMetricDelete": "{name} removed a tracked value type", + "documentStore": "{name} added a document", + "documentUpdate": "{name} changed a document", + "documentDiscard": "{name} discarded a document", + "medicationIntakeImportFinished": "The dose import {name} started has finished", + "medicationOneShotReconciled": "A one-off medication was opened or closed after a dose {name} recorded" }, "section": { "measurements": { diff --git a/messages/es.json b/messages/es.json index 089f1ca4f..11b290b17 100644 --- a/messages/es.json +++ b/messages/es.json @@ -9708,7 +9708,35 @@ "cyclePeriodBoundary": "{name} cambió el inicio o el final de una regla", "cycleSymptomCreate": "{name} añadió un síntoma del ciclo", "nutrientWaterWrite": "{name} registró la ingesta de agua", - "healthRecordExport": "{name} exportó el historial" + "healthRecordExport": "{name} exportó el historial", + "rejectedMeasurementQuery": "{name} envió una búsqueda de mediciones que este historial no pudo leer", + "rejectedDoseQuery": "{name} envió una búsqueda de dosis que este historial no pudo leer", + "rejectedMoodQuery": "{name} envió una búsqueda de ánimo que este historial no pudo leer", + "rejectedMeasurementCreate": "{name} intentó añadir una medición y fue rechazado", + "rejectedMeasurementBatch": "{name} intentó añadir varias mediciones y fue rechazado", + "rejectedMeasurementUpdate": "{name} intentó cambiar una medición y fue rechazado", + "rejectedLabCreate": "{name} intentó añadir un resultado de laboratorio y fue rechazado", + "rejectedLabUpdate": "{name} intentó cambiar un resultado de laboratorio y fue rechazado", + "rejectedBiomarkerCreate": "{name} intentó añadir un biomarcador y fue rechazado", + "rejectedBiomarkerUpdate": "{name} intentó cambiar un biomarcador y fue rechazado", + "rejectedReminderCreate": "{name} intentó añadir un recordatorio de medición y fue rechazado", + "rejectedReminderUpdate": "{name} intentó cambiar un recordatorio de medición y fue rechazado", + "rejectedMoodCreate": "{name} intentó añadir una entrada de ánimo y fue rechazado", + "rejectedMoodUpdate": "{name} intentó cambiar una entrada de ánimo y fue rechazado", + "rejectedDoseCreate": "{name} intentó registrar una dosis y fue rechazado", + "rejectedDoseUpdate": "{name} intentó cambiar una dosis y fue rechazado", + "rejectedDoseImport": "{name} intentó importar dosis y fue rechazado", + "rejectedTrackedValue": "{name} intentó registrar un valor propio y fue rechazado", + "deniedDoseImport": "Una importación de dosis iniciada por {name} no se pudo poner en cola", + "anamnesisFactCreate": "{name} añadió un dato de antecedentes de salud", + "customMetricCreate": "{name} añadió un tipo de valor propio", + "customMetricUpdate": "{name} cambió un tipo de valor propio", + "customMetricDelete": "{name} eliminó un tipo de valor propio", + "documentStore": "{name} añadió un documento", + "documentUpdate": "{name} cambió un documento", + "documentDiscard": "{name} descartó un documento", + "medicationIntakeImportFinished": "La importación de dosis que inició {name} ha terminado", + "medicationOneShotReconciled": "Un medicamento de una sola toma se abrió o se cerró tras una dosis que registró {name}" }, "section": { "measurements": { diff --git a/messages/fr.json b/messages/fr.json index 53f77823a..664d71d1f 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -9708,7 +9708,35 @@ "cyclePeriodBoundary": "{name} a modifié le début ou la fin de règles", "cycleSymptomCreate": "{name} a ajouté un symptôme de cycle", "nutrientWaterWrite": "{name} a enregistré la consommation d'eau", - "healthRecordExport": "{name} a exporté le dossier" + "healthRecordExport": "{name} a exporté le dossier", + "rejectedMeasurementQuery": "{name} a envoyé une recherche de mesures que ce dossier n'a pas pu lire", + "rejectedDoseQuery": "{name} a envoyé une recherche de prises que ce dossier n'a pas pu lire", + "rejectedMoodQuery": "{name} a envoyé une recherche d'humeur que ce dossier n'a pas pu lire", + "rejectedMeasurementCreate": "{name} a tenté d'ajouter une mesure et a été refusé", + "rejectedMeasurementBatch": "{name} a tenté d'ajouter plusieurs mesures et a été refusé", + "rejectedMeasurementUpdate": "{name} a tenté de modifier une mesure et a été refusé", + "rejectedLabCreate": "{name} a tenté d'ajouter un résultat de laboratoire et a été refusé", + "rejectedLabUpdate": "{name} a tenté de modifier un résultat de laboratoire et a été refusé", + "rejectedBiomarkerCreate": "{name} a tenté d'ajouter un biomarqueur et a été refusé", + "rejectedBiomarkerUpdate": "{name} a tenté de modifier un biomarqueur et a été refusé", + "rejectedReminderCreate": "{name} a tenté d'ajouter un rappel de mesure et a été refusé", + "rejectedReminderUpdate": "{name} a tenté de modifier un rappel de mesure et a été refusé", + "rejectedMoodCreate": "{name} a tenté d'ajouter une entrée d'humeur et a été refusé", + "rejectedMoodUpdate": "{name} a tenté de modifier une entrée d'humeur et a été refusé", + "rejectedDoseCreate": "{name} a tenté d'enregistrer une prise et a été refusé", + "rejectedDoseUpdate": "{name} a tenté de modifier une prise et a été refusé", + "rejectedDoseImport": "{name} a tenté d'importer des prises et a été refusé", + "rejectedTrackedValue": "{name} a tenté d'enregistrer une valeur personnalisée et a été refusé", + "deniedDoseImport": "Un import de prises lancé par {name} n'a pas pu être mis en file", + "anamnesisFactCreate": "{name} a ajouté un élément d'antécédents de santé", + "customMetricCreate": "{name} a ajouté un type de valeur personnalisée", + "customMetricUpdate": "{name} a modifié un type de valeur personnalisée", + "customMetricDelete": "{name} a supprimé un type de valeur personnalisée", + "documentStore": "{name} a ajouté un document", + "documentUpdate": "{name} a modifié un document", + "documentDiscard": "{name} a écarté un document", + "medicationIntakeImportFinished": "L'import de prises lancé par {name} est terminé", + "medicationOneShotReconciled": "Un médicament à prise unique a été rouvert ou clos après une prise enregistrée par {name}" }, "section": { "measurements": { diff --git a/messages/it.json b/messages/it.json index 91cfcacba..90cd6c579 100644 --- a/messages/it.json +++ b/messages/it.json @@ -9708,7 +9708,35 @@ "cyclePeriodBoundary": "{name} ha modificato l'inizio o la fine di una mestruazione", "cycleSymptomCreate": "{name} ha aggiunto un sintomo del ciclo", "nutrientWaterWrite": "{name} ha registrato l'assunzione di acqua", - "healthRecordExport": "{name} ha esportato la cartella" + "healthRecordExport": "{name} ha esportato la cartella", + "rejectedMeasurementQuery": "{name} ha inviato una ricerca di misurazioni che questa cartella non ha potuto leggere", + "rejectedDoseQuery": "{name} ha inviato una ricerca di dosi che questa cartella non ha potuto leggere", + "rejectedMoodQuery": "{name} ha inviato una ricerca dell'umore che questa cartella non ha potuto leggere", + "rejectedMeasurementCreate": "{name} ha provato ad aggiungere una misurazione ed è stato rifiutato", + "rejectedMeasurementBatch": "{name} ha provato ad aggiungere più misurazioni ed è stato rifiutato", + "rejectedMeasurementUpdate": "{name} ha provato a modificare una misurazione ed è stato rifiutato", + "rejectedLabCreate": "{name} ha provato ad aggiungere un risultato di laboratorio ed è stato rifiutato", + "rejectedLabUpdate": "{name} ha provato a modificare un risultato di laboratorio ed è stato rifiutato", + "rejectedBiomarkerCreate": "{name} ha provato ad aggiungere un biomarcatore ed è stato rifiutato", + "rejectedBiomarkerUpdate": "{name} ha provato a modificare un biomarcatore ed è stato rifiutato", + "rejectedReminderCreate": "{name} ha provato ad aggiungere un promemoria di misurazione ed è stato rifiutato", + "rejectedReminderUpdate": "{name} ha provato a modificare un promemoria di misurazione ed è stato rifiutato", + "rejectedMoodCreate": "{name} ha provato ad aggiungere una voce dell'umore ed è stato rifiutato", + "rejectedMoodUpdate": "{name} ha provato a modificare una voce dell'umore ed è stato rifiutato", + "rejectedDoseCreate": "{name} ha provato a registrare una dose ed è stato rifiutato", + "rejectedDoseUpdate": "{name} ha provato a modificare una dose ed è stato rifiutato", + "rejectedDoseImport": "{name} ha provato a importare delle dosi ed è stato rifiutato", + "rejectedTrackedValue": "{name} ha provato a registrare un valore personalizzato ed è stato rifiutato", + "deniedDoseImport": "Un'importazione di dosi avviata da {name} non è stata messa in coda", + "anamnesisFactCreate": "{name} ha aggiunto un dato di anamnesi", + "customMetricCreate": "{name} ha aggiunto un tipo di valore personalizzato", + "customMetricUpdate": "{name} ha modificato un tipo di valore personalizzato", + "customMetricDelete": "{name} ha eliminato un tipo di valore personalizzato", + "documentStore": "{name} ha aggiunto un documento", + "documentUpdate": "{name} ha modificato un documento", + "documentDiscard": "{name} ha scartato un documento", + "medicationIntakeImportFinished": "L'importazione delle dosi avviata da {name} è terminata", + "medicationOneShotReconciled": "Un farmaco monodose è stato riaperto o chiuso dopo una dose registrata da {name}" }, "section": { "measurements": { diff --git a/messages/pl.json b/messages/pl.json index e8eb37c86..bdbd39496 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -9708,7 +9708,35 @@ "cyclePeriodBoundary": "{name} zmienił początek lub koniec miesiączki", "cycleSymptomCreate": "{name} dodał objaw cyklu", "nutrientWaterWrite": "{name} zapisał ilość wypitej wody", - "healthRecordExport": "{name} wyeksportował kartę" + "healthRecordExport": "{name} wyeksportował kartę", + "rejectedMeasurementQuery": "{name} wysłał zapytanie o pomiary, którego ta karta nie mogła odczytać", + "rejectedDoseQuery": "{name} wysłał zapytanie o dawki, którego ta karta nie mogła odczytać", + "rejectedMoodQuery": "{name} wysłał zapytanie o nastrój, którego ta karta nie mogła odczytać", + "rejectedMeasurementCreate": "{name} próbował dodać pomiar i został odrzucony", + "rejectedMeasurementBatch": "{name} próbował dodać kilka pomiarów i został odrzucony", + "rejectedMeasurementUpdate": "{name} próbował zmienić pomiar i został odrzucony", + "rejectedLabCreate": "{name} próbował dodać wynik badania i został odrzucony", + "rejectedLabUpdate": "{name} próbował zmienić wynik badania i został odrzucony", + "rejectedBiomarkerCreate": "{name} próbował dodać biomarker i został odrzucony", + "rejectedBiomarkerUpdate": "{name} próbował zmienić biomarker i został odrzucony", + "rejectedReminderCreate": "{name} próbował dodać przypomnienie o pomiarze i został odrzucony", + "rejectedReminderUpdate": "{name} próbował zmienić przypomnienie o pomiarze i został odrzucony", + "rejectedMoodCreate": "{name} próbował dodać wpis nastroju i został odrzucony", + "rejectedMoodUpdate": "{name} próbował zmienić wpis nastroju i został odrzucony", + "rejectedDoseCreate": "{name} próbował zapisać przyjęcie dawki i został odrzucony", + "rejectedDoseUpdate": "{name} próbował zmienić przyjęcie dawki i został odrzucony", + "rejectedDoseImport": "{name} próbował zaimportować przyjęcia dawek i został odrzucony", + "rejectedTrackedValue": "{name} próbował zapisać własny pomiar i został odrzucony", + "deniedDoseImport": "Import przyjęć rozpoczęty przez {name} nie mógł zostać zakolejkowany", + "anamnesisFactCreate": "{name} dodał fakt z wywiadu zdrowotnego", + "customMetricCreate": "{name} dodał własny typ pomiaru", + "customMetricUpdate": "{name} zmienił własny typ pomiaru", + "customMetricDelete": "{name} usunął własny typ pomiaru", + "documentStore": "{name} dodał dokument", + "documentUpdate": "{name} zmienił dokument", + "documentDiscard": "{name} odrzucił dokument", + "medicationIntakeImportFinished": "Import przyjęć rozpoczęty przez {name} zakończył się", + "medicationOneShotReconciled": "Lek jednorazowy został otwarty lub zamknięty po dawce zapisanej przez {name}" }, "section": { "measurements": { diff --git a/src/__tests__/manage-activity-inventory.test.ts b/src/__tests__/manage-activity-inventory.test.ts index 6dace765f..626d32e58 100644 --- a/src/__tests__/manage-activity-inventory.test.ts +++ b/src/__tests__/manage-activity-inventory.test.ts @@ -1,5 +1,5 @@ -import { readFileSync, readdirSync } from "node:fs"; -import { join, relative } from "node:path"; +import { existsSync, readFileSync, readdirSync } from "node:fs"; +import { dirname, join, relative } from "node:path"; import ts from "typescript"; import { describe, expect, it } from "vitest"; @@ -53,7 +53,8 @@ import { recordActivityVerbLine } from "@/lib/record-activity/activity-verb"; * both halves against synthetic modules, including the one shape that silenced * the sibling guard: a resolver call prettier broke across lines. */ -const API = join(process.cwd(), "src/app/api"); +const SRC = join(process.cwd(), "src"); +const API = join(SRC, "app/api"); const MESSAGES = join(process.cwd(), "messages"); const ACTIVITY_ROUTE = join( process.cwd(), @@ -64,10 +65,31 @@ const GRANTS_GIVEN_CARD = join( "src/components/settings/access/grants-given-card.tsx", ); -/** The declaration a MANAGE-level route makes. */ +/** The declaration a record-delegated route makes, at any level. */ const RECORD_RESOLVER = "requireRecordAuth"; -/** The level this file is the inventory of. */ -const MANAGE = "manage"; + +/** + * Rows the feed cannot show, and the reason each one cannot. + * + * `src/lib/api-handler.ts` is a direct import of every route, so the walk + * below reaches the three actions the auth kit files itself. None of them can + * appear in a record owner's activity view, and the reason is the same for all + * three: they are filed under `auth.user.id` — the CALLER — at a point where + * no record has been resolved. The feed selects rows where `userId` is the + * owner and `actorUserId` is somebody else, so a caller-filed row is excluded + * twice over. + * + * Frozen at three. A fourth entry is a claim about a row nobody looked at, and + * the leg below fails rather than letting one join quietly. + */ +const NOT_IN_THE_FEED = [ + // The bearer path never resolves a record at all. + "auth.bearer.failure", + // Step-up is cookie-only and filed under the account being elevated. + "auth.stepup.elevation.rejected", + // `auditRefusal` files under the refused caller, before a record exists. + "sharing.access.denied", +] as const; /** What an action with no sentence of its own renders as. */ const FALLBACK_KEY = "recordSharing.activity.acted"; /** The six bundles the product ships. */ @@ -86,15 +108,24 @@ function routeFiles(dir: string): string[] { } interface ModuleFacts { - /** Does the module pass `"manage"` to the record resolver anywhere. */ - managesRecord: boolean; + /** Does the module reach the record resolver at ANY level. */ + delegatesRecord: boolean; /** - * The action names of the module's AWAITED `auditLog("…")` calls. + * Every action the module hands to `auditLog("…")`, awaited or not. * - * Awaited on purpose: a fire-and-forget breadcrumb on a validation failure - * is not something to narrate to the owner. + * v1.37.0 — this used to keep only the AWAITED calls, on the reasoning that + * a fire-and-forget breadcrumb is not worth narrating. The feed disagrees, + * and the feed is what the owner reads: `/api/account/activity` selects on + * `actorUserId IS NOT NULL` with no action filter and no await in sight, so + * a `void auditLog(…)` breadcrumb lands in the view exactly like an awaited + * row. Ten `*.validation-failed` actions were invisible to this guard for + * that reason and rendered to owners as "made a change in your record" — + * including one a READ delegate can trigger. A guard whose inclusion rule + * differs from its subject's proves a property about the wrong set. */ - awaitedAuditActions: string[]; + auditActions: string[]; + /** First-party modules this one imports, resolved to files. */ + imports: string[]; } /** @@ -119,8 +150,15 @@ export function moduleFacts(source: string, fileName: string): ModuleFacts { const aliases = new Map(); const namespaces = new Set(); + const importPaths: string[] = []; parsed.forEachChild((node) => { if (!ts.isImportDeclaration(node)) return; + if (ts.isStringLiteral(node.moduleSpecifier)) { + const resolved = resolveFirstParty(node.moduleSpecifier.text, fileName); + if (resolved !== null && !resolved.includes("__tests__")) { + importPaths.push(resolved); + } + } const bindings = node.importClause?.namedBindings; if (!bindings) return; if (ts.isNamespaceImport(bindings)) { @@ -151,28 +189,69 @@ export function moduleFacts(source: string, fileName: string): ModuleFacts { return first && ts.isStringLiteralLike(first) ? first.text : null; }; - let managesRecord = false; - const awaitedAuditActions: string[] = []; + let delegatesRecord = false; + const auditActions: string[] = []; const visit = (node: ts.Node): void => { if (ts.isCallExpression(node)) { const name = calleeName(node.expression); - if (name === RECORD_RESOLVER && firstStringArg(node) === MANAGE) { - managesRecord = true; - } - } - if (ts.isAwaitExpression(node) && ts.isCallExpression(node.expression)) { - const call = node.expression; - if (calleeName(call.expression) === "auditLog") { - const action = firstStringArg(call); - if (action !== null) awaitedAuditActions.push(action); + if (name === RECORD_RESOLVER) delegatesRecord = true; + if (name === "auditLog") { + const action = firstStringArg(node); + if (action !== null) auditActions.push(action); } } node.forEachChild(visit); }; parsed.forEachChild(visit); - return { managesRecord, awaitedAuditActions }; + return { delegatesRecord, auditActions, imports: importPaths }; +} + +/** + * A module specifier resolved to a first-party file, or null. + * + * `@/…` and relative paths only. A package import is somebody else's tree and + * cannot be walked usefully. + */ +export function resolveFirstParty( + specifier: string, + fromFile: string, +): string | null { + let base: string; + if (specifier.startsWith("@/")) base = join(SRC, specifier.slice(2)); + else if (specifier.startsWith(".")) base = join(dirname(fromFile), specifier); + else return null; + for (const candidate of [ + `${base}.ts`, + `${base}.tsx`, + join(base, "index.ts"), + ]) { + if (existsSync(candidate)) return candidate; + } + return null; +} + +/** + * Every action one route module can file, including through what it imports. + * + * One level deep, deliberately. The transitive closure pulls in the whole auth + * and grant machinery through `api-handler`, which reports actions no request + * to this route can file; one level is where a route's own collaborators live + * — the import-admission module and the medication lifecycle module are both + * direct imports of the routes that use them, and both were invisible to a + * walk that read only `route.ts`. + */ +function actionsReachableFrom( + file: string, + read: (path: string) => ModuleFacts, +): string[] { + const own = read(file); + const out = [...own.auditActions]; + for (const imported of own.imports) { + out.push(...read(imported).auditActions); + } + return out; } /** The message key one action resolves to, or the fallback. */ @@ -232,8 +311,8 @@ describe("the matcher sees what it looks for", () => { // own line and a substring test goes quiet with nothing else changed. expect(REFORMATTED.includes('requireRecordAuth("manage"')).toBe(false); const facts = moduleFacts(REFORMATTED, "r.ts"); - expect(facts.managesRecord).toBe(true); - expect(facts.awaitedAuditActions).toEqual(["thing.delete"]); + expect(facts.delegatesRecord).toBe(true); + expect(facts.auditActions).toEqual(["thing.delete"]); }); it("matches an aliased and a namespace-qualified resolver call", () => { @@ -247,10 +326,9 @@ describe("the matcher sees what it looks for", () => { return apiSuccess({ id: user.id }); }); `; - expect(moduleFacts(ALIASED, "a.ts")).toEqual({ - managesRecord: true, - awaitedAuditActions: ["thing.aliased"], - }); + const facts = moduleFacts(ALIASED, "a.ts"); + expect(facts.delegatesRecord).toBe(true); + expect(facts.auditActions).toEqual(["thing.aliased"]); const NAMESPACED = ` import * as auth from "@/lib/api-handler"; @@ -260,27 +338,28 @@ describe("the matcher sees what it looks for", () => { return apiSuccess({ id: user.id }); }); `; - expect(moduleFacts(NAMESPACED, "n.ts").managesRecord).toBe(true); + expect(moduleFacts(NAMESPACED, "n.ts").delegatesRecord).toBe(true); }); - it("does not enrol a write route that only talks about managing", () => { - const WRITE_ONLY = ` - /** Delegable for writes. It must never take requireRecordAuth("manage"). */ - import { requireRecordAuth } from "@/lib/api-handler"; - - const LEVEL = "manage"; + it("does not enrol a route that only talks about delegating", () => { + const ACTOR_ONLY = ` + /** An actor surface. It must never take requireRecordAuth("manage"). */ + import { requireActorAuth } from "@/lib/api-handler"; export const POST = apiHandler(async () => { - const { user } = await requireRecordAuth("write", "labs"); - return apiSuccess({ id: user.id, level: LEVEL }); + const { user } = await requireActorAuth(); + return apiSuccess({ id: user.id }); }); `; // The raw text carries the exact substring a naive membership test uses. - expect(WRITE_ONLY.includes('requireRecordAuth("manage")')).toBe(true); - expect(moduleFacts(WRITE_ONLY, "wo.ts").managesRecord).toBe(false); + expect(ACTOR_ONLY.includes('requireRecordAuth("manage")')).toBe(true); + expect(moduleFacts(ACTOR_ONLY, "ao.ts").delegatesRecord).toBe(false); }); - it("reads only the awaited audit calls", () => { + it("reads the fire-and-forget audit calls too", () => { + // The defect this file shipped with. `void auditLog(…)` was invisible + // here and perfectly visible in the owner's feed, so ten refusal + // breadcrumbs rendered as "made a change in your record". const MIXED = ` import { requireRecordAuth } from "@/lib/api-handler"; import { auditLog } from "@/lib/auth/audit"; @@ -292,11 +371,28 @@ describe("the matcher sees what it looks for", () => { return apiSuccess({ id: user.id }); }); `; - expect(moduleFacts(MIXED, "m.ts").awaitedAuditActions).toEqual([ + expect(moduleFacts(MIXED, "m.ts").auditActions).toEqual([ + "thing.breadcrumb", "thing.delete", ]); }); + it("resolves a first-party import and skips a package one", () => { + // The other half of the same defect: two of the twelve unmapped actions + // were filed by library modules the routes import, which a walk over + // `route.ts` alone could not see. + const admission = resolveFirstParty( + "@/lib/medications/intake-import-admission", + join(API, "medications/route.ts"), + ); + expect(admission).not.toBeNull(); + expect(readFileSync(admission!, "utf8")).toContain( + "medication.intake.import.kickoff.denied", + ); + expect(resolveFirstParty("next/server", "x.ts")).toBeNull(); + expect(resolveFirstParty("@/lib/nothing-here", "x.ts")).toBeNull(); + }); + it("still reports the fallback for an action nobody mapped", () => { // The positive control for the recording translator below. Without it, a // `verbKeyFor` that always returned the same string would report every @@ -305,30 +401,62 @@ describe("the matcher sees what it looks for", () => { }); }); -describe("every MANAGE action has a sentence of its own", () => { +describe("every delegated action has a sentence of its own", () => { const routes = routeFiles(API); - const manageModules = routes - .map((file) => ({ file, ...moduleFacts(readFileSync(file, "utf8"), file) })) - .filter((m) => m.managesRecord); - const actions = [ - ...new Set(manageModules.flatMap((m) => m.awaitedAuditActions)), - ].sort(); + const cache = new Map(); + const read = (file: string): ModuleFacts => { + let facts = cache.get(file); + if (!facts) { + facts = moduleFacts(readFileSync(file, "utf8"), file); + cache.set(file, facts); + } + return facts; + }; - it("matches the MANAGE modules, and not zero of them", () => { + const delegatedModules = routes.filter((file) => read(file).delegatesRecord); + const excluded = new Set(NOT_IN_THE_FEED); + const reachable = [ + ...new Set( + delegatedModules.flatMap((file) => actionsReachableFrom(file, read)), + ), + ].sort(); + const actions = reachable.filter((action) => !excluded.has(action)); + + it("matches the delegated modules, and not zero of them", () => { // A walk that finds nothing agrees with an inventory that covers nothing. expect(routes.length).toBeGreaterThan(300); - expect(manageModules.length).toBeGreaterThan(0); + expect(delegatedModules.length).toBeGreaterThan(0); // Named anchors, so a matcher that collapsed to one stray file fails // rather than clearing the floor. Deleting a reading and purging a dose - // history are the two acts the level exists for and the two an owner most - // needs named. - const found = manageModules.map((m) => relative(process.cwd(), m.file)); + // history are the two acts MANAGE exists for; the mood list is the READ + // route whose refusal breadcrumb reaches the feed, and it is here because + // the level is no longer part of the inclusion rule. + const found = delegatedModules.map((file) => relative(process.cwd(), file)); expect(found).toContain("src/app/api/measurements/[id]/route.ts"); expect(found).toContain( "src/app/api/medications/[id]/intake/purge/route.ts", ); + expect(found).toContain("src/app/api/mood-entries/route.ts"); // And the actions themselves were read, not merely looked for. - expect(actions.length).toBeGreaterThan(40); + expect(actions.length).toBeGreaterThan(60); + }); + + it("reaches the actions a route files through what it imports", () => { + // Both halves of the fix, pinned against the two that were invisible. + expect(actions).toContain("medication.intake.import.kickoff.denied"); + expect(actions).toContain("medication.oneShot.reconciled"); + // And the fire-and-forget half, on the READ route that started this. + expect(actions).toContain("mood-entries.list.validation-failed"); + }); + + it("excludes exactly the three rows the feed cannot show", () => { + // The exclusion list is the one place a real action could be hidden, so + // its size is pinned and every member is asserted to have been REACHED — + // an entry naming an action nothing files would be a claim about nothing. + expect(NOT_IN_THE_FEED).toHaveLength(3); + for (const action of NOT_IN_THE_FEED) { + expect(reachable, action).toContain(action); + } }); it("gives every one of them a specific verb rather than the fallback", () => { @@ -338,12 +466,46 @@ describe("every MANAGE action has a sentence of its own", () => { expect( generic, - "a MANAGE action with no verb line renders to the owner as the generic " + - "'made a change'. Add a case to activity-verb.ts and a sentence to " + - "all six bundles.", + "a delegated action with no verb line renders to the owner as the " + + "generic 'made a change'. Add a case to activity-verb.ts and a " + + "sentence to all six bundles — or, if the row genuinely should not " + + "reach the feed, stop stamping the actor on it.", ).toEqual([]); }); + it("never reports a refused attempt as something that happened", () => { + // The half of the HIGH finding that a coverage count cannot express. Every + // one of these rows is an attempt that changed NOTHING, and the generic + // line told the owner their record had been edited. A future arm that maps + // `…validation-failed` onto an ordinary verb would be the same defect with + // coverage, so the shape of the key is asserted rather than its presence. + const attempts = actions.filter( + (action) => + action.endsWith(".validation-failed") || action.endsWith(".denied"), + ); + expect(attempts.length).toBeGreaterThan(10); + + const dishonest = attempts.filter( + (action) => !/\.(rejected|denied)[A-Z]/.test(verbKeyFor(action).key), + ); + expect( + dishonest, + "an attempt that was refused must not render as an act that happened", + ).toEqual([]); + + // And the sentences themselves say so, in the source language. A key named + // `rejected…` whose EN text read "added a reading" would pass the check + // above and be exactly the lie it exists to stop. + const en = bundle("en"); + for (const action of attempts) { + const sentence = leaf(en, verbKeyFor(action).key); + expect(typeof sentence, action).toBe("string"); + expect(String(sentence), action).toMatch( + /tried to|could not|was refused|search this record/, + ); + } + }); + it("resolves every verb key in all six bundles", () => { const bundles = new Map(LOCALES.map((l) => [l, bundle(l)] as const)); const missing: string[] = []; diff --git a/src/lib/record-activity/activity-verb.ts b/src/lib/record-activity/activity-verb.ts index d4c78eab0..3d4b1f334 100644 --- a/src/lib/record-activity/activity-verb.ts +++ b/src/lib/record-activity/activity-verb.ts @@ -256,6 +256,111 @@ export function recordActivityVerbLine( case "health-record.export": return t("recordSharing.activityVerb.healthRecordExport", { name }); + /* ---------------------------------------------------------------- */ + /* Attempts that were REFUSED, and things that happened around them. */ + /* */ + /* Every delegable route files a `*.validation-failed` breadcrumb */ + /* when a request does not parse, and it files it through */ + /* `auditLog()` deliberately — that helper is what stamps */ + /* `actorUserId`, and without the stamp a delegate's malformed */ + /* request would read as the owner's own. So the rows are MEANT to */ + /* reach the feed, and until this block existed every one of them */ + /* rendered as "made a change in your record". Nothing changed. The */ + /* one on a READ route is the sharpest case: a person with */ + /* view-only access was reported to the owner as having edited */ + /* something. */ + /* */ + /* The language below never says "changed" or "added". A refusal is */ + /* an attempt, and the sentence has to be one an owner can read */ + /* without reaching for a phone. */ + /* ---------------------------------------------------------------- */ + + case "measurements.list.validation-failed": + case "measurements.series.validation-failed": + return t("recordSharing.activityVerb.rejectedMeasurementQuery", { name }); + case "medications.intake.list.validation-failed": + return t("recordSharing.activityVerb.rejectedDoseQuery", { name }); + case "mood-entries.list.validation-failed": + return t("recordSharing.activityVerb.rejectedMoodQuery", { name }); + + case "measurements.create.validation-failed": + return t("recordSharing.activityVerb.rejectedMeasurementCreate", { + name, + }); + case "measurements.create.batch.validation-failed": + return t("recordSharing.activityVerb.rejectedMeasurementBatch", { name }); + case "measurements.update.validation-failed": + return t("recordSharing.activityVerb.rejectedMeasurementUpdate", { + name, + }); + case "labs.create.validation-failed": + return t("recordSharing.activityVerb.rejectedLabCreate", { name }); + case "labs.update.validation-failed": + return t("recordSharing.activityVerb.rejectedLabUpdate", { name }); + case "labs.biomarker.create.validation-failed": + return t("recordSharing.activityVerb.rejectedBiomarkerCreate", { name }); + case "labs.biomarker.update.validation-failed": + return t("recordSharing.activityVerb.rejectedBiomarkerUpdate", { name }); + case "measurement-reminders.create.validation-failed": + return t("recordSharing.activityVerb.rejectedReminderCreate", { name }); + case "measurement-reminders.update.validation-failed": + return t("recordSharing.activityVerb.rejectedReminderUpdate", { name }); + case "mood-entries.create.validation-failed": + return t("recordSharing.activityVerb.rejectedMoodCreate", { name }); + case "mood-entries.update.validation-failed": + return t("recordSharing.activityVerb.rejectedMoodUpdate", { name }); + case "medications.intake.create.validation-failed": + return t("recordSharing.activityVerb.rejectedDoseCreate", { name }); + // Both spellings of the same refusal, for the same reason the admitted + // pair above carries both: the canonical route and the per-medication + // one audit under different names and have since v1.0.0. + case "medications.intake.update.validation-failed": + case "medications.intake.event.update.validation-failed": + return t("recordSharing.activityVerb.rejectedDoseUpdate", { name }); + case "medications.intake.import.validation-failed": + return t("recordSharing.activityVerb.rejectedDoseImport", { name }); + case "custom-metric.entry.create.validation-failed": + return t("recordSharing.activityVerb.rejectedTrackedValue", { name }); + // Not a validation failure: the request was well-formed and admitted, + // and the queue would not take it. The owner sees an import that never + // ran rather than one that was refused, because those need different + // next steps. + case "medication.intake.import.kickoff.denied": + return t("recordSharing.activityVerb.deniedDoseImport", { name }); + + /* ---------------------------------------------------------------- */ + /* Acts a delegable module performs outside its own route file. */ + /* */ + /* The two below are filed by library modules the routes import — a */ + /* background worker's completion row and a lifecycle reconciliation */ + /* — and a guard that walked only `route.ts` could not see either. */ + /* Both carry the actor through, so both reach the owner's feed. */ + /* ---------------------------------------------------------------- */ + + case "medication.intake.import": + return t("recordSharing.activityVerb.medicationIntakeImportFinished", { + name, + }); + case "medication.oneShot.reconciled": + return t("recordSharing.activityVerb.medicationOneShotReconciled", { + name, + }); + + case "anamnesis.fact.create": + return t("recordSharing.activityVerb.anamnesisFactCreate", { name }); + case "customMetric.create": + return t("recordSharing.activityVerb.customMetricCreate", { name }); + case "customMetric.update": + return t("recordSharing.activityVerb.customMetricUpdate", { name }); + case "customMetric.delete": + return t("recordSharing.activityVerb.customMetricDelete", { name }); + case "documents.inbound.store": + return t("recordSharing.activityVerb.documentStore", { name }); + case "documents.inbound.update": + return t("recordSharing.activityVerb.documentUpdate", { name }); + case "documents.inbound.discard": + return t("recordSharing.activityVerb.documentDiscard", { name }); + default: return t("recordSharing.activity.acted", { name }); } diff --git a/tests/integration/sharing-audit-actor.test.ts b/tests/integration/sharing-audit-actor.test.ts index 80cc88377..d3499c2e5 100644 --- a/tests/integration/sharing-audit-actor.test.ts +++ b/tests/integration/sharing-audit-actor.test.ts @@ -14,6 +14,9 @@ */ import { beforeEach, describe, expect, it, vi } from "vitest"; +import { readFile } from "node:fs/promises"; +import { join } from "node:path"; + import { NextRequest, NextResponse } from "next/server"; import { cookieJar, headerJar } from "./mock-next-headers"; @@ -330,3 +333,74 @@ describe("the lifecycle verbs leave their own trail", () => { expect(trail.every((r) => r.actorUserId === null)).toBe(true); }); }); + +/** + * A view-only delegate sends a query the route cannot parse. + * + * This is the sharpest case the activity feed has, and it shipped saying the + * wrong thing. `GET /api/mood-entries` resolves `requireRecordAuth("read", + * "mind")` and files `mood-entries.list.validation-failed` through `auditLog` + * on a malformed query — deliberately through the helper, because that is what + * stamps `actorUserId` and the row is meant to be attributable. The feed + * selects every actor-stamped row with no action allowlist, and the verb map + * had no arm for any `*.validation-failed` action, so the owner was told that + * a person holding VIEW-ONLY access had made a change in their record. + * + * Nothing had changed. Nothing could have: the request was refused at the + * schema. So the whole round trip is asserted here rather than only the map — + * that the row lands, that it is attributed, and that the sentence the owner + * would read is an attempt rather than an act. + */ +describe("a refused read is reported as a refused read", () => { + it("files an attributable row and renders it as an attempt, not a change", async () => { + const { owner, delegate } = await household(); + + const { GET } = await import("@/app/api/mood-entries/route"); + const response = await GET( + new NextRequest("http://localhost/api/mood-entries?limit=not-a-number", { + method: "GET", + }), + ); + expect(response.status).toBe(422); + + // The breadcrumb is fire-and-forget, which is the property that made it + // invisible to a guard reading only awaited calls. + await settle(); + const rows = await rowsFor("mood-entries.list.validation-failed"); + expect(rows, "no breadcrumb row was written").toHaveLength(1); + expect(rows[0]?.userId).toBe(owner.id); + expect(rows[0]?.actorUserId).toBe(delegate.id); + + // Which means it reaches the owner's feed: same predicate the route uses. + const feedRows = await getPrismaClient().auditLog.findMany({ + where: { + userId: owner.id, + actorUserId: { not: null }, + NOT: { actorUserId: owner.id }, + }, + }); + expect(feedRows.map((r) => r.action)).toContain( + "mood-entries.list.validation-failed", + ); + + // And what it says there. The English bundle is read rather than a key + // asserted, because the defect was the SENTENCE, not the wiring. + const { recordActivityVerbLine } = + await import("@/lib/record-activity/activity-verb"); + const en = JSON.parse( + await readFile(join(process.cwd(), "messages/en.json"), "utf8"), + ) as Record>>; + const sentence = recordActivityVerbLine( + (key, params) => + (en.recordSharing.activityVerb[key.split(".").pop()!] ?? key).replace( + "{name}", + String(params?.name ?? ""), + ), + rows[0]!.action, + "Alex", + ); + + expect(sentence).not.toContain("made a change"); + expect(sentence).toBe("Alex sent a mood search this record could not read"); + }); +}); From b1667ef69242b3d50e878110061360ed74949d63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 7 Aug 2026 00:43:39 +0200 Subject: [PATCH 6/9] fix: let the audit window an operator sets reach the container MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AUDIT_LOG_RETENTION_DAYS is not only enforced, it is published: the shared-access panel prints the window under the activity list, and the dialog that ends somebody else's access states how long "who entered what" stays answerable. Both read the value the server resolved. It was missing from the compose environment: whitelist and from both example files, so an operator who set 90 got 365 told to every user, in a sentence about their own data. That block is a whitelist and Compose substitutes from .env regardless, which is what makes the variable look configured while never arriving — the same shape as the session-cookie flag in v1.5.2. A guard now covers the three places an operator meets such a variable, and its control leg found that the cookie flag itself is still absent from the production example. Documented there too. --- .env.example | 14 +++ .env.production.example | 26 +++++ docker-compose.yml | 10 ++ ...ublished-env-reaches-the-container.test.ts | 101 ++++++++++++++++++ 4 files changed, 151 insertions(+) create mode 100644 src/__tests__/published-env-reaches-the-container.test.ts diff --git a/.env.example b/.env.example index 20f090f7b..e1bc76e38 100644 --- a/.env.example +++ b/.env.example @@ -210,6 +210,20 @@ API_TOKEN_HMAC_KEY="REPLACE-WITH-OUTPUT-OF-openssl-rand-hex-32" # BACKUP_ENCRYPTION_KEY="" # 64-hex-char or base64 32-byte; SEPARATE from ENCRYPTION_KEY +# ----------------------------------------------------------------------------- +# Audit-trail retention (optional) +# ----------------------------------------------------------------------------- +# How long rows in `audit_logs` are kept. Default 365 days; the floor is 7 and +# anything below it is ignored, so a window set in seconds by mistake cannot +# empty a fresh table. A daily pg-boss job does the pruning. +# +# The number is not internal. Settings -> Shared access states it under the +# activity list, and the dialog that ends somebody's access says how long +# "who entered what" stays answerable. Both read the value the server +# resolved, so an instance running 90 says 90 everywhere. +# AUDIT_LOG_RETENTION_DAYS="365" + + # ----------------------------------------------------------------------------- # APNs — Apple Push Notification service (v1.4.23+, optional) # ----------------------------------------------------------------------------- diff --git a/.env.production.example b/.env.production.example index 76c50df27..93aa99e1e 100644 --- a/.env.production.example +++ b/.env.production.example @@ -184,6 +184,17 @@ API_TOKEN_HMAC_KEY="" # SMTP_SECURE="false" +# ----------------------------------------------------------------------------- +# Session cookie Secure flag -- the most common self-host snag +# ----------------------------------------------------------------------------- +# Unset, the flag tracks NODE_ENV, so a production image sets `Secure` and a +# browser on plain HTTP silently drops the session cookie: login appears to +# succeed and every following request is anonymous. Set it to false ONLY on a +# LAN, Tailscale or VPN-only host that deliberately serves HTTP. Behind a TLS +# reverse proxy, leave it alone. +# SESSION_COOKIE_SECURE="false" + + # ----------------------------------------------------------------------------- # Deploy webhook -- Coolify -> HealthLog auto-deploy feedback # ----------------------------------------------------------------------------- @@ -230,6 +241,21 @@ API_TOKEN_HMAC_KEY="" # BACKUP_ENCRYPTION_KEY="" +# ----------------------------------------------------------------------------- +# Audit-trail retention -- optional +# ----------------------------------------------------------------------------- +# How long rows in `audit_logs` survive, in days. Default 365. The floor is 7: +# a smaller value is ignored rather than obeyed, so a window entered in seconds +# by mistake cannot empty a fresh table. A daily job does the pruning. +# +# This number is shown to users, not just enforced. Settings -> Shared access +# prints the window under the activity list, and the dialog that ends somebody +# else's access states how long "who entered what" stays answerable. Both take +# the value the server resolved from this variable, so an instance running 90 +# says 90 on both surfaces. +# AUDIT_LOG_RETENTION_DAYS="365" + + # ----------------------------------------------------------------------------- # FHIR health-record export -- optional # ----------------------------------------------------------------------------- diff --git a/docker-compose.yml b/docker-compose.yml index 503e87ed8..345c19d88 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -98,6 +98,16 @@ x-healthlog-env: &healthlog_env BACKUP_S3_SECRET_KEY: "${BACKUP_S3_SECRET_KEY:-}" BACKUP_S3_REGION: "${BACKUP_S3_REGION:-auto}" BACKUP_RETENTION_DAYS: "${BACKUP_RETENTION_DAYS:-30}" + # How long the audit trail is kept, in days. Default 365, floor 7 (a value + # below the floor is ignored rather than obeyed, so a window set in seconds + # by mistake cannot empty a fresh table). It is not only a retention knob: + # the shared-access panel PUBLISHES this number — "who entered what stays + # answerable for N days" is on the dialog that ends somebody's access, and + # the activity view states the same window. An instance running 90 says 90, + # so the var has to actually reach the container. Listed here for the reason + # the block exists: `environment:` is a whitelist, and a value in .env that + # is not named here never arrives. + AUDIT_LOG_RETENTION_DAYS: "${AUDIT_LOG_RETENTION_DAYS:-}" # Per-export ceiling on MedicationAdministration rows in the FHIR # health-record Bundle. Bounded 1..50000, default 5000. Listed here so # an operator override in .env actually reaches the container (the diff --git a/src/__tests__/published-env-reaches-the-container.test.ts b/src/__tests__/published-env-reaches-the-container.test.ts new file mode 100644 index 000000000..1d7ca6afa --- /dev/null +++ b/src/__tests__/published-env-reaches-the-container.test.ts @@ -0,0 +1,101 @@ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +import { describe, expect, it } from "vitest"; + +/** + * An environment variable the product SHOWS a user has to reach the container. + * + * ## The failure this exists for + * + * `docker-compose.yml`'s `environment:` block is a whitelist. Docker Compose + * reads `.env` for `${VAR}` substitution, so a value an operator sets there + * appears to be configured, and a variable not named in the block never + * arrives in the process. v1.5.2 shipped that exact defect for + * `SESSION_COOKIE_SECURE`: documented, settable, and silently absent, which on + * a plain-HTTP self-host reads as "login is broken". + * + * `AUDIT_LOG_RETENTION_DAYS` is a sharper version of the same class, because + * the number is not merely enforced — it is PUBLISHED. `GET + * /api/account/grants` and `GET /api/account/activity` both send it, the + * activity list prints it, and the dialog that ends somebody's access states + * how long "who entered what" stays answerable. An operator who sets 90 and + * does not get it told every user 365, in a sentence about their own data. + * + * ## What this checks, and what it cannot + * + * The three places an operator meets a variable: the compose whitelist, the + * self-host example, and the production example. It cannot check that the + * value is honoured — `getAuditLogRetentionDays()` has its own tests — only + * that a value set the documented way is not dropped on the floor between + * `.env` and the process. + * + * The list is deliberately short. It is not every variable in the manifest: + * `scripts/check-env.ts` owns that question and CI runs it against + * `.env.production.example`. This is the narrower one that check-env cannot + * answer, because it reads a file rather than the compose whitelist. + */ +const ROOT = process.cwd(); +const COMPOSE = readFileSync(join(ROOT, "docker-compose.yml"), "utf8"); +const ENV_EXAMPLE = readFileSync(join(ROOT, ".env.example"), "utf8"); +const PROD_EXAMPLE = readFileSync( + join(ROOT, ".env.production.example"), + "utf8", +); + +/** + * Variables whose value the product states back to a user. + * + * A variable earns a place here by being readable somewhere in the interface, + * not by being important. That is the property that makes a missing whitelist + * entry a lie rather than an inconvenience. + */ +const PUBLISHED_TO_USERS = [ + { + name: "AUDIT_LOG_RETENTION_DAYS", + shownBy: "the shared-access activity window and the revoke dialog", + }, + { + name: "SESSION_COOKIE_SECURE", + shownBy: + "nothing directly — the original of this defect class, kept as a control", + }, +] as const; + +describe("a variable the product publishes reaches the container", () => { + it("has a matcher that can fail", () => { + // The pins. Every leg below asserts a substring is PRESENT, and a file + // that failed to load would be an empty string in which nothing is + // present — so the failure would be loud rather than silent. These make + // the fixtures' own size the evidence. + expect(PUBLISHED_TO_USERS.length).toBeGreaterThan(1); + expect(COMPOSE).toContain("environment:"); + expect(ENV_EXAMPLE.length).toBeGreaterThan(1000); + expect(PROD_EXAMPLE.length).toBeGreaterThan(1000); + // And a name nothing declares is genuinely absent from all three, so the + // matchers below are not passing on a substring that is everywhere. + for (const file of [COMPOSE, ENV_EXAMPLE, PROD_EXAMPLE]) { + expect(file).not.toContain("HEALTHLOG_NOT_A_REAL_VARIABLE"); + } + }); + + for (const { name, shownBy } of PUBLISHED_TO_USERS) { + describe(name, () => { + it(`is on the compose whitelist, so a value in .env arrives (${shownBy})`, () => { + // The substitution form, not merely the name: a bare mention in a + // comment is how this class of defect hides. + expect(COMPOSE).toMatch( + new RegExp(`^\\s*${name}:\\s*"\\$\\{${name}`, "m"), + ); + }); + + it("is documented in the self-host example", () => { + expect(ENV_EXAMPLE).toContain(name); + }); + + it("is documented in the production example", () => { + expect(PROD_EXAMPLE).toContain(name); + }); + }); + } +}); From c7e847a7661334903276806121e236d76f0c16f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 7 Aug 2026 00:48:58 +0200 Subject: [PATCH 7/9] fix: bound the activity feed to the window it promises MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The revoke dialog tells an owner that "who entered what" stays answerable for N days, and the activity list prints the same number under its heading. The query behind that list had no date filter: on an instance whose retention was shortened it showed rows the sentence said were already gone, and the number was a caption over an unbounded read rather than its limit. It is the filter now, resolved once so the bound and the sentence cannot drift apart. The list also stopped at a hundred rows and said nothing about it, which is the more dangerous of the two silences — an owner who scrolls to the bottom reads the oldest visible line as the beginning, and a busy household reaches the cap in a fraction of the window. The response says which of the two limits it hit, and the view states it only when it did: a caveat that is always on is one people learn to skip. The self-hosting page said an empty list means nobody opened the record. It means nobody did so within the window. Corrected, with the row cap named alongside it. --- docs/api/openapi.yaml | 10 +- docs/self-hosting/account-sharing.md | 18 +++- messages/de.json | 3 +- messages/en.json | 3 +- messages/es.json | 3 +- messages/fr.json | 3 +- messages/it.json | 3 +- messages/pl.json | 3 +- src/app/api/account/activity/route.ts | 38 +++++++- .../access/__tests__/access-panel.test.tsx | 24 ++++- .../settings/access/record-activity-card.tsx | 14 +++ src/lib/openapi/routes/account-sharing.ts | 7 +- src/lib/queries/use-account-grants.ts | 9 ++ tests/integration/sharing-audit-actor.test.ts | 91 +++++++++++++++++++ 14 files changed, 213 insertions(+), 16 deletions(-) diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 8fbadf170..89fe122e2 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -36916,11 +36916,17 @@ components: minimum: -9007199254740991 maximum: 9007199254740991 description: How far back this feed can see, in days. Resolved from the instance's audit retention setting (365 by - default, operator-configurable), NOT a constant — an instance running a 90-day window sends 90. Show it - beside the rows. + default, operator-configurable), NOT a constant — an instance running a 90-day window sends 90. The query is + bounded to this window, so it is the real limit of the list and not only a caption. Show it beside the rows. + truncated: + type: boolean + description: True when the read reached its row ceiling, so `entries` is the most recent activity inside the window + rather than all of it. A client must say so — a list that stops at its own cap in silence lets the oldest + row read as the beginning. False means the list is everything inside the window. required: - entries - retentionDays + - truncated additionalProperties: false description: The owner's record of what somebody else did in their record, bounded by the instance's audit retention. RecordActivityEntry: diff --git a/docs/self-hosting/account-sharing.md b/docs/self-hosting/account-sharing.md index 46222f719..4b5190c31 100644 --- a/docs/self-hosting/account-sharing.md +++ b/docs/self-hosting/account-sharing.md @@ -104,9 +104,21 @@ Below that, a list of the days somebody else opened it and how many times That list is bounded by the instance's audit retention, and the page says so with the actual number rather than a hardcoded one. The default is 365 days (`AUDIT_LOG_RETENTION_DAYS`); an operator who shortens it shortens -this view too, and the sentence under the heading changes to match. An -empty list means nobody opened the record within that window — not that -nobody ever did. +this view too, and the sentence under the heading changes to match. The +query is bounded to that window, so the sentence is the real limit of the +list rather than a caption over an unbounded one. + +Two things the list does not claim, both worth reading before treating it +as an answer: + +- **An empty list is not proof that nobody opened the record.** It means + nobody did so within the window, and that any earlier row has been + purged. +- **The list is capped.** It returns at most the hundred most recent rows + inside the window; when it hits that ceiling it says so under the + heading. A busy household can reach the cap in far less time than the + retention window, and the oldest line on screen is then the oldest line + shown, not the oldest that exists. ## What this does not change about the instance diff --git a/messages/de.json b/messages/de.json index 823ce93a1..82318dc1f 100644 --- a/messages/de.json +++ b/messages/de.json @@ -9627,7 +9627,8 @@ "loadError": "Die Aktivität konnte nicht geladen werden.", "opened": "{name} hat deine Akte {count}× geöffnet", "acted": "{name} hat etwas in deiner Akte geändert", - "deletedAccount": "Ein gelöschtes Konto" + "deletedAccount": "Ein gelöschtes Konto", + "truncated": "Diese Liste zeigt die neueste Aktivität, nicht die gesamte." }, "delegate": { "addOnlyNote": "Du kannst zu dieser Akte etwas hinzufügen. Einträge ändern oder löschen kann nur {name}." diff --git a/messages/en.json b/messages/en.json index 2225ac156..af1f56262 100644 --- a/messages/en.json +++ b/messages/en.json @@ -9627,7 +9627,8 @@ "loadError": "The activity could not be loaded.", "opened": "{name} opened your record {count}×", "acted": "{name} made a change in your record", - "deletedAccount": "A deleted account" + "deletedAccount": "A deleted account", + "truncated": "This list shows the most recent activity, not all of it." }, "delegate": { "addOnlyNote": "You can add to this record. Changing or removing entries is something only {name} can do." diff --git a/messages/es.json b/messages/es.json index 11b290b17..49c232874 100644 --- a/messages/es.json +++ b/messages/es.json @@ -9627,7 +9627,8 @@ "loadError": "No se pudo cargar la actividad.", "opened": "{name} abrió tu historial {count}×", "acted": "{name} hizo un cambio en tu historial", - "deletedAccount": "Una cuenta eliminada" + "deletedAccount": "Una cuenta eliminada", + "truncated": "Esta lista muestra la actividad más reciente, no toda." }, "delegate": { "addOnlyNote": "Puedes añadir cosas a este historial. Modificar o borrar entradas solo lo puede hacer {name}." diff --git a/messages/fr.json b/messages/fr.json index 664d71d1f..97cb05c19 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -9627,7 +9627,8 @@ "loadError": "L'activité n'a pas pu être chargée.", "opened": "{name} a ouvert votre dossier {count}×", "acted": "{name} a fait une modification dans votre dossier", - "deletedAccount": "Un compte supprimé" + "deletedAccount": "Un compte supprimé", + "truncated": "Cette liste montre l'activité la plus récente, pas la totalité." }, "delegate": { "addOnlyNote": "Vous pouvez ajouter des éléments à ce dossier. Seul {name} peut modifier ou supprimer des entrées." diff --git a/messages/it.json b/messages/it.json index 90cd6c579..c5c97cfc3 100644 --- a/messages/it.json +++ b/messages/it.json @@ -9627,7 +9627,8 @@ "loadError": "Non è stato possibile caricare l'attività.", "opened": "{name} ha aperto la tua cartella {count}×", "acted": "{name} ha fatto una modifica nella tua cartella", - "deletedAccount": "Un account eliminato" + "deletedAccount": "Un account eliminato", + "truncated": "Questo elenco mostra l'attività più recente, non tutta." }, "delegate": { "addOnlyNote": "Puoi aggiungere voci a questa cartella. Solo {name} può modificarle o eliminarle." diff --git a/messages/pl.json b/messages/pl.json index bdbd39496..f7e6d23a1 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -9627,7 +9627,8 @@ "loadError": "Nie udało się wczytać aktywności.", "opened": "{name} otworzył twoją kartę {count}×", "acted": "{name} wprowadził zmianę w twojej karcie", - "deletedAccount": "Usunięte konto" + "deletedAccount": "Usunięte konto", + "truncated": "Ta lista pokazuje najnowszą aktywność, a nie całą." }, "delegate": { "addOnlyNote": "Możesz dodawać wpisy do tej karty. Zmieniać je i usuwać może tylko {name}." diff --git a/src/app/api/account/activity/route.ts b/src/app/api/account/activity/route.ts index 993cf473a..26ca3deb2 100644 --- a/src/app/api/account/activity/route.ts +++ b/src/app/api/account/activity/route.ts @@ -16,6 +16,20 @@ * purge job uses, so the sentence in the UI cannot drift from the job that * makes it true — an instance running a 90-day window says 90. * + * **And the window is now enforced, not only stated.** The query is bounded to + * it. Without the filter the two halves of the same sentence disagreed: the + * revoke dialog promised attribution "for the next N days" while this read + * returned whatever the last hundred rows happened to be, which on a quiet + * record reaches back past a shortened retention window and on a busy one + * covers a fortnight. The bound the copy states is the bound the query uses. + * + * **The row cap says when it was hit.** A hundred rows is a display bound, and + * a list that silently stops at its own ceiling is the more dangerous of the + * two completeness lies — an owner scrolling to the bottom reads the oldest + * line as the beginning. `truncated` is true when the cap was reached, and the + * view says it is showing the most recent activity rather than all of it. + * `false` means the list IS everything inside the window. + * * Not delegable: bare `requireAuth()` refuses under a switch. A delegate must * not be able to read the owner's record of who has been in it — that list * names other delegates, and a grant is not an introduction to the household. @@ -34,6 +48,9 @@ export const dynamic = "force-dynamic"; * How many rows the feed returns. One row per (delegate, day) for reads, so a * hundred covers months of a busy household — and a longer list answers a * question nobody asked while making the recent activity harder to see. + * + * The cap is a display bound, not a completeness claim, and the response says + * which it hit. See {@link GET}. */ const MAX_ROWS = 100; @@ -77,6 +94,11 @@ function readAccessCount( export const GET = apiHandler(async () => { const { user } = await requireAuth(); + // The same number the response publishes and the purge job reads, resolved + // once so the filter and the sentence cannot disagree. + const retentionDays = getAuditLogRetentionDays(); + const windowStart = new Date(Date.now() - retentionDays * 86_400_000); + const rows = await prisma.auditLog.findMany({ where: { userId: user.id, @@ -85,6 +107,10 @@ export const GET = apiHandler(async () => { // report. `actingActorFor` already refuses to stamp one, so this is the // second end of that rule rather than its only enforcement. NOT: { actorUserId: user.id }, + // The window the copy promises. The purge job makes it true eventually; + // this makes it true now, which matters on an instance whose retention + // was shortened after rows were already written. + createdAt: { gte: windowStart }, }, orderBy: { createdAt: "desc" }, take: MAX_ROWS, @@ -114,15 +140,23 @@ export const GET = apiHandler(async () => { createdAt: row.createdAt.toISOString(), })); + // Equality, not `>=`: `take` cannot return more than it was given, and a + // `>` would be a comparison that can never be true. + const truncated = rows.length === MAX_ROWS; + annotate({ action: { name: "sharing.activity.list" }, - meta: { entry_count: entries.length }, + meta: { entry_count: entries.length, truncated }, }); return apiSuccess({ entries, // The bound the view states out loud. Resolved here so the UI never has to // hardcode a number the operator can change. - retentionDays: getAuditLogRetentionDays(), + retentionDays, + // Whether the list is everything inside that window, or its most recent + // hundred rows. The view says which; silence would let the oldest line + // read as the beginning. + truncated, }); }); diff --git a/src/components/settings/access/__tests__/access-panel.test.tsx b/src/components/settings/access/__tests__/access-panel.test.tsx index 42147f829..fe87aa436 100644 --- a/src/components/settings/access/__tests__/access-panel.test.tsx +++ b/src/components/settings/access/__tests__/access-panel.test.tsx @@ -205,14 +205,32 @@ describe("when my record was opened", () => { // see would imply a completeness it does not have: somebody checking last // spring would read an empty list as "no" when the honest answer is // "further back than this reaches". - activityRef.value = { entries: [], retentionDays: 90 }; + activityRef.value = { entries: [], retentionDays: 90, truncated: false }; const html = render(); expect(html).toContain('data-slot="record-activity-retention"'); expect(html).toContain("last 90 days"); }); + it("says when the list is the most recent rows rather than all of them", () => { + // The second completeness claim. The window bounds how far back; the row + // cap bounds how many, and a list that stopped at its ceiling in silence + // let the oldest visible line read as the beginning. + activityRef.value = { entries: [], retentionDays: 365, truncated: true }; + const capped = render(); + expect(capped).toContain('data-slot="record-activity-truncated"'); + expect(capped).toContain("most recent activity"); + + // And it is not an unconditional hedge: a caveat that is always on is one + // people learn to skip, which would cost the sentence its meaning on the + // one screen where it is true. + activityRef.value = { entries: [], retentionDays: 365, truncated: false }; + const whole = render(); + expect(whole).not.toContain('data-slot="record-activity-truncated"'); + expect(whole).toContain('data-slot="record-activity-retention"'); + }); + it("prints the operator's window, never a compiled-in 365", () => { - activityRef.value = { entries: [], retentionDays: 30 }; + activityRef.value = { entries: [], retentionDays: 30, truncated: false }; const html = render(); expect(html).toContain("last 30 days"); expect(html).not.toContain("365"); @@ -229,6 +247,7 @@ describe("when my record was opened", () => { it("counts the openings rather than listing them one by one", () => { activityRef.value = { retentionDays: 365, + truncated: false, entries: [ { id: "a1", @@ -251,6 +270,7 @@ describe("when my record was opened", () => { // it themselves. activityRef.value = { retentionDays: 365, + truncated: false, entries: [ { id: "a2", diff --git a/src/components/settings/access/record-activity-card.tsx b/src/components/settings/access/record-activity-card.tsx index e30a5ef8b..cc8dbede9 100644 --- a/src/components/settings/access/record-activity-card.tsx +++ b/src/components/settings/access/record-activity-card.tsx @@ -62,6 +62,20 @@ export function RecordActivityCard() { {t("recordSharing.activity.retention", { days: data.retentionDays, })} + {/* The second completeness claim. The window says how far back + this can see; the cap says how many rows came back, and a list + that stopped at its ceiling without saying so lets the oldest + line read as the beginning. Rendered only when the ceiling was + actually hit — an unconditional hedge would teach people to + ignore it. */} + {data.truncated && ( + <> + {" "} + + {t("recordSharing.activity.truncated")} + + + )}

)} diff --git a/src/lib/openapi/routes/account-sharing.ts b/src/lib/openapi/routes/account-sharing.ts index 41a4d2fb7..ae3d479e4 100644 --- a/src/lib/openapi/routes/account-sharing.ts +++ b/src/lib/openapi/routes/account-sharing.ts @@ -210,7 +210,12 @@ const recordActivityResponse = z .number() .int() .describe( - "How far back this feed can see, in days. Resolved from the instance's audit retention setting (365 by default, operator-configurable), NOT a constant — an instance running a 90-day window sends 90. Show it beside the rows.", + "How far back this feed can see, in days. Resolved from the instance's audit retention setting (365 by default, operator-configurable), NOT a constant — an instance running a 90-day window sends 90. The query is bounded to this window, so it is the real limit of the list and not only a caption. Show it beside the rows.", + ), + truncated: z + .boolean() + .describe( + "True when the read reached its row ceiling, so `entries` is the most recent activity inside the window rather than all of it. A client must say so — a list that stops at its own cap in silence lets the oldest row read as the beginning. False means the list is everything inside the window.", ), }) .meta({ diff --git a/src/lib/queries/use-account-grants.ts b/src/lib/queries/use-account-grants.ts index 2d7cd6fb9..f2db0e1b9 100644 --- a/src/lib/queries/use-account-grants.ts +++ b/src/lib/queries/use-account-grants.ts @@ -93,6 +93,15 @@ export interface RecordActivity { * the list is everything that ever happened. */ retentionDays: number; + /** + * True when the read hit its row ceiling, so the list is the most recent + * activity inside the window rather than all of it. + * + * Two different completeness claims, and both have to be stated: the window + * bounds how far back, this bounds how many. A list that silently stopped at + * its own cap would let the oldest line read as the beginning. + */ + truncated: boolean; } export function useAccountGrants(enabled = true) { diff --git a/tests/integration/sharing-audit-actor.test.ts b/tests/integration/sharing-audit-actor.test.ts index d3499c2e5..387217e35 100644 --- a/tests/integration/sharing-audit-actor.test.ts +++ b/tests/integration/sharing-audit-actor.test.ts @@ -404,3 +404,94 @@ describe("a refused read is reported as a refused read", () => { expect(sentence).toBe("Alex sent a mood search this record could not read"); }); }); + +/** + * What the owner's activity feed claims, and what it can actually see. + * + * Two completeness claims live on this response and the surface states both. + * They were both wrong in the same direction — too confident — and in opposite + * ways: + * + * * the copy promised a window ("who entered what stays answerable for N + * days") while the query had no date filter at all, so on an instance + * whose retention was shortened the view showed rows the sentence said + * were gone; + * * the query stopped at a hundred rows and said nothing, so an owner who + * scrolled to the bottom read the oldest visible line as the beginning. + * + * Driven against real rows rather than a mocked Prisma, because both claims + * are about what the DATABASE returns for a given `where` — a stubbed client + * would answer whatever the stub was told and prove neither. + */ +describe("the activity feed states its own limits", () => { + async function feed() { + const { GET } = await import("@/app/api/account/activity/route"); + const response = await GET(); + expect(response.status).toBe(200); + return (await response.json()) as { + data: { + entries: { action: string }[]; + retentionDays: number; + truncated: boolean; + }; + }; + } + + /** One delegated row, back-dated. */ + async function stampedRow( + ownerId: string, + delegateId: string, + createdAt: Date, + action = "measurement.update", + ) { + await getPrismaClient().auditLog.create({ + data: { userId: ownerId, actorUserId: delegateId, action, createdAt }, + }); + } + + it("does not show a row older than the window it promises", async () => { + const { owner, delegate } = await household(); + await signIn(owner.id); + + const days = (await feed()).data.retentionDays; + expect(days).toBeGreaterThan(0); + + const inside = new Date(Date.now() - 60_000); + const outside = new Date(Date.now() - (days + 2) * 86_400_000); + await stampedRow(owner.id, delegate.id, inside, "measurement.update"); + await stampedRow(owner.id, delegate.id, outside, "measurement.delete"); + + const { data } = await feed(); + const actions = data.entries.map((e) => e.action); + // The positive control matters more than the absence: a filter that + // excluded everything would satisfy the second assertion alone. + expect(actions).toContain("measurement.update"); + expect(actions).not.toContain("measurement.delete"); + }); + + it("says when the list is the most recent rows rather than all of them", async () => { + const { owner, delegate } = await household(); + await signIn(owner.id); + + // Under the ceiling: the list IS everything, and says so. + for (let i = 0; i < 3; i += 1) { + await stampedRow(owner.id, delegate.id, new Date(Date.now() - i * 1000)); + } + const small = await feed(); + expect(small.data.entries).toHaveLength(3); + expect(small.data.truncated).toBe(false); + + // Over it: same shape, different claim. + await getPrismaClient().auditLog.createMany({ + data: Array.from({ length: 120 }, (_, i) => ({ + userId: owner.id, + actorUserId: delegate.id, + action: "measurement.update", + createdAt: new Date(Date.now() - (i + 10) * 1000), + })), + }); + const large = await feed(); + expect(large.data.entries).toHaveLength(100); + expect(large.data.truncated).toBe(true); + }); +}); From 5de488c35cfcd509e5a26ec935488b7d1b665c54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 7 Aug 2026 00:52:25 +0200 Subject: [PATCH 8/9] docs: state the sharing boundary that is actually enforced MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two sentences promised that a manage grant reaches no settings at all. Both were true when they were written and stopped being true when the anamnesis destination opened, and both are in the worst places for a stale promise: one is the access description generated into the public contract and read by the native client, the other is the paragraph a delegate meets on a page sharing does not cover. Both now name the boundary the code draws — record content versus account configuration — and say which part of the record a manage grant reaches. What stays with the account owner is spelled out rather than implied: login, the second factor, provider connections, tokens, notification routing, module and threshold configuration, and who else has access. A guard checks the copy against the classification rather than against itself, so opening a second destination or closing this one moves the sentence with it. The contract is regenerated. --- docs/api/openapi.yaml | 14 ++- messages/de.json | 2 +- messages/en.json | 2 +- messages/es.json | 2 +- messages/fr.json | 2 +- messages/it.json | 2 +- messages/pl.json | 2 +- src/__tests__/sharing-boundary-copy.test.ts | 119 ++++++++++++++++++++ src/lib/validations/account-sharing.ts | 2 +- 9 files changed, 134 insertions(+), 13 deletions(-) create mode 100644 src/__tests__/sharing-boundary-copy.test.ts diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 89fe122e2..86c2b2da4 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -12538,12 +12538,14 @@ paths: default: READ description: "READ can read the record and change nothing. WRITE can additionally ADD entries (readings, results, observations, a medication, a marked dose) and can still edit or delete nothing, including its own. - MANAGE can additionally change and remove entries, including ones the owner wrote, and read the - generated insights; it never reaches the identity surfaces (login, second factor, connections, - tokens, settings, or who else has access). Omitted means READ, so a client that predates the field - keeps working. The level is fixed when the invitation is written: no endpoint raises a live grant, - because that would widen what the delegate accepted without asking them again. The way up is a new - invitation the delegate accepts." + MANAGE can additionally change and remove entries, including ones the owner wrote, read the + generated insights, and record the record's own health background (allergies, family history). The + boundary is record content versus account configuration: MANAGE never reaches login, the second + factor, provider connections, API tokens, notification routing, module and threshold configuration, + or who else has access. Omitted means READ, so a client that predates the field keeps working. The + level is fixed when the invitation is written: no endpoint raises a live grant, because that would + widen what the delegate accepted without asking them again. The way up is a new invitation the + delegate accepts." type: string enum: - READ diff --git a/messages/de.json b/messages/de.json index 82318dc1f..b928bc2ac 100644 --- a/messages/de.json +++ b/messages/de.json @@ -9522,7 +9522,7 @@ }, "unavailable": { "title": "Gehört nicht zum geteilten Zugriff", - "description": "Geteilt wird die Gesundheitsakte selbst. Einstellungen, verbundene Dienste, Benachrichtigungen und die KI-Funktionen bleiben bei der Person, der das Konto gehört.", + "description": "Geteilt wird die Gesundheitsakte selbst, einschließlich der Vorgeschichte unter Anamnese. Kontoeinstellungen, verbundene Dienste, Benachrichtigungen und die KI-Funktionen bleiben bei der Person, der das Konto gehört.", "leave": "Zurück zu meiner Akte" }, "state": { diff --git a/messages/en.json b/messages/en.json index af1f56262..208915163 100644 --- a/messages/en.json +++ b/messages/en.json @@ -9522,7 +9522,7 @@ }, "unavailable": { "title": "Not part of shared access", - "description": "Sharing covers the health record itself. Settings, connected services, notifications and the AI features stay with the person who owns the account.", + "description": "Sharing covers the health record itself, including the health background under Anamnesis. Account settings, connected services, notifications and the AI features stay with the person who owns the account.", "leave": "Back to my record" }, "state": { diff --git a/messages/es.json b/messages/es.json index 49c232874..8cc2cb450 100644 --- a/messages/es.json +++ b/messages/es.json @@ -9522,7 +9522,7 @@ }, "unavailable": { "title": "No forma parte del acceso compartido", - "description": "Se comparte el historial de salud en sí. Los ajustes, los servicios conectados, las notificaciones y las funciones de IA siguen siendo de la persona propietaria de la cuenta.", + "description": "Se comparte el historial de salud en sí, incluidos los antecedentes que hay en Anamnesis. Los ajustes de la cuenta, los servicios conectados, las notificaciones y las funciones de IA siguen siendo de la persona propietaria de la cuenta.", "leave": "Volver a mi historial" }, "state": { diff --git a/messages/fr.json b/messages/fr.json index 97cb05c19..6668aba8d 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -9522,7 +9522,7 @@ }, "unavailable": { "title": "Ne fait pas partie de l'accès partagé", - "description": "Le partage porte sur le dossier de santé lui-même. Les réglages, les services connectés, les notifications et les fonctions d'IA restent à la personne propriétaire du compte.", + "description": "Le partage porte sur le dossier de santé lui-même, y compris les antécédents dans Anamnèse. Les réglages du compte, les services connectés, les notifications et les fonctions d'IA restent à la personne propriétaire du compte.", "leave": "Revenir à mon dossier" }, "state": { diff --git a/messages/it.json b/messages/it.json index c5c97cfc3..e922a3689 100644 --- a/messages/it.json +++ b/messages/it.json @@ -9522,7 +9522,7 @@ }, "unavailable": { "title": "Non fa parte dell'accesso condiviso", - "description": "La condivisione riguarda la cartella sanitaria in sé. Impostazioni, servizi collegati, notifiche e funzioni di IA restano alla persona proprietaria dell'account.", + "description": "La condivisione riguarda la cartella sanitaria in sé, compresa l'anamnesi. Le impostazioni dell'account, i servizi collegati, le notifiche e le funzioni di IA restano alla persona proprietaria dell'account.", "leave": "Torna alla mia cartella" }, "state": { diff --git a/messages/pl.json b/messages/pl.json index f7e6d23a1..1866ce06b 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -9522,7 +9522,7 @@ }, "unavailable": { "title": "Nie należy do udostępnionego dostępu", - "description": "Udostępniana jest sama karta zdrowia. Ustawienia, połączone usługi, powiadomienia i funkcje SI pozostają przy osobie, do której należy konto.", + "description": "Udostępniana jest sama karta zdrowia, w tym wywiad zdrowotny w sekcji Anamneza. Ustawienia konta, połączone usługi, powiadomienia i funkcje SI pozostają przy osobie, do której należy konto.", "leave": "Wróć do mojej karty" }, "state": { diff --git a/src/__tests__/sharing-boundary-copy.test.ts b/src/__tests__/sharing-boundary-copy.test.ts new file mode 100644 index 000000000..a96aaa9a0 --- /dev/null +++ b/src/__tests__/sharing-boundary-copy.test.ts @@ -0,0 +1,119 @@ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +import { describe, expect, it } from "vitest"; + +import { inviteGrantSchema } from "@/lib/validations/account-sharing"; +import { + SETTINGS_DESTINATION_INVENTORY, + isManageDelegateSettingsDestination, +} from "@/lib/record-settings/classification"; + +/** + * What the product SAYS a manage grant cannot reach agrees with what it cannot + * reach. + * + * Two sentences make that promise, and both were written when it was true: + * + * * the `access` field's description in the sharing request schema, which is + * generated into `docs/api/openapi.yaml` and read by the native client — + * it said MANAGE "never reaches … settings"; + * * `recordSharing.unavailable.description`, the paragraph a delegate meets + * on a page sharing does not cover — it said settings "stay with the + * person who owns the account". + * + * v1.37.0 opened one Settings destination to a manage grant, and both + * sentences became false in the most expensive place a sentence can be false: + * one is a consent screen, the other is a published contract. A promise that + * has quietly stopped holding is worse than no promise, because somebody has + * already decided based on it. + * + * So the copy is checked against the classification rather than against + * itself. If a second destination is ever opened, or the anamnesis one is + * closed again, the sentence that describes the boundary has to move with it. + * + * What this cannot check is whether the wording is GOOD. It checks that the + * absolute claim is gone, that the boundary is named the way the code draws it + * (record content versus account configuration), and that all six locales say + * it — which is the part that rots silently. + */ +const ROOT = process.cwd(); +const LOCALES = ["de", "en", "es", "fr", "it", "pl"] as const; + +/** The published description of the level, off the Zod schema itself. */ +function accessDescription(): string { + const shape = ( + inviteGrantSchema as unknown as { + _def: { shape: Record }; + } + )._def.shape; + return shape.access?.description ?? ""; +} + +function description(locale: string): string { + const bundle = JSON.parse( + readFileSync(join(ROOT, `messages/${locale}.json`), "utf8"), + ) as { + recordSharing: { unavailable: { description: string } }; + }; + return bundle.recordSharing.unavailable.description; +} + +describe("the published boundary matches the one the code draws", () => { + const openDestinations = Object.keys(SETTINGS_DESTINATION_INVENTORY).filter( + isManageDelegateSettingsDestination, + ); + + it("has something to be wrong about", () => { + // The pin every leg below rests on. With no manage-reachable destination + // the old absolute sentence would have been true and this whole file would + // be asserting a style preference. + expect(openDestinations).not.toHaveLength(0); + expect(openDestinations).toContain("anamnesis"); + expect(accessDescription().length).toBeGreaterThan(200); + }); + + it("does not claim a manage grant reaches no settings at all", () => { + const published = accessDescription(); + // The exact clause that went stale, and the shape of any replacement for + // it. `settings` may appear — it has to, to say which parts stay with the + // owner — but not as part of a blanket denial. + expect(published).not.toMatch(/never reaches[^.]*\bsettings\b/i); + expect(published).not.toMatch(/no access to settings/i); + }); + + it("names the boundary the classification actually draws", () => { + // Not a synonym check for its own sake: "record content versus account + // configuration" IS the rule `isManageDelegateSettingsDestination` + // implements, and a description that described some other boundary would + // be a second, disagreeing specification. + expect(accessDescription()).toMatch( + /record content versus account configuration/i, + ); + // And the destination that was opened is named, so a reader of the + // contract can tell which part of the record it means. + expect(accessDescription()).toMatch(/allergies, family history/i); + }); + + it("keeps the refusal paragraph honest in all six locales", () => { + // The delegate-facing half. It is shown on a page sharing does NOT cover, + // so a sentence that over-claims there is read at the exact moment + // somebody is working out where the edge is. + const missing: string[] = []; + for (const locale of LOCALES) { + const text = description(locale); + expect(text.length, locale).toBeGreaterThan(40); + // Every locale has to acknowledge the health background, because that + // is the part the old sentence implicitly denied. Matched on the stem + // rather than the English spelling, and short enough to survive the + // accent: the section is "Anamnèse" in French and "Anamneza" in Polish, + // so anything past "anamn" is already language-specific and a matcher + // tuned to one of them reports the others as broken. + if (!/anamn/i.test(text)) missing.push(locale); + // And it must still say the account itself stays with its owner — + // dropping that would fix one over-claim by removing the boundary. + expect(text, locale).toMatch(/konto|account|compte|cuenta|konta|koncie/i); + } + expect(missing).toEqual([]); + }); +}); diff --git a/src/lib/validations/account-sharing.ts b/src/lib/validations/account-sharing.ts index a6c6315c7..5e47ad567 100644 --- a/src/lib/validations/account-sharing.ts +++ b/src/lib/validations/account-sharing.ts @@ -58,7 +58,7 @@ export const inviteGrantSchema = z .enum(["READ", "WRITE", "MANAGE"]) .default("READ") .describe( - "READ can read the record and change nothing. WRITE can additionally ADD entries (readings, results, observations, a medication, a marked dose) and can still edit or delete nothing, including its own. MANAGE can additionally change and remove entries, including ones the owner wrote, and read the generated insights; it never reaches the identity surfaces (login, second factor, connections, tokens, settings, or who else has access). Omitted means READ, so a client that predates the field keeps working. The level is fixed when the invitation is written: no endpoint raises a live grant, because that would widen what the delegate accepted without asking them again. The way up is a new invitation the delegate accepts.", + "READ can read the record and change nothing. WRITE can additionally ADD entries (readings, results, observations, a medication, a marked dose) and can still edit or delete nothing, including its own. MANAGE can additionally change and remove entries, including ones the owner wrote, read the generated insights, and record the record's own health background (allergies, family history). The boundary is record content versus account configuration: MANAGE never reaches login, the second factor, provider connections, API tokens, notification routing, module and threshold configuration, or who else has access. Omitted means READ, so a client that predates the field keeps working. The level is fixed when the invitation is written: no endpoint raises a live grant, because that would widen what the delegate accepted without asking them again. The way up is a new invitation the delegate accepts.", ), /** * v1.37.0 — which sections of the record the invitation opens. From 6ae0cc993ffa9fdca56bc5aafa5029cd8755e03b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 7 Aug 2026 01:00:02 +0200 Subject: [PATCH 9/9] fix: close four small gaps around the record surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../managed-record-navigation.test.tsx | 15 ++++ .../record-settings-contract.test.ts | 18 ++++ src/app/api/settings/privacy-summary/route.ts | 20 +++-- .../record-settings-section-gate.test.tsx | 86 ++++++++++++++++++- .../settings/record-settings-section-gate.tsx | 33 ++++++- src/components/settings/settings-shell.tsx | 9 +- .../interpolation-is-literal.test.ts | 83 ++++++++++++++++++ src/lib/i18n/context.tsx | 11 ++- src/lib/i18n/server-translator.ts | 11 ++- 9 files changed, 272 insertions(+), 14 deletions(-) create mode 100644 src/lib/i18n/__tests__/interpolation-is-literal.test.ts diff --git a/src/__tests__/managed-record-navigation.test.tsx b/src/__tests__/managed-record-navigation.test.tsx index 1f0c56634..5f8fd08d7 100644 --- a/src/__tests__/managed-record-navigation.test.tsx +++ b/src/__tests__/managed-record-navigation.test.tsx @@ -151,6 +151,21 @@ describe("managed record navigation", () => { } }); + it("offers a managed entry below MANAGE nothing at all", () => { + // A guardian grant is always MANAGE today, which is exactly why the level + // check was easy to leave out of the managed branch. Without it a managed + // entry that ever arrived below MANAGE would list destinations the section + // gate refuses, sending somebody to a page that explains it cannot open. + recordRef.value = { kind: "managed", level: "read" }; + const html = renderSettings(); + + for (const section of SETTINGS_SECTIONS) { + expect(html, section.slug).not.toContain( + `href="/settings/${section.slug}"`, + ); + } + }); + it("keeps the exit control present while administering a managed record", () => { recordRef.value = { kind: "managed", level: "manage" }; const html = renderToStaticMarkup( diff --git a/src/__tests__/record-settings-contract.test.ts b/src/__tests__/record-settings-contract.test.ts index 50d3781ab..0e1c2b3d0 100644 --- a/src/__tests__/record-settings-contract.test.ts +++ b/src/__tests__/record-settings-contract.test.ts @@ -10,6 +10,7 @@ import { isManageDelegateSettingsDestination, SETTINGS_DESTINATION_INVENTORY, } from "@/lib/record-settings/classification"; +import { RECORD_CONTENT_SECTIONS } from "@/components/settings/record-settings-section-gate"; import { toRecordSettingsDto } from "@/lib/record-settings/dto"; import { resolveManagedIntegrationState } from "@/lib/record-settings/integrations"; import { assertRecordSettingsResponseForRecord } from "@/lib/record-settings/response"; @@ -114,6 +115,23 @@ describe("record settings contract", () => { } }); + it("gives every record-content destination a page of its own", () => { + // Both directions, and the first is the one that matters. `manage-writable` + // is a classification; the gate has to know which PAGE each slug is, or a + // destination added to the kind would silently inherit the allergy and + // family-history managers under its own heading. The map is what says + // which, and this is what stops the two drifting apart — a new + // manage-writable slug fails here, at the moment it is added, rather than + // rendering somebody else's forms. + const classified = Object.keys(SETTINGS_DESTINATION_INVENTORY) + .filter(isManageDelegateSettingsDestination) + .sort(); + const rendered = Object.keys(RECORD_CONTENT_SECTIONS).sort(); + + expect(classified).not.toHaveLength(0); + expect(rendered).toEqual(classified); + }); + it("leaves exactly one destination on the manage-writable list", () => { // The list is short on purpose and a second entry is a consent decision, // not a classification tidy-up. Pinning the count means a destination diff --git a/src/app/api/settings/privacy-summary/route.ts b/src/app/api/settings/privacy-summary/route.ts index 8bc58c600..f7e8fae74 100644 --- a/src/app/api/settings/privacy-summary/route.ts +++ b/src/app/api/settings/privacy-summary/route.ts @@ -8,17 +8,25 @@ */ import { apiHandler, requireAuth } from "@/lib/api-handler"; import { apiSuccess } from "@/lib/api-response"; +import { getAuditLogRetentionDays } from "@/lib/jobs/audit-log-cleanup"; import { annotate } from "@/lib/logging/context"; import { ENCRYPTED_COLUMNS } from "@/lib/crypto/encrypted-columns"; export const dynamic = "force-dynamic"; /** - * Retention windows. These mirror the defaults + env names the cleanup jobs - * read (`coach-message-cleanup.ts`, `audit-log-cleanup.ts`, the reminder - * cleanup handlers); read here directly so the dashboard never imports a - * pg-boss job module. Off-host backup retention is operator infrastructure - * (not an app setting), so it is disclosed as prose, not a number. + * Retention windows, as the privacy dashboard states them. + * + * The audit window comes from `getAuditLogRetentionDays()` rather than from a + * second parse of the same variable. It used to be re-implemented here with + * its own default and its own floor, which is two implementations of one + * disclosure — and this surface, the sharing panel and the purge job all quote + * the number to the same person. A drift between them is a page telling + * somebody their data is kept for a length of time it is not. + * + * The rest still parse locally so the dashboard never imports a pg-boss job + * module. Off-host backup retention is operator infrastructure (not an app + * setting), so it is disclosed as prose, not a number. */ function resolveRetention() { const intEnv = (name: string, fallback: number, min: number): number => { @@ -28,7 +36,7 @@ function resolveRetention() { }; return { coachMessagesDays: intEnv("COACH_MESSAGE_RETENTION_DAYS", 365, 30), - auditLogDays: intEnv("AUDIT_LOG_RETENTION_DAYS", 365, 7), + auditLogDays: getAuditLogRetentionDays(), // The push-attempt + mood-dispatch ledgers are a fixed 90-day window. deliveryLogDays: 90, }; diff --git a/src/components/settings/__tests__/record-settings-section-gate.test.tsx b/src/components/settings/__tests__/record-settings-section-gate.test.tsx index 63045a08e..9bac09382 100644 --- a/src/components/settings/__tests__/record-settings-section-gate.test.tsx +++ b/src/components/settings/__tests__/record-settings-section-gate.test.tsx @@ -8,7 +8,7 @@ const authRef: { active: { accountId: string; recordKind: "managed" | "shared"; - level: "manage"; + level: "manage" | "write" | "read"; sections: null; canWrite: true; } | null; @@ -38,10 +38,13 @@ vi.mock("../managed-record-settings-section", () => ({ vi.mock("../managed-integration-status", () => ({ ManagedIntegrationStatus: () =>
, })); +vi.mock("../record-anamnesis-section", () => ({ + RecordAnamnesisSection: () =>
, +})); import { RecordSettingsSectionGate } from "../record-settings-section-gate"; -function render(section: "modules" | "integrations") { +function render(section: "modules" | "integrations" | "anamnesis" | "labs") { return renderToStaticMarkup( @@ -77,4 +80,83 @@ describe("RecordSettingsSectionGate", () => { expect(html).toContain("managed-settings"); expect(html).not.toContain("actor-settings"); }); + + /** + * The record-content destination, and the two ways it must not open. + * + * The branch keys on the SLUG as well as on the classification. Keying on + * the kind alone would render the allergy and family-history managers under + * the heading of any future `manage-writable` destination — silently, + * because a category matched where a page was meant. + */ + describe("the anamnesis destination", () => { + it("opens for an adult manager on an ordinary shared record", () => { + authRef.isLoading = false; + authRef.active = { + accountId: "shared-record", + recordKind: "shared", + level: "manage", + sections: null, + canWrite: true, + }; + + const html = render("anamnesis"); + + expect(html).toContain("record-anamnesis"); + expect(html).not.toContain("actor-settings"); + expect(html).not.toContain("managed-settings"); + }); + + it("opens for a Guardian, rather than the guardian configuration panel", () => { + authRef.isLoading = false; + authRef.active = { + accountId: "managed-record", + recordKind: "managed", + level: "manage", + sections: null, + canWrite: true, + }; + + const html = render("anamnesis"); + + expect(html).toContain("record-anamnesis"); + expect(html).not.toContain("managed-settings"); + }); + + it("stays shut below MANAGE", () => { + authRef.isLoading = false; + authRef.active = { + accountId: "shared-record", + recordKind: "shared", + level: "write", + sections: null, + canWrite: true, + }; + + const html = render("anamnesis"); + + expect(html).not.toContain("record-anamnesis"); + expect(html).toContain("shared-record-settings-unavailable-title"); + }); + + it("does not lend its page to another destination of the same kind", () => { + // `labs` is `adult-shared-unavailable` today, so this passes for the + // right reason now; the claim it pins is that the branch asks WHICH + // page, not which category. A second `manage-writable` slug added + // without an arm falls through to the refusal panel. + authRef.isLoading = false; + authRef.active = { + accountId: "shared-record", + recordKind: "shared", + level: "manage", + sections: null, + canWrite: true, + }; + + const html = render("labs"); + + expect(html).not.toContain("record-anamnesis"); + expect(html).toContain("shared-record-settings-unavailable-title"); + }); + }); }); diff --git a/src/components/settings/record-settings-section-gate.tsx b/src/components/settings/record-settings-section-gate.tsx index 449c55bf6..aaf697477 100644 --- a/src/components/settings/record-settings-section-gate.tsx +++ b/src/components/settings/record-settings-section-gate.tsx @@ -14,6 +14,20 @@ import { RecordAnamnesisSection } from "@/components/settings/record-anamnesis-s import { SettingsCard } from "@/components/settings/settings-card"; import type { SettingsSectionSlug } from "@/components/settings/section-slugs"; +/** + * The record-CONTENT destinations, and what each one renders inside a record. + * + * Keyed by slug so the branch below asks which page it is on rather than which + * category the page belongs to. Every `manage-writable` slug must have an + * entry and nothing else may: the contract test holds both directions, which + * is what stops a new destination inheriting this one's forms. + */ +export const RECORD_CONTENT_SECTIONS: Partial< + Record ReactNode> +> = { + anamnesis: RecordAnamnesisSection, +}; + const MANAGED_RECORD_SETTINGS_FAMILY_BY_SECTION = { account: "profile", modules: "modules", @@ -65,9 +79,22 @@ export function RecordSettingsSectionGate({ // Checked before the managed branches so a Guardian gets the same record // surface an adult manager does, rather than the guardian configuration // panel, which has no family for it. - if (destination.kind === "manage-writable" && level === "manage") { - return ; - } + // + // Resolved through a slug map rather than by matching the kind, and that is + // not belt-and-braces. `manage-writable` is a classification, not a + // component: a second destination joining it — a future record-content page + // — would otherwise render the allergy and family-history managers under its + // own heading, silently, because the branch matched a category where a page + // was meant. `RECORD_CONTENT_SECTIONS` says which page each slug is, a slug + // with no entry falls through to the refusal panel below, and + // `record-settings-contract.test.ts` fails if the map and the classification + // ever disagree — so a destination cannot join the kind without somebody + // deciding what it renders. + const RecordContent = + level === "manage" && destination.kind === "manage-writable" + ? RECORD_CONTENT_SECTIONS[section] + : undefined; + if (RecordContent) return ; if ( recordKind === "managed" && diff --git a/src/components/settings/settings-shell.tsx b/src/components/settings/settings-shell.tsx index 7d1a03655..15d3f9335 100644 --- a/src/components/settings/settings-shell.tsx +++ b/src/components/settings/settings-shell.tsx @@ -460,10 +460,17 @@ export function SettingsShell({ ? allVisibleSections : allVisibleSections.filter((section) => { const kind = classifySettingsDestination(section.slug).kind; + // Every destination on either list needs MANAGE, so the level is + // checked once for both record kinds rather than only for the adult + // one. A guardian grant is always MANAGE today, which is exactly why + // the check was easy to leave out — and a managed entry that ever + // arrived below it would have listed destinations the section gate + // refuses, sending somebody to a page that explains it cannot open. + if (activeRecord.level !== "manage") return false; if (activeRecord.recordKind === "managed") { return kind === "managed-guardian" || kind === "manage-writable"; } - return kind === "manage-writable" && activeRecord.level === "manage"; + return kind === "manage-writable"; }); const activeSection = allVisibleSections.find( (section) => section.slug === activeSlug, diff --git a/src/lib/i18n/__tests__/interpolation-is-literal.test.ts b/src/lib/i18n/__tests__/interpolation-is-literal.test.ts new file mode 100644 index 000000000..16459558a --- /dev/null +++ b/src/lib/i18n/__tests__/interpolation-is-literal.test.ts @@ -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}"); + }); +}); diff --git a/src/lib/i18n/context.tsx b/src/lib/i18n/context.tsx index b46e57dd6..45149980c 100644 --- a/src/lib/i18n/context.tsx +++ b/src/lib/i18n/context.tsx @@ -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)); } } diff --git a/src/lib/i18n/server-translator.ts b/src/lib/i18n/server-translator.ts index 950e4d17f..096d9f8da 100644 --- a/src/lib/i18n/server-translator.ts +++ b/src/lib/i18n/server-translator.ts @@ -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;