fix(persona): Show asks the agent for the value, because it was never sent one - #194
Merged
Merged
Conversation
… sent one `persona/attribute/list` answers a values request with the metadata of every attribute resolving to `sensitivity: high` and the plaintext of none, unless the caller also sets `includeSensitive`. The console never set it. So the pane held no value for `profile.github`, `employer`, `phone.mobile` or anything else the registry treats as sensitive — and it masked the placeholder that stood in for them. The result was two states wearing one shape. A card read `••••` beside a *Show* button; pressing *Show* said "not requested"; and a line underneath explained that the agent "has already sent this value here", which was the one thing that had not happened. The reassuring reading was the wrong one, and nothing on screen distinguished a value being kept off the screen from a value that was never there. The specification is blunt about which half of sensitivity matters: "a consumer that masks a value it has already received defends a screen; it does not keep a card number out of a log, a crash dump or a process's memory. Ignoring this member and masking client-side is a conforming implementation of nothing." - `AttributeListParams.includeSensitive` in `@openvtc/pnm-core/admin`, passed through to the payload. The wire member has existed since trust-tasks 0.17.4; only the client was missing. - The pane keeps listing **without** it, so the plaintext genuinely is not in the page, and *Show* becomes the request: `manager/reveal-value.ts` asks for one attribute, narrowed by `typePrefix` to its own type and matched back by `attributeId` — there is no `attribute/get`, and a type can have siblings. *Hide* drops what was fetched rather than covering it over. - `FactValue` never masks a withheld value. The mask says "a value is here and is being kept off your screen"; over an absent one it said the opposite of the truth. A withheld value now reads "not on this page", and the strip's explanation splits into the two sentences it always needed. - The obvious shortcut — `includeSensitive` on the pane's own listing — is refused in `reveal-value.ts`'s header, with the reason: it is three lines, it makes every *Show* instant, and it puts every card and passport number the holder owns into a React tree because one button might be pressed. Three notes corrected while in the same paragraph, all of them stale rather than wrong when written: the read-path control now exists and is used (CLAUDE.md and `claim-sensitivity.ts`'s header both said it did not); the prefix walk landed in trust-tasks#377 and only ever tightens; and the vocabulary paragraph still said the on-screen word is *fact*, which #191 changed and #192 built on. 11 new tests (339 extension, 549 core), `tsc -b` clean, `npm run build` clean. Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
🛡️ AI Agentic Security Code Review🔎 A manual security review is recommended before merging. Please contact the Security team for specifics and remediation guidance.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pressing Show on an attribute said not requested. It was telling the truth, and everything around it was not.
What was happening
persona/attribute/listanswers a values request with the metadata of every attribute resolving tosensitivity: highand the plaintext of none, unless the caller also setsincludeSensitive. The console never set it — the member landed in trust-tasks 0.17.4 and the client was never given it.So the pane held no value for
profile.github,employer,phone.mobile, or anything else the registry treats as sensitive. And it masked the placeholder standing in for the absent value:GitHub · ••••+ Shownot requestedTwo states wearing one shape, and the reassuring reading was the wrong one.
What it does now
The specification is blunt about which half of sensitivity matters:
includeSensitivereaches the wire.AttributeListParams.includeSensitivein@openvtc/pnm-core/admin, passed through to the payload.manager/reveal-value.tsasks for one attribute, narrowed bytypePrefixto its own type and matched back byattributeId: there is noattribute/getin the vocabulary, and a type can have siblings, so position would show the holder the wrong value with no sign anything was wrong.includeSensitiveon the pane's own listing is three lines, makes every Show instant, and puts every card and passport number the holder owns into a React tree because one button might be pressed.reveal-value.ts's header says so, so it is not "fixed" back.Notes corrected in passing
All three were true when written and had gone stale:
CLAUDE.mdandclaim-sensitivity.ts's header both said the read-path control "does not exist yet". It exists, and this PR is the console using it.payment.giftCardinheritspayment's gating,name.somethingNewdoes not inheritname'snone.Checks
tsc -bclean across workspaces;npm run buildclean,manager.jsa single chunk.includeValuesandincludeSensitiveand is narrowed to one type; the answer is matched by id; a still-withheld value and a stale credential are two different sentences. The four render tests drive the real pane — withheld reads as missing, Show fetches exactly one attribute, Hide drops it, and a value the agent did send still covers locally without asking again.Not in this PR
There is still no way to change an attribute's visibility from the console. The wire supports it —
sensitivityandreleaseare per-attribute overrides onattribute/put, and absent means "resolve from the registry" — but the attribute editor exposes neither, soprofile.githubandemployerare masked because they are unregistered types and there is no way to say otherwise. That is a separate change to the editor.