From ebc668d28d3e655511660097a809fe16905df1eb Mon Sep 17 00:00:00 2001 From: flemming-it Date: Mon, 3 Aug 2026 23:50:34 +0200 Subject: [PATCH] feat(approvals,audit): record the reviewer as the unchecked claim it is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hub copies the reviewer string a client sends straight into decided_by (DecideApproval, ClearEventLog); nothing on the wire ties it to the authenticated caller. Studio filled it from the OS account, so an export read like non-repudiation while being an arbitrary client claim — the legal finding of the 2026-07-26 usertest panel. The real fix is hub-side (derive decided_by from CALLER_IDENTITY); that contract is written down in docs/reviewer-identity.md and needs a hub release. Until then Studio does the one thing it can do honestly and marks its own claim as a claim, inside the record: - data/reviewer_identity.dart is the single place that produces and reads the value; wire() is idempotent, so page and HubService may both normalise. Every write path funnels through HubService, so no surface can send a bare handle. - The inbox states before the decision who will be recorded, what that attribution is worth on this hub (from AuthStatus), and the literal string that lands in decided_by. An unreadable auth policy stays unreadable — never optimistic. - Reading back: a marked value shows its plain name plus an unchecked flag; an unmarked one (legacy row, CLI decision, or a future hub-derived identity) is not classified either way. - The audit wipe seeds the same kind of marked attribution into its chain.reset marker. When the hub starts deriving the value it overwrites the field and the prefix disappears by itself — no Studio release needed. Guards: reviewer_identity_test (the value) and approvals_reviewer_identity_test (every surface that writes or renders it, against the hermetic fake hub). Visual proof for both themes via the dialog-shot harness. Signed-off-by: flemming-it --- CHANGELOG.md | 18 ++ docs/reviewer-identity.md | 89 +++++++ integration_test/dialog_shots_test.dart | 64 ++++- lib/data/hub.dart | 28 +- lib/data/reviewer_identity.dart | 139 ++++++++++ lib/l10n/app_de.arb | 8 + lib/l10n/app_en.arb | 8 + lib/l10n/app_localizations.dart | 48 ++++ lib/l10n/app_localizations_de.dart | 31 +++ lib/l10n/app_localizations_en.dart | 31 +++ lib/pages/approvals.dart | 240 +++++++++++++++-- lib/pages/audit.dart | 24 +- test/approvals_reviewer_identity_test.dart | 288 +++++++++++++++++++++ test/reviewer_identity_test.dart | 105 ++++++++ test/support/fake_hub.dart | 49 ++++ 15 files changed, 1129 insertions(+), 41 deletions(-) create mode 100644 docs/reviewer-identity.md create mode 100644 lib/data/reviewer_identity.dart create mode 100644 test/approvals_reviewer_identity_test.dart create mode 100644 test/reviewer_identity_test.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index e3e1eb3..8e414ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,24 @@ Approvals usertest-panel hardening: (not "PROMPT"), and the no-data hint drops developer jargon. - **Copy the run id in one click**, and the history payload renders with the same JSON pretty-printing as the card. +- **A reviewer name no longer poses as a proven identity.** The hub + stores whatever `reviewer` string a client sends as `decided_by`, + so Studio's `$USER@studio` read like non-repudiation while being + an unchecked claim (legal finding of the 2026-07-26 panel). Every + value Studio writes — approvals and the `chain.reset` marker of + the audit wipe — now carries an `unverified:` marker inside the + record, so an export, a CLI reader or a database dump sees the + trust level without knowing how the deciding Studio was set up. + The inbox states before the decision who will be recorded and + what that is worth on this hub (derived from the hub's auth + policy; "unreadable" stays unreadable, never optimistic). Reading + back, a marked value shows its plain name with an "unchecked" + flag, an unmarked one (legacy row, CLI decision) is not + classified either way. The real fix is hub-side — derive + `decided_by` from the authenticated caller; the contract is in + `docs/reviewer-identity.md`, and once the hub does it the marker + disappears without a Studio release. Guards: + `reviewer_identity_test` + `approvals_reviewer_identity_test`. - **Audit overflow-menu entries no longer run off the menu.** A popup menu is width-capped, so the full-sentence German labels were clipped — on the reset entry exactly the "(nur local/dev)" diff --git a/docs/reviewer-identity.md b/docs/reviewer-identity.md new file mode 100644 index 0000000..7ccd509 --- /dev/null +++ b/docs/reviewer-identity.md @@ -0,0 +1,89 @@ +# Reviewer identity — what `decided_by` is worth + +## The problem + +The hub copies the `reviewer` string a client sends straight into +`decided_by`: + +- `DecideApproval` — who approved or rejected a paused flow step +- `ClearEventLog` — who wiped the audit log, recorded in the seeded + `chain.reset` marker + +Nothing on the wire ties that string to the authenticated caller. +Studio fills it from the OS account (`$USER@studio`); any other +client can send any string at all, including someone else's name. + +The legal review of the approvals page (usertest panel, 2026-07-26) +graded this HIGH: a value that reads like an identity but is an +unchecked client claim lends the audit trail a non-repudiation it +does not have. An export, a court-facing report or an auditor +reading the database sees `stefan@studio` and has no way to tell +whether the hub checked anything. + +This is not a leak and not remote-exploitable on its own — the +finding is about what the record *proves*, not about access. + +## What Studio does today (0.81.0) + +Studio marks its own claim as a claim, **inside the recorded +value**: + +``` +decided_by = "unverified:stefan@studio" +``` + +- `lib/data/reviewer_identity.dart` is the single place that + produces and reads that value. `ReviewerIdentity.wire()` is + idempotent, so both the page and `HubService` may normalise. +- Every write path funnels through `HubService.approve` / + `reject` / `clearEventLog`, so no surface can send a bare handle. +- The approvals inbox states before the decision who will be + recorded, what the attribution is worth on *this* hub (derived + from `AuthStatus.anonymous_allowed`), and the literal string that + will be stored. +- Reading back: a marked value is shown by its name with an + "unchecked" flag; an **unmarked** value (legacy row, a CLI + decision, or a future hub-derived identity) gets no badge at all + — Studio does not know its provenance and must not classify it. + +Guards: `test/reviewer_identity_test.dart` (the value itself) and +`test/approvals_reviewer_identity_test.dart` (every surface that +writes or renders it, against the hermetic fake hub). + +This is honest, but it is a *label*, not a fix. It does not stop a +client from sending `unverified:someone.else@studio`. + +## The actual fix — hub side + +`decided_by` must be derived server-side from the verified caller +and the client-supplied `reviewer` ignored: + +1. In the `DecideApproval` and `ClearEventLog` handlers, take the + identity from the same source `_caller` comes from + (`CALLER_IDENTITY`) rather than from the request message. + - static validator: the configured token's name + - `jwt-rs256` validator: the JWT subject + - anonymous call: no identity — record `anonymous` explicitly + (never a client-supplied name), or refuse the decision on + channels where the operator requires attribution. +2. Keep accepting the request field for one release (ignored) so + older Studios and CLIs keep working; log a deprecation when it + is present and differs from the derived value. +3. Once the hub derives the value, it overwrites the field and the + `unverified:` prefix disappears from new records by itself — no + Studio release is needed to stop labelling. Studio's reader + already treats an unmarked value as "provenance unknown", so a + hub-derived value renders cleanly. +4. Old rows keep the prefix. That is correct: they *were* unchecked. + +Open product decision for step 1: whether an anonymous hub may +decide approvals at all, or whether approvals require an +authenticated caller. Studio surfaces the anonymous case today; it +does not block it. + +## Related + +- `lib/data/reviewer_identity.dart` — the Studio-side contract +- Backlog T014, point 1 (usertest 2026-07-26, legal persona) +- Points 2–7 of that finding (hash-chain-backed history, deeplink, + sealed slugs, UTC offsets, auto-refresh) are untouched by this. diff --git a/integration_test/dialog_shots_test.dart b/integration_test/dialog_shots_test.dart index 3fa17a0..870ad16 100644 --- a/integration_test/dialog_shots_test.dart +++ b/integration_test/dialog_shots_test.dart @@ -19,8 +19,11 @@ import 'package:shared_preferences/shared_preferences.dart'; import 'package:chain_studio_flow_editor/src/l10n.dart'; import 'package:chain_studio_flow_editor/src/widgets/missing_modules_badge.dart'; -import 'package:chain_studio/data/hub.dart' show StoreItem; +import 'package:chain_studio/data/hub.dart' + show ApprovalRecord, StoreItem, ThemeModeValue; +import 'package:chain_studio/data/reviewer_identity.dart'; import 'package:chain_studio/l10n/app_localizations.dart'; +import 'package:chain_studio/main.dart'; import 'package:chain_studio/pages/federation.dart'; import 'package:chain_studio/widgets/chain_install_confirm.dart'; @@ -200,4 +203,63 @@ void main() { await _shot(tester, '$name-dark'); }); } + + // Reviewer identity on the approvals inbox — the strip that names + // who the audit trail will record and says, in plain words, what + // that attribution is worth on this hub. Captured in both themes + // because the "not verified" pill and the literal recorded value + // have to stay legible in each (release gate, project CLAUDE.md). + for (final (themeName, mode) in [ + ('light', ThemeModeValue.light), + ('dark', ThemeModeValue.dark), + ]) { + testWidgets('approvals reviewer identity — $themeName', (tester) async { + SharedPreferences.setMockInitialValues({}); + ReviewerIdentity.debugHandle = 'stefan@studio'; + addTearDown(() => ReviewerIdentity.debugHandle = null); + final fake = installFakeHub(); + fake.approvals = [ + ApprovalRecord( + id: 'apr-1', + flowName: 'rechnung-klassifizieren', + stepId: 'pruefen', + prompt: 'Bitte die Klassifikation dieser Rechnung bestätigen.', + payloadPreview: '{"label":"Rechnung","betrag":"128,40 EUR"}', + createdAt: DateTime.utc(2026, 7, 26, 12, 0, 0), + expiresAt: null, + status: 'pending', + decidedAt: null, + decidedBy: '', + reason: '', + project: 'buergeramt', + flowExecution: 'run-abc123', + ), + ]; + tester.view.physicalSize = const Size(1280, 900); + tester.view.devicePixelRatio = 1.0; + addTearDown(tester.view.reset); + // The real app shell, not a bare MaterialApp: the strip has to + // be judged inside the Chain theme it ships in. + await tester.pumpWidget( + RepaintBoundary( + key: _shotKey, + child: StudioApp(initialThemeMode: mode, initialLocale: Locale('de')), + ), + ); + await tester.pump(const Duration(milliseconds: 100)); + await tester.tap(find.byKey(const ValueKey('sidebar-item-approvals'))); + for (var i = 0; i < 12; i++) { + await tester.pump(const Duration(milliseconds: 200)); + } + await _shot(tester, 'approvals-reviewer-identity-$themeName'); + expect(find.text('Sie entscheiden als'), findsOneWidget); + expect( + find.text('Wird gespeichert als: unverified:stefan@studio'), + findsOneWidget, + ); + // Long-lived timers: unmount before the harness tears down. + await tester.pumpWidget(const SizedBox.shrink()); + await tester.pump(const Duration(minutes: 1)); + }); + } } diff --git a/lib/data/hub.dart b/lib/data/hub.dart index 9f94b31..93f253f 100644 --- a/lib/data/hub.dart +++ b/lib/data/hub.dart @@ -16,6 +16,7 @@ import 'package:shared_preferences/shared_preferences.dart'; import '../l10n/app_localizations.dart'; import 'flow_output.dart'; import 'hub_auth_token.dart'; +import 'reviewer_identity.dart'; export 'flow_output.dart'; class HubService { @@ -649,12 +650,18 @@ class HubService { /// `production`; the gRPC error surfaces as an exception so /// the caller can show the operator why it was blocked. /// Returns `(purged, channel)` so the UI can confirm what - /// just happened. + /// just happened. [reviewer] is the operator's typed handle and + /// goes out through [ReviewerIdentity.wire] for the same reason + /// approvals do: the marker the hub seeds is read by auditors and + /// must not claim a verified identity Studio cannot supply. Future<({int purged, String channel})> clearEventLog({ required String reviewer, required String reason, }) async { - final r = await _client.clearEventLog(reviewer: reviewer, reason: reason); + final r = await _client.clearEventLog( + reviewer: ReviewerIdentity.wire(reviewer), + reason: reason, + ); return (purged: r.purged.toInt(), channel: r.channel); } @@ -1175,11 +1182,22 @@ class HubService { .toList(); } + /// Decide an approval. [reviewer] is the operator-facing handle; + /// it leaves Studio through [ReviewerIdentity.wire], which labels + /// it as the unchecked client claim it is — the hub stores the + /// string verbatim, so the record has to carry its own trust + /// level (see `lib/data/reviewer_identity.dart`). Every decide + /// path (approvals page, batch actions, the flow-editor run + /// driver) funnels through here so none of them can bypass that. Future approve(String id, String reviewer) => - _client.approve(approvalId: id, reviewer: reviewer); + _client.approve(approvalId: id, reviewer: ReviewerIdentity.wire(reviewer)); - Future reject(String id, String reviewer, String reason) => - _client.reject(approvalId: id, reviewer: reviewer, reason: reason); + Future reject(String id, String reviewer, String reason) => _client + .reject( + approvalId: id, + reviewer: ReviewerIdentity.wire(reviewer), + reason: reason, + ); /// Detached-runs monitor snapshot: every tracked invocation /// (newest-first, optionally scoped to one [project]) plus whether diff --git a/lib/data/reviewer_identity.dart b/lib/data/reviewer_identity.dart new file mode 100644 index 0000000..a50f2f8 --- /dev/null +++ b/lib/data/reviewer_identity.dart @@ -0,0 +1,139 @@ +// Who decided? — the single place Studio answers that question. +// +// The hub copies the `reviewer` string a client sends straight into +// `decided_by` (`DecideApproval` / `ClearEventLog`); nothing on the +// wire ties that string to the authenticated caller. The legal +// review of the approvals page (usertest 2026-07-26) called it out: +// a value that reads like an identity but is an unchecked client +// string lends the audit trail a non-repudiation it does not have. +// +// The real fix is hub-side — derive `decided_by` from the verified +// caller (`CALLER_IDENTITY`, the same source `_caller` comes from) +// and ignore what the client claims. That contract is written down +// in `docs/reviewer-identity.md`; it needs a hub release. +// +// Until then Studio does the one thing it *can* do honestly: it +// marks its own claim as a claim, inside the record. Every reviewer +// string Studio sends carries the `unverified:` prefix, so an +// export, an SQL reader or another client sees the trust level +// without having to know how the deciding Studio was configured. +// When the hub starts deriving the value it overwrites the field +// and the prefix disappears by itself — no Studio release needed +// to stop lying. + +import 'dart:io'; + +import 'package:meta/meta.dart'; + +/// Marks a reviewer attribution as a client-side claim. Part of the +/// recorded value, not just a UI decoration — see the file header. +const String kUnverifiedReviewerPrefix = 'unverified:'; + +/// How much a reviewer attribution Studio is about to write is +/// worth, derived from the hub's authentication policy. +enum ReviewerAssurance { + /// The hub accepts calls without credentials: a decision cannot + /// be tied to anyone at all. + anonymousHub, + + /// The hub authenticates the connection (static token or JWT), + /// but still records the reviewer name the client supplies. + accessControlled, + + /// Studio could not read the hub's auth policy — `AuthStatus` is + /// admin-scoped, so a plain reviewer token gets PermissionDenied, + /// and hubs older than the RPC answer UNIMPLEMENTED. + unknown, +} + +/// Map the hub's `anonymous_allowed` flag onto an assurance level. +/// `null` = policy unreadable (denied / unsupported / offline). +ReviewerAssurance reviewerAssuranceFor(bool? anonymousAllowed) => + switch (anonymousAllowed) { + true => ReviewerAssurance.anonymousHub, + false => ReviewerAssurance.accessControlled, + null => ReviewerAssurance.unknown, + }; + +/// What a stored `decided_by` value is worth when read back. +enum RecordedReviewerTrust { + /// Written by a Studio that labelled its own claim (`unverified:`). + selfDeclared, + + /// No trust marker: a legacy row, a CLI decision, or a future + /// hub-derived identity. Studio does not know which — and must + /// not present it as proven either way. + unknown, +} + +/// A `decided_by` value split into what it says and what it is worth. +@immutable +class RecordedReviewer { + /// The name without the trust marker — what a human should read. + final String handle; + final RecordedReviewerTrust trust; + + const RecordedReviewer({required this.handle, required this.trust}); + + bool get isSelfDeclared => trust == RecordedReviewerTrust.selfDeclared; +} + +/// Resolves the reviewer identity Studio submits and reads recorded +/// ones back. Never contacts the hub — the assurance level comes +/// from the caller (see [reviewerAssuranceFor]). +class ReviewerIdentity { + ReviewerIdentity._(); + + static String? _debugHandle; + + /// Test seam: pins [localHandle] so suites do not depend on the + /// OS account of whoever runs them. `null` restores the default. + @visibleForTesting + static set debugHandle(String? value) => _debugHandle = value; + + /// Human-readable handle of the operator sitting in front of + /// Studio. The OS account is the closest stable label available + /// locally — a label, never a proof, which is exactly why it + /// leaves the machine through [wire]. + static String get localHandle { + final pinned = _debugHandle; + if (pinned != null) return pinned; + final user = + Platform.environment['USER'] ?? + Platform.environment['USERNAME'] ?? + 'studio'; + return '$user@studio'; + } + + /// The value that goes on the wire for [handle] (default: + /// [localHandle]). Idempotent, so a value that already carries the + /// marker — or one round-tripped through the UI — is not prefixed + /// twice. An empty/blank handle falls back to [localHandle]: the + /// hub rejects an empty reviewer, and a blank one would be a worse + /// record than a labelled guess. + static String wire([String? handle]) { + final trimmed = (handle ?? localHandle).trim(); + final name = trimmed.isEmpty ? localHandle : trimmed; + if (name.startsWith(kUnverifiedReviewerPrefix)) return name; + return '$kUnverifiedReviewerPrefix$name'; + } + + /// Split a recorded `decided_by` into name + trust. + static RecordedReviewer parse(String recorded) { + final value = recorded.trim(); + if (!value.startsWith(kUnverifiedReviewerPrefix)) { + return RecordedReviewer( + handle: value, + trust: RecordedReviewerTrust.unknown, + ); + } + final handle = value.substring(kUnverifiedReviewerPrefix.length).trim(); + return RecordedReviewer( + // A marker with nothing behind it still says something true + // ("someone claimed nothing"); show the marker rather than an + // empty cell. + handle: handle.isEmpty ? value : handle, + trust: RecordedReviewerTrust.selfDeclared, + ); + } +} diff --git a/lib/l10n/app_de.arb b/lib/l10n/app_de.arb index 13bc40f..e40fab3 100644 --- a/lib/l10n/app_de.arb +++ b/lib/l10n/app_de.arb @@ -674,6 +674,7 @@ "auditClearDialogTitle": "Audit-Log löschen?", "auditClearDialogBody": "Löscht jedes Audit-Event auf dem aktiven Kanal und seedet einen neuen chain.reset-Marker mit Prüfer und Begründung. Auf beta / production verweigert. Nicht umkehrbar.", "auditClearReviewerLabel": "Prüfer", + "auditClearReviewerHelper": "Wird als ungeprüfte Angabe festgehalten (Präfix „unverified:“) — der Hub übernimmt den Namen unverändert.", "auditClearReasonLabel": "Begründung (im chain.reset-Marker festgehalten)", "auditClearReasonHelper": "Erforderlich — Auditoren werden das lesen.", "auditClearLogButton": "Log löschen", @@ -1102,6 +1103,13 @@ "approvalsRequestFallback": "Freigabe für diesen Schritt erforderlich", "approvalsFlowStepMeta": "Flow: {flow} · Schritt: {step}", "approvalsIntroHelp": "Hier warten pausierte Vorgänge (Flows) auf Ihre Entscheidung. Jede Karte zeigt, welcher Vorgang an welchem Schritt hält und welche Daten er Ihnen vorlegt — Freigeben setzt ihn fort, Ablehnen stoppt ihn mit Ihrer Begründung.", + "approvalsReviewerLabel": "Sie entscheiden als", + "approvalsReviewerUnverifiedPill": "nicht überprüft", + "approvalsReviewerRecordedAs": "Wird gespeichert als: {value}", + "approvalsReviewerNoteAnonymous": "Dieser Hub nimmt Aufrufe ohne Zugangsdaten an — eine Entscheidung lässt sich damit niemandem nachweisen. Studio kennzeichnet den Namen deshalb im Eintrag selbst als ungeprüfte Angabe.", + "approvalsReviewerNoteAuthenticated": "Der Hub prüft Ihren Zugang, übernimmt den Namen aber unverändert von Studio. Bis der Hub die Identität aus dem geprüften Zugang selbst ableitet, kennzeichnet Studio den Namen im Eintrag als ungeprüfte Angabe.", + "approvalsReviewerNoteUnknown": "Studio konnte die Zugangs-Einstellungen dieses Hubs nicht lesen (dafür braucht es Admin-Rechte). Der Name wird unverändert übernommen und deshalb im Eintrag als ungeprüfte Angabe gekennzeichnet.", + "approvalsReviewerUnverifiedTooltip": "Ungeprüfte Angabe: Dieser Name stammt vom entscheidenden Studio, nicht vom Hub.", "approvalsRejectDialogHelp": "Die Ablehnung stoppt den Vorgang an diesem Schritt und wird mit Ihrer Begründung im Prüfprotokoll festgehalten.", "approvalsRejectReasonHelper": "Pflichtfeld — wird im Prüfprotokoll (Audit-Log) festgehalten.", "approvalsBatchNoDataTitle": "Ohne Prüfdaten freigeben?", diff --git a/lib/l10n/app_en.arb b/lib/l10n/app_en.arb index bbee0d6..51012a2 100644 --- a/lib/l10n/app_en.arb +++ b/lib/l10n/app_en.arb @@ -692,6 +692,7 @@ "auditClearDialogTitle": "Clear audit log?", "auditClearDialogBody": "Wipes every audit event on the active channel and seeds a fresh chain.reset marker carrying reviewer + reason. Refused on beta / production. Irreversible.", "auditClearReviewerLabel": "Reviewer", + "auditClearReviewerHelper": "Recorded as an unchecked claim (prefix \"unverified:\") — the hub stores this name as sent.", "auditClearReasonLabel": "Reason (recorded in chain.reset marker)", "auditClearReasonHelper": "Required — auditors will read this.", "auditClearLogButton": "Clear log", @@ -1120,6 +1121,13 @@ "approvalsRequestFallback": "Approval required for this step", "approvalsFlowStepMeta": "Flow: {flow} · Step: {step}", "approvalsIntroHelp": "Paused processes (flows) waiting for your decision. Each card shows which process is holding at which step and what data it puts in front of you — Approve resumes it, Reject stops it with your reason.", + "approvalsReviewerLabel": "You are deciding as", + "approvalsReviewerUnverifiedPill": "not verified", + "approvalsReviewerRecordedAs": "Recorded as: {value}", + "approvalsReviewerNoteAnonymous": "This hub accepts calls without credentials, so a decision cannot be proven to be anyone's. Studio therefore marks the name inside the record itself as an unchecked claim.", + "approvalsReviewerNoteAuthenticated": "The hub checks your access but records the name exactly as Studio sends it. Until the hub derives the identity from the checked access itself, Studio marks the name inside the record as an unchecked claim.", + "approvalsReviewerNoteUnknown": "Studio could not read this hub's access settings (that needs admin rights). The name is stored exactly as sent and is therefore marked inside the record as an unchecked claim.", + "approvalsReviewerUnverifiedTooltip": "Unchecked claim: this name comes from the deciding Studio, not from the hub.", "approvalsRejectDialogHelp": "Rejecting stops the process at this step and is recorded with your reason in the audit trail.", "approvalsRejectReasonHelper": "Required — recorded in the audit log.", "approvalsBatchNoDataTitle": "Approve without review data?", diff --git a/lib/l10n/app_localizations.dart b/lib/l10n/app_localizations.dart index 285e659..241658b 100644 --- a/lib/l10n/app_localizations.dart +++ b/lib/l10n/app_localizations.dart @@ -2384,6 +2384,12 @@ abstract class AppLocalizations { /// **'Reviewer'** String get auditClearReviewerLabel; + /// No description provided for @auditClearReviewerHelper. + /// + /// In en, this message translates to: + /// **'Recorded as an unchecked claim (prefix \"unverified:\") — the hub stores this name as sent.'** + String get auditClearReviewerHelper; + /// No description provided for @auditClearReasonLabel. /// /// In en, this message translates to: @@ -3560,6 +3566,48 @@ abstract class AppLocalizations { /// **'Paused processes (flows) waiting for your decision. Each card shows which process is holding at which step and what data it puts in front of you — Approve resumes it, Reject stops it with your reason.'** String get approvalsIntroHelp; + /// No description provided for @approvalsReviewerLabel. + /// + /// In en, this message translates to: + /// **'You are deciding as'** + String get approvalsReviewerLabel; + + /// No description provided for @approvalsReviewerUnverifiedPill. + /// + /// In en, this message translates to: + /// **'not verified'** + String get approvalsReviewerUnverifiedPill; + + /// No description provided for @approvalsReviewerRecordedAs. + /// + /// In en, this message translates to: + /// **'Recorded as: {value}'** + String approvalsReviewerRecordedAs(Object value); + + /// No description provided for @approvalsReviewerNoteAnonymous. + /// + /// In en, this message translates to: + /// **'This hub accepts calls without credentials, so a decision cannot be proven to be anyone\'s. Studio therefore marks the name inside the record itself as an unchecked claim.'** + String get approvalsReviewerNoteAnonymous; + + /// No description provided for @approvalsReviewerNoteAuthenticated. + /// + /// In en, this message translates to: + /// **'The hub checks your access but records the name exactly as Studio sends it. Until the hub derives the identity from the checked access itself, Studio marks the name inside the record as an unchecked claim.'** + String get approvalsReviewerNoteAuthenticated; + + /// No description provided for @approvalsReviewerNoteUnknown. + /// + /// In en, this message translates to: + /// **'Studio could not read this hub\'s access settings (that needs admin rights). The name is stored exactly as sent and is therefore marked inside the record as an unchecked claim.'** + String get approvalsReviewerNoteUnknown; + + /// No description provided for @approvalsReviewerUnverifiedTooltip. + /// + /// In en, this message translates to: + /// **'Unchecked claim: this name comes from the deciding Studio, not from the hub.'** + String get approvalsReviewerUnverifiedTooltip; + /// No description provided for @approvalsRejectDialogHelp. /// /// In en, this message translates to: diff --git a/lib/l10n/app_localizations_de.dart b/lib/l10n/app_localizations_de.dart index a26d558..a074c42 100644 --- a/lib/l10n/app_localizations_de.dart +++ b/lib/l10n/app_localizations_de.dart @@ -1351,6 +1351,10 @@ class AppLocalizationsDe extends AppLocalizations { @override String get auditClearReviewerLabel => 'Prüfer'; + @override + String get auditClearReviewerHelper => + 'Wird als ungeprüfte Angabe festgehalten (Präfix „unverified:“) — der Hub übernimmt den Namen unverändert.'; + @override String get auditClearReasonLabel => 'Begründung (im chain.reset-Marker festgehalten)'; @@ -2062,6 +2066,33 @@ class AppLocalizationsDe extends AppLocalizations { String get approvalsIntroHelp => 'Hier warten pausierte Vorgänge (Flows) auf Ihre Entscheidung. Jede Karte zeigt, welcher Vorgang an welchem Schritt hält und welche Daten er Ihnen vorlegt — Freigeben setzt ihn fort, Ablehnen stoppt ihn mit Ihrer Begründung.'; + @override + String get approvalsReviewerLabel => 'Sie entscheiden als'; + + @override + String get approvalsReviewerUnverifiedPill => 'nicht überprüft'; + + @override + String approvalsReviewerRecordedAs(Object value) { + return 'Wird gespeichert als: $value'; + } + + @override + String get approvalsReviewerNoteAnonymous => + 'Dieser Hub nimmt Aufrufe ohne Zugangsdaten an — eine Entscheidung lässt sich damit niemandem nachweisen. Studio kennzeichnet den Namen deshalb im Eintrag selbst als ungeprüfte Angabe.'; + + @override + String get approvalsReviewerNoteAuthenticated => + 'Der Hub prüft Ihren Zugang, übernimmt den Namen aber unverändert von Studio. Bis der Hub die Identität aus dem geprüften Zugang selbst ableitet, kennzeichnet Studio den Namen im Eintrag als ungeprüfte Angabe.'; + + @override + String get approvalsReviewerNoteUnknown => + 'Studio konnte die Zugangs-Einstellungen dieses Hubs nicht lesen (dafür braucht es Admin-Rechte). Der Name wird unverändert übernommen und deshalb im Eintrag als ungeprüfte Angabe gekennzeichnet.'; + + @override + String get approvalsReviewerUnverifiedTooltip => + 'Ungeprüfte Angabe: Dieser Name stammt vom entscheidenden Studio, nicht vom Hub.'; + @override String get approvalsRejectDialogHelp => 'Die Ablehnung stoppt den Vorgang an diesem Schritt und wird mit Ihrer Begründung im Prüfprotokoll festgehalten.'; diff --git a/lib/l10n/app_localizations_en.dart b/lib/l10n/app_localizations_en.dart index bf6c5d9..dd54266 100644 --- a/lib/l10n/app_localizations_en.dart +++ b/lib/l10n/app_localizations_en.dart @@ -1364,6 +1364,10 @@ class AppLocalizationsEn extends AppLocalizations { @override String get auditClearReviewerLabel => 'Reviewer'; + @override + String get auditClearReviewerHelper => + 'Recorded as an unchecked claim (prefix \"unverified:\") — the hub stores this name as sent.'; + @override String get auditClearReasonLabel => 'Reason (recorded in chain.reset marker)'; @@ -2066,6 +2070,33 @@ class AppLocalizationsEn extends AppLocalizations { String get approvalsIntroHelp => 'Paused processes (flows) waiting for your decision. Each card shows which process is holding at which step and what data it puts in front of you — Approve resumes it, Reject stops it with your reason.'; + @override + String get approvalsReviewerLabel => 'You are deciding as'; + + @override + String get approvalsReviewerUnverifiedPill => 'not verified'; + + @override + String approvalsReviewerRecordedAs(Object value) { + return 'Recorded as: $value'; + } + + @override + String get approvalsReviewerNoteAnonymous => + 'This hub accepts calls without credentials, so a decision cannot be proven to be anyone\'s. Studio therefore marks the name inside the record itself as an unchecked claim.'; + + @override + String get approvalsReviewerNoteAuthenticated => + 'The hub checks your access but records the name exactly as Studio sends it. Until the hub derives the identity from the checked access itself, Studio marks the name inside the record as an unchecked claim.'; + + @override + String get approvalsReviewerNoteUnknown => + 'Studio could not read this hub\'s access settings (that needs admin rights). The name is stored exactly as sent and is therefore marked inside the record as an unchecked claim.'; + + @override + String get approvalsReviewerUnverifiedTooltip => + 'Unchecked claim: this name comes from the deciding Studio, not from the hub.'; + @override String get approvalsRejectDialogHelp => 'Rejecting stops the process at this step and is recorded with your reason in the audit trail.'; diff --git a/lib/pages/approvals.dart b/lib/pages/approvals.dart index 4d05883..7b5e28f 100644 --- a/lib/pages/approvals.dart +++ b/lib/pages/approvals.dart @@ -1,11 +1,11 @@ import 'dart:convert'; -import 'dart:io'; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; import '../data/error_presentation.dart'; import '../data/hub.dart'; +import '../data/reviewer_identity.dart'; import '../data/workspace.dart'; import '../l10n/app_localizations.dart'; import '../theme/theme.dart'; @@ -52,18 +52,26 @@ class _ApprovalsPageState extends State { /// and falls back to per-row Approve / Reject buttons. final Set _selectedIds = {}; bool _batchInFlight = false; - // Reviewer identity recorded in the audit log. Defaults to - // the OS user (closest stable identity Studio has without an - // auth backend); operators can override it per session. - late final String _reviewer = _defaultReviewer(); - static String _defaultReviewer() { - final user = - Platform.environment['USER'] ?? - Platform.environment['USERNAME'] ?? - 'studio'; - return '$user@studio'; - } + /// Operator-facing handle of whoever is deciding here. The OS + /// account is a label, not an identity — [HubService.approve] + /// marks it as an unchecked claim on the wire, and the strip + /// above the inbox says so in plain words. See + /// `lib/data/reviewer_identity.dart`. + final String _reviewer = ReviewerIdentity.localHandle; + + /// What actually goes into `decided_by` — the handle plus the + /// marker saying it is an unchecked client claim. Computed here + /// (not only inside [HubService], which normalises again as a + /// backstop) because the strip above the inbox shows the operator + /// this very string before they decide. + String get _reviewerWire => ReviewerIdentity.wire(_reviewer); + + /// What that attribution is worth on THIS hub. Starts unknown and + /// stays unknown when the policy cannot be read (AuthStatus is + /// admin-scoped, and old hubs answer UNIMPLEMENTED) — the strip + /// has an honest line for each case. + ReviewerAssurance _assurance = ReviewerAssurance.unknown; @override void initState() { @@ -71,6 +79,21 @@ class _ApprovalsPageState extends State { Workspace.instance.addListener(_refresh); Workspace.instance.ensureLoaded(); _refresh(); + _loadAssurance(); + } + + Future _loadAssurance() async { + ReviewerAssurance resolved; + try { + final policy = await HubService.instance.authStatus(); + resolved = reviewerAssuranceFor(policy.anonymousAllowed); + } catch (_) { + // Denied / unsupported / hub gone: not knowing is a state of + // its own, never an excuse to imply the attribution is sound. + resolved = ReviewerAssurance.unknown; + } + if (!mounted) return; + setState(() => _assurance = resolved); } @override @@ -127,7 +150,7 @@ class _ApprovalsPageState extends State { } if (!mounted) return; try { - await HubService.instance.approve(a.id, _reviewer); + await HubService.instance.approve(a.id, _reviewerWire); _toast(l.approvalsApprovedToast(a.flowName, a.stepId)); _refresh(); } catch (e) { @@ -142,7 +165,7 @@ class _ApprovalsPageState extends State { final reason = await _promptReason(context); if (reason == null || reason.isEmpty) return; try { - await HubService.instance.reject(a.id, _reviewer, reason); + await HubService.instance.reject(a.id, _reviewerWire, reason); _toast(l.approvalsRejectedToast(a.flowName, a.stepId)); _refresh(); } catch (e) { @@ -205,7 +228,7 @@ class _ApprovalsPageState extends State { Object? firstError; for (final a in picked) { try { - await HubService.instance.approve(a.id, _reviewer); + await HubService.instance.approve(a.id, _reviewerWire); ok += 1; _selectedIds.remove(a.id); } catch (e) { @@ -235,7 +258,7 @@ class _ApprovalsPageState extends State { Object? firstError; for (final a in picked) { try { - await HubService.instance.reject(a.id, _reviewer, reason); + await HubService.instance.reject(a.id, _reviewerWire, reason); ok += 1; _selectedIds.remove(a.id); } catch (e) { @@ -370,6 +393,8 @@ class _ApprovalsPageState extends State { children: [ _PendingList( future: _pendingFuture, + reviewer: _reviewer, + assurance: _assurance, selectedIds: _selectedIds, batchInFlight: _batchInFlight, onApprove: _approve, @@ -390,6 +415,8 @@ class _ApprovalsPageState extends State { class _PendingList extends StatelessWidget { final Future> future; + final String reviewer; + final ReviewerAssurance assurance; final Set selectedIds; final bool batchInFlight; final void Function(ApprovalRecord) onApprove; @@ -403,6 +430,8 @@ class _PendingList extends StatelessWidget { const _PendingList({ required this.future, + required this.reviewer, + required this.assurance, required this.selectedIds, required this.batchInFlight, required this.onApprove, @@ -453,6 +482,21 @@ class _PendingList extends StatelessWidget { ), child: ChainInlineHelp(text: l.approvalsIntroHelp), ), + // Who the audit trail will name, and what that name is + // worth — stated before the decision, not discovered + // afterwards by a lawyer reading the export. + Padding( + padding: const EdgeInsets.fromLTRB( + ChainSpace.xl, + ChainSpace.md, + ChainSpace.xl, + 0, + ), + child: _ReviewerIdentityStrip( + handle: reviewer, + assurance: assurance, + ), + ), Expanded( child: Stack( children: [ @@ -505,6 +549,108 @@ class _PendingList extends StatelessWidget { } } +/// "You are deciding as …" — names the attribution the audit trail +/// will carry and, in one plain sentence, what it is worth on this +/// hub. Studio cannot prove the identity (the hub stores whatever a +/// client sends as `decided_by`), so the honest move is to say that +/// out loud instead of letting the name pass for proof. +class _ReviewerIdentityStrip extends StatelessWidget { + final String handle; + final ReviewerAssurance assurance; + + const _ReviewerIdentityStrip({ + required this.handle, + required this.assurance, + }); + + @override + Widget build(BuildContext context) { + final theme = Theme.of(context); + final l = AppLocalizations.of(context)!; + final note = switch (assurance) { + ReviewerAssurance.anonymousHub => l.approvalsReviewerNoteAnonymous, + ReviewerAssurance.accessControlled => + l.approvalsReviewerNoteAuthenticated, + ReviewerAssurance.unknown => l.approvalsReviewerNoteUnknown, + }; + return Container( + padding: const EdgeInsets.all(ChainSpace.md), + decoration: BoxDecoration( + color: theme.colorScheme.surfaceContainerHigh, + borderRadius: BorderRadius.circular(ChainRadius.sm), + border: Border.all(color: theme.colorScheme.outlineVariant), + ), + child: Row( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Icon( + Icons.badge_outlined, + size: 16, + color: theme.colorScheme.onSurfaceVariant, + ), + const SizedBox(width: ChainSpace.sm), + Expanded( + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + // Wrap, not Row: a long handle plus the pill must + // reflow instead of overflowing on narrow windows. + Wrap( + crossAxisAlignment: WrapCrossAlignment.center, + spacing: ChainSpace.sm, + runSpacing: 4, + children: [ + Text( + l.approvalsReviewerLabel, + style: theme.textTheme.bodySmall?.copyWith( + color: theme.colorScheme.onSurfaceVariant, + ), + ), + SelectableText( + handle, + maxLines: 1, + style: ChainTheme.mono( + size: 12, + color: theme.colorScheme.onSurface, + ), + ), + ChainPill( + label: l.approvalsReviewerUnverifiedPill, + tone: ChainPillTone.warning, + icon: Icons.gpp_maybe_outlined, + ), + ], + ), + const SizedBox(height: 4), + Text( + note, + style: theme.textTheme.bodySmall?.copyWith( + color: theme.colorScheme.onSurfaceVariant, + height: 1.4, + ), + ), + const SizedBox(height: 4), + // The literal value that lands in `decided_by`, so + // nobody is surprised by the prefix when they read + // the log, the CLI output or a DB export. + SelectableText( + l.approvalsReviewerRecordedAs( + ReviewerIdentity.wire(handle), + ), + style: ChainTheme.mono( + size: 11, + color: theme.colorScheme.onSurfaceVariant, + ), + ), + ], + ), + ), + ], + ), + ); + } +} + /// Floating action bar that surfaces when the operator /// multi-selects pending approvals. Lets them approve or /// reject the whole picked set in one round-trip per item; @@ -967,13 +1113,7 @@ class _HistoryRow extends StatelessWidget { ), ), const SizedBox(width: ChainSpace.md), - if (record.decidedBy.isNotEmpty) - Text( - record.decidedBy, - style: theme.textTheme.bodySmall?.copyWith( - color: theme.colorScheme.onSurfaceVariant, - ), - ), + if (record.decidedBy.isNotEmpty) _DecidedBy(record.decidedBy), ], ), ), @@ -1002,6 +1142,58 @@ class _HistoryRow extends StatelessWidget { } } +/// Renders a stored `decided_by`. A value Studio wrote carries the +/// `unverified:` marker — show the readable name and keep the marker +/// visible as a warning glyph rather than dropping either. A value +/// without the marker (legacy row, a CLI decision) gets no badge at +/// all: Studio does not know where it came from and must not imply +/// it was checked. +class _DecidedBy extends StatelessWidget { + final String recorded; + const _DecidedBy(this.recorded); + + @override + Widget build(BuildContext context) { + final theme = Theme.of(context); + final l = AppLocalizations.of(context)!; + final parsed = ReviewerIdentity.parse(recorded); + final name = Text( + parsed.handle, + style: theme.textTheme.bodySmall?.copyWith( + color: theme.colorScheme.onSurfaceVariant, + ), + overflow: TextOverflow.ellipsis, + ); + if (!parsed.isSelfDeclared) return name; + return Tooltip( + message: l.approvalsReviewerUnverifiedTooltip, + child: Row( + mainAxisSize: MainAxisSize.min, + children: [ + Icon( + Icons.gpp_maybe_outlined, + size: 14, + color: theme.colorScheme.onSurfaceVariant, + ), + const SizedBox(width: 4), + Flexible(child: name), + ], + ), + ); + } +} + +/// "who decided" for the detail dialog's one-line summary: the +/// readable name, and — when Studio wrote it — the plain-words +/// hint that the attribution is an unchecked claim. +String _decidedByLabel(BuildContext context, String recorded) { + final l = AppLocalizations.of(context)!; + if (recorded.isEmpty) return l.approvalsUnknownReviewer; + final parsed = ReviewerIdentity.parse(recorded); + if (!parsed.isSelfDeclared) return parsed.handle; + return '${parsed.handle} (${l.approvalsReviewerUnverifiedPill})'; +} + class _HistoryDialog extends StatelessWidget { final ApprovalRecord record; const _HistoryDialog({required this.record}); @@ -1039,7 +1231,7 @@ class _HistoryDialog extends StatelessWidget { theme, AppLocalizations.of(context)!.approvalsDialogDecided, '${_formatTimestamp(record.decidedAt!.toLocal())} ' - '· ${record.decidedBy.isEmpty ? AppLocalizations.of(context)!.approvalsUnknownReviewer : record.decidedBy}', + '· ${_decidedByLabel(context, record.decidedBy)}', ), if (record.createdAt != null) _kv( diff --git a/lib/pages/audit.dart b/lib/pages/audit.dart index 6dda034..1dd0012 100644 --- a/lib/pages/audit.dart +++ b/lib/pages/audit.dart @@ -8,6 +8,7 @@ import 'package:flutter/material.dart'; import '../data/error_presentation.dart'; import '../data/friendly_error.dart'; import '../data/hub.dart'; +import '../data/reviewer_identity.dart'; import '../data/workspace.dart'; import '../l10n/app_localizations.dart'; import '../theme/theme.dart'; @@ -1258,7 +1259,9 @@ class _ExplanationPanel extends StatelessWidget { } } -/// Outcome of the clear-audit confirmation dialog. +/// Outcome of the clear-audit confirmation dialog. [reviewer] is +/// already the wire value — the marked, unchecked claim that lands +/// in the `chain.reset` marker (see `data/reviewer_identity.dart`). class _ClearOutcome { final String reviewer; final String reason; @@ -1266,10 +1269,11 @@ class _ClearOutcome { } /// Two-field confirmation dialog for "clear the audit log". -/// Reviewer defaults to the OS user (closest stable identity -/// without an auth backend); reason has no default so the -/// operator has to type *something* — the chain.reset marker -/// must carry context. +/// Reviewer prefills with the local handle (the OS account is a +/// label, not an identity — [HubService.clearEventLog] marks it as +/// an unchecked claim on the wire, and the field's helper says so); +/// reason has no default so the operator has to type *something* — +/// the chain.reset marker must carry context. class _ClearAuditDialog extends StatefulWidget { const _ClearAuditDialog(); @@ -1291,11 +1295,7 @@ class _ClearAuditDialogState extends State<_ClearAuditDialog> { @override void initState() { super.initState(); - final user = - Platform.environment['USER'] ?? - Platform.environment['USERNAME'] ?? - 'operator'; - _reviewer = TextEditingController(text: '$user@studio'); + _reviewer = TextEditingController(text: ReviewerIdentity.localHandle); _reason = TextEditingController(); } @@ -1329,6 +1329,8 @@ class _ClearAuditDialogState extends State<_ClearAuditDialog> { controller: _reviewer, decoration: InputDecoration( labelText: l.auditClearReviewerLabel, + helperText: l.auditClearReviewerHelper, + helperMaxLines: 3, border: const OutlineInputBorder(), isDense: true, ), @@ -1365,7 +1367,7 @@ class _ClearAuditDialogState extends State<_ClearAuditDialog> { : () => Navigator.pop( context, _ClearOutcome( - reviewer: _reviewer.text.trim(), + reviewer: ReviewerIdentity.wire(_reviewer.text), reason: _reason.text.trim(), ), ), diff --git a/test/approvals_reviewer_identity_test.dart b/test/approvals_reviewer_identity_test.dart new file mode 100644 index 0000000..58e77bf --- /dev/null +++ b/test/approvals_reviewer_identity_test.dart @@ -0,0 +1,288 @@ +// Reviewer identity on the surfaces that write it — approvals page +// and the audit-log wipe. +// +// Guard for the legal finding of the 2026-07-26 usertest: the hub +// stores whatever reviewer string a client sends as `decided_by`, +// so Studio must (a) never send a bare handle that reads like a +// proven identity, (b) tell the reviewer BEFORE the decision what +// the attribution is worth on this hub, and (c) present a stored +// value by what it actually proves. +// +// Runs against the scriptable FakeHubService — never a real hub. + +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +import 'package:chain_studio/data/hub.dart'; +import 'package:chain_studio/data/reviewer_identity.dart'; +import 'package:chain_studio/main.dart'; + +import 'support/fake_hub.dart'; + +String _allText(WidgetTester tester) { + final buf = StringBuffer(); + for (final w in tester.widgetList(find.byType(Text))) { + buf.writeln(w.data ?? w.textSpan?.toPlainText() ?? ''); + } + for (final w in tester.widgetList( + find.byType(SelectableText), + )) { + buf.writeln(w.data ?? ''); + } + return buf.toString(); +} + +ApprovalRecord _pending() => ApprovalRecord( + id: 'apr-1', + flowName: 'classify-and-file', + stepId: 'review', + prompt: 'Bitte die Klassifikation prüfen', + payloadPreview: '{"label":"Rechnung"}', + createdAt: DateTime.utc(2026, 7, 26, 12, 0, 0), + expiresAt: null, + status: 'pending', + decidedAt: null, + decidedBy: '', + reason: '', + project: 'lbs', + flowExecution: 'run-abc123', +); + +ApprovalRecord _decided(String decidedBy) => ApprovalRecord( + id: 'apr-2', + flowName: 'classify-and-file', + stepId: 'review', + prompt: 'Bitte die Klassifikation prüfen', + payloadPreview: null, + createdAt: DateTime.utc(2026, 7, 26, 12, 0, 0), + expiresAt: null, + status: 'approved', + decidedAt: DateTime.utc(2026, 7, 26, 12, 5, 0), + decidedBy: decidedBy, + reason: '', + project: 'lbs', + flowExecution: 'run-abc123', +); + +/// Fixed frame budget instead of `pumpAndSettle`: the page keeps a +/// progress indicator alive on the hidden tab, so settling never +/// completes (same reason the origin suite pumps explicitly). +Future _frames(WidgetTester tester, [int count = 12]) async { + for (var i = 0; i < count; i++) { + await tester.pump(const Duration(milliseconds: 200)); + } +} + +/// Boot Studio on the approvals page against [fake]. +Future _openApprovals(WidgetTester tester, FakeHubService fake) async { + SharedPreferences.setMockInitialValues({}); + tester.view.physicalSize = const Size(1280, 900); + tester.view.devicePixelRatio = 1.0; + addTearDown(tester.view.reset); + await tester.pumpWidget( + const StudioApp( + initialThemeMode: ThemeModeValue.dark, + initialLocale: Locale('de'), + ), + ); + await tester.pump(const Duration(milliseconds: 100)); + await tester.tap(find.byKey(const ValueKey('sidebar-item-approvals'))); + for (var i = 0; i < 12; i++) { + await tester.pump(const Duration(milliseconds: 200)); + } +} + +Future _teardownFrames(WidgetTester tester) async { + await tester.pumpWidget(const SizedBox.shrink()); + await tester.pump(const Duration(minutes: 1)); +} + +void main() { + setUp(() => ReviewerIdentity.debugHandle = 'stefan@studio'); + tearDown(() => ReviewerIdentity.debugHandle = null); + + testWidgets('the inbox names who will be recorded, flags the claim as ' + 'unchecked and shows the literal stored value', (tester) async { + final fake = installFakeHub(); + fake.approvals = [_pending()]; + await _openApprovals(tester, fake); + final text = _allText(tester); + + expect(text, contains('Sie entscheiden als')); + expect(text, contains('stefan@studio')); + expect(text, contains('nicht überprüft')); + // No surprise later: the exact string that lands in decided_by. + expect(text, contains('unverified:stefan@studio')); + await _teardownFrames(tester); + }); + + testWidgets('an anonymous hub says a decision cannot be pinned on ' + 'anyone', (tester) async { + final fake = installFakeHub(); + fake.approvals = [_pending()]; + // Default fake policy: static validator, anonymous allowed. + await _openApprovals(tester, fake); + expect(_allText(tester), contains('ohne Zugangsdaten')); + await _teardownFrames(tester); + }); + + testWidgets('an auth-enabled hub says the access is checked but the ' + 'name is not', (tester) async { + final fake = installFakeHub(); + fake.approvals = [_pending()]; + fake.authPolicy = const HubAuthPolicy( + validator: 'static', + anonymousAllowed: false, + tokens: [], + ); + await _openApprovals(tester, fake); + expect(_allText(tester), contains('prüft Ihren Zugang')); + await _teardownFrames(tester); + }); + + testWidgets('an unreadable auth policy stays honest instead of ' + 'guessing', (tester) async { + final fake = installFakeHub(); + fake.approvals = [_pending()]; + // AuthStatus is admin-scoped: a plain reviewer token is denied. + fake.authPolicy = null; + await _openApprovals(tester, fake); + expect(_allText(tester), contains('Admin-Rechte')); + // Still flagged — not knowing never upgrades the attribution. + expect(_allText(tester), contains('nicht überprüft')); + await _teardownFrames(tester); + }); + + testWidgets('approving sends the marked attribution, never a bare ' + 'handle', (tester) async { + final fake = installFakeHub(); + fake.approvals = [_pending()]; + await _openApprovals(tester, fake); + + await tester.tap(find.widgetWithText(FilledButton, 'Freigeben').first); + for (var i = 0; i < 8; i++) { + await tester.pump(const Duration(milliseconds: 200)); + } + + expect(fake.decisions, hasLength(1)); + expect(fake.decisions.single.id, 'apr-1'); + expect(fake.decisions.single.reviewer, 'unverified:stefan@studio'); + expect( + fake.decisions.single.reviewer, + startsWith(kUnverifiedReviewerPrefix), + reason: 'the decide path must not claim an identity Studio ' + 'cannot prove', + ); + await _teardownFrames(tester); + }); + + testWidgets('rejecting sends the marked attribution too', (tester) async { + final fake = installFakeHub(); + fake.approvals = [_pending()]; + await _openApprovals(tester, fake); + + await tester.tap(find.widgetWithText(OutlinedButton, 'Ablehnen').first); + await _frames(tester); + await tester.enterText(find.byType(TextField).last, 'Beleg fehlt'); + await _frames(tester); + await tester.tap(find.widgetWithText(FilledButton, 'Ablehnen').last); + for (var i = 0; i < 8; i++) { + await tester.pump(const Duration(milliseconds: 200)); + } + + expect(fake.decisions, hasLength(1)); + expect(fake.decisions.single.reviewer, 'unverified:stefan@studio'); + expect(fake.decisions.single.reason, 'Beleg fehlt'); + await _teardownFrames(tester); + }); + + testWidgets('the history shows a marked value by its name and keeps ' + 'the "unchecked" flag', (tester) async { + final fake = installFakeHub(); + fake.approvals = [_decided('unverified:anna@ops')]; + await _openApprovals(tester, fake); + await tester.tap(find.text('Verlauf')); + await _frames(tester); + + final text = _allText(tester); + expect(text, contains('anna@ops')); + // The raw marker is not what a human should read in the list. + expect(text, isNot(contains('unverified:anna@ops'))); + + // The detail dialog spells the trust level out in words. + await tester.tap(find.textContaining('classify-and-file').last); + await _frames(tester); + expect(_allText(tester), contains('anna@ops (nicht überprüft)')); + await _teardownFrames(tester); + }); + + testWidgets('the audit-log wipe records the same kind of marked ' + 'attribution in the chain.reset marker', (tester) async { + // Same bug class as approvals: the reset marker names a + // "Prüfer" the hub never checked, and auditors read exactly + // that marker to explain a chain discontinuity. + final fake = installFakeHub(); + SharedPreferences.setMockInitialValues({}); + tester.view.physicalSize = const Size(1280, 900); + tester.view.devicePixelRatio = 1.0; + addTearDown(tester.view.reset); + await tester.pumpWidget( + const StudioApp( + initialThemeMode: ThemeModeValue.dark, + initialLocale: Locale('de'), + ), + ); + await tester.pump(const Duration(milliseconds: 100)); + await tester.tap(find.byKey(const ValueKey('sidebar-item-audit'))); + await _frames(tester); + + await tester.tap(find.byTooltip('Weitere Aktionen')); + await _frames(tester); + // Tap the entry, not its label: the overflow menu's labels are + // wider than the 256px menu, so the label's centre lies outside + // the hit box (a separate, unrelated layout defect on that + // menu). The entry itself is what the operator hits. + await tester.tap( + find.ancestor( + of: find.textContaining('Entwicklungs-Reset'), + matching: find.byType(PopupMenuItem), + ), + ); + await _frames(tester); + + // The field says what will be stored, before it is stored. + expect(_allText(tester), contains('unverified:')); + + await tester.enterText(find.byType(TextField).last, 'Testdaten verworfen'); + await _frames(tester); + await tester.tap(find.widgetWithText(FilledButton, 'Log löschen')); + await _frames(tester); + + expect(fake.clears, hasLength(1)); + expect(fake.clears.single.reviewer, 'unverified:stefan@studio'); + expect(fake.clears.single.reason, 'Testdaten verworfen'); + await _teardownFrames(tester); + }); + + testWidgets('a legacy / CLI value without a marker is shown as-is and ' + 'never labelled either way', (tester) async { + final fake = installFakeHub(); + fake.approvals = [_decided('ops-lead')]; + await _openApprovals(tester, fake); + await tester.tap(find.text('Verlauf')); + await _frames(tester); + + await tester.tap(find.textContaining('classify-and-file').last); + await _frames(tester); + final text = _allText(tester); + expect(text, contains('ops-lead')); + expect( + text, + isNot(contains('ops-lead (nicht überprüft)')), + reason: 'Studio does not know where an unmarked value came ' + 'from and must not classify it', + ); + await _teardownFrames(tester); + }); +} diff --git a/test/reviewer_identity_test.dart b/test/reviewer_identity_test.dart new file mode 100644 index 0000000..e9a6872 --- /dev/null +++ b/test/reviewer_identity_test.dart @@ -0,0 +1,105 @@ +// Reviewer identity — the attribution Studio writes into the audit +// trail must never look better than it is. +// +// Background (usertest 2026-07-26, legal persona): the hub copies +// the client's `reviewer` string into `decided_by` unchecked, so a +// bare "stefan@studio" in an export reads like a proven identity +// while being an arbitrary client claim. Until the hub derives the +// value from the authenticated caller (contract: +// docs/reviewer-identity.md), every value Studio sends carries the +// `unverified:` marker — and everything Studio *reads* is presented +// by what it actually proves. + +import 'package:flutter_test/flutter_test.dart'; + +import 'package:chain_studio/data/reviewer_identity.dart'; + +void main() { + setUp(() => ReviewerIdentity.debugHandle = 'stefan@studio'); + tearDown(() => ReviewerIdentity.debugHandle = null); + + group('what Studio sends', () { + test('every reviewer string leaves Studio marked as a claim', () { + expect(ReviewerIdentity.wire(), 'unverified:stefan@studio'); + expect(ReviewerIdentity.wire('anna@ops'), 'unverified:anna@ops'); + }); + + test('marking is idempotent — a value that already carries the ' + 'marker is not wrapped twice', () { + final once = ReviewerIdentity.wire('anna@ops'); + expect(ReviewerIdentity.wire(once), once); + // The page computes the wire value for display AND HubService + // normalises again as a backstop; that must stay harmless. + expect(ReviewerIdentity.wire(ReviewerIdentity.wire(once)), once); + }); + + test('a blank handle falls back to the local one instead of ' + 'sending an empty attribution', () { + expect(ReviewerIdentity.wire(' '), 'unverified:stefan@studio'); + expect(ReviewerIdentity.wire(''), 'unverified:stefan@studio'); + }); + + test('surrounding whitespace never reaches the record', () { + expect(ReviewerIdentity.wire(' anna@ops '), 'unverified:anna@ops'); + }); + + test('the local handle is a label, not an identity — but always ' + 'present', () { + ReviewerIdentity.debugHandle = null; + expect(ReviewerIdentity.localHandle, isNotEmpty); + expect( + ReviewerIdentity.wire(), + startsWith(kUnverifiedReviewerPrefix), + reason: 'no code path may send a bare handle', + ); + }); + }); + + group('what Studio reads back', () { + test('a marked value is shown by its name and flagged as a claim', () { + final parsed = ReviewerIdentity.parse('unverified:anna@ops'); + expect(parsed.handle, 'anna@ops'); + expect(parsed.trust, RecordedReviewerTrust.selfDeclared); + expect(parsed.isSelfDeclared, isTrue); + }); + + test('an unmarked value (legacy row, CLI decision) is never ' + 'presented as checked', () { + final parsed = ReviewerIdentity.parse('stefan@studio'); + expect(parsed.handle, 'stefan@studio'); + expect(parsed.trust, RecordedReviewerTrust.unknown); + expect( + parsed.isSelfDeclared, + isFalse, + reason: 'unknown provenance must not be labelled either way', + ); + }); + + test('a marker with nothing behind it keeps the marker visible ' + 'rather than rendering an empty reviewer', () { + final parsed = ReviewerIdentity.parse('unverified:'); + expect(parsed.handle, 'unverified:'); + expect(parsed.isSelfDeclared, isTrue); + }); + + test('round-trips what Studio wrote', () { + final parsed = ReviewerIdentity.parse(ReviewerIdentity.wire()); + expect(parsed.handle, 'stefan@studio'); + expect(parsed.isSelfDeclared, isTrue); + }); + }); + + group('assurance level per hub policy', () { + test('anonymous hub — nobody can be tied to a decision', () { + expect(reviewerAssuranceFor(true), ReviewerAssurance.anonymousHub); + }); + + test('auth-enabled hub — the access is checked, the name is not', () { + expect(reviewerAssuranceFor(false), ReviewerAssurance.accessControlled); + }); + + test('unreadable policy stays unknown — never optimistic', () { + expect(reviewerAssuranceFor(null), ReviewerAssurance.unknown); + }); + }); +} diff --git a/test/support/fake_hub.dart b/test/support/fake_hub.dart index 542638a..21b166d 100644 --- a/test/support/fake_hub.dart +++ b/test/support/fake_hub.dart @@ -230,6 +230,55 @@ class FakeHubService extends Fake implements HubService { Future> listSatellites() => _async('listSatellites', () => const []); + /// Auth policy the pages read (approvals derives from it what a + /// reviewer attribution is worth). Default: the local-dev hub — + /// static validator, anonymous calls allowed. Set to `null` to + /// script a hub whose policy Studio may not read; `authStatus` + /// then fails like a PermissionDenied. + HubAuthPolicy? authPolicy = const HubAuthPolicy( + validator: 'static', + anonymousAllowed: true, + tokens: [], + ); + + @override + Future authStatus() => _async('authStatus', () { + final policy = authPolicy; + if (policy == null) { + throw const FakeGrpcError(7, 'admin scope required'); + } + return policy; + }); + + /// Every decision that reached the wire, in order — the guard for + /// "Studio never claims an identity it cannot prove" inspects the + /// reviewer string exactly as the hub would store it. + final List<({String id, String reviewer, String? reason})> decisions = []; + + /// Reviewer + reason handed to the audit-log wipe, same purpose. + final List<({String reviewer, String reason})> clears = []; + + @override + Future<({int purged, String channel})> clearEventLog({ + required String reviewer, + required String reason, + }) => _async('clearEventLog', () { + clears.add((reviewer: reviewer, reason: reason)); + return (purged: 0, channel: 'local'); + }); + + @override + Future approve(String id, String reviewer) => _async( + 'approve', + () => decisions.add((id: id, reviewer: reviewer, reason: null)), + ); + + @override + Future reject(String id, String reviewer, String reason) => _async( + 'reject', + () => decisions.add((id: id, reviewer: reviewer, reason: reason)), + ); + @override Future systemAiStatus() => _async( 'systemAiStatus',