feat(security): add privacy-egress-diligence skill - #90
Draft
MajorLift wants to merge 3 commits into
Draft
Conversation
`app/scripts/constants/sentry-state.ts` decides what user data leaves the machine. It carries 116 fields currently set to `true` — meaning the real value is copied and sent — and it is edited inside ordinary feature PRs (onboarding, swaps, rewards, the analytics controller) with no CODEOWNERS entry, so no privacy reviewer is automatically tagged. Same shape as `lavamoat-policy-diligence`: the diff is mechanical, the judgement is what each grant means. `git diff` finds every newly-`true` field exactly, so the deliverable is not "is it listed" but what the field holds at runtime — a mask path cannot distinguish `selectedTab` from `selectedAddress`. Sorts findings into safe / needs-narrowing / must-not-egress with the evidence and a proposed mask for each, and renders no accept verdict: that call belongs to privacy and legal, and a confident reviewer "this is fine" is precisely what lets an unreviewed field through. Also covers the sibling pipes a PR widens at the same time — new MetaMetrics or Segment properties, and error strings that interpolate runtime values, both of which egress regardless of the mask.
Contributor
Author
Context budgetWhat this PR costs an agent, measured from an install rather than read from the diff. Three tiers, and only the first is unavoidable.
Frontmatter is the only tier paid unconditionally — every agent loads it on every run once the skill is installed, used or not, because it is what the agent reads to decide relevance. The 28 skills across the eleven open skill PRs sit at a median of ~1,716 tokens selected and ~1,860 with references followed. All are within the 1,536-character description budget. Selected is paid only when the agent picks the skill. + refs & knowledge is the ceiling if every bundled reference is then read; it is a worst case, not an expectation. Method
These figures are pinned to the commit above and drift on every push; #96 tracks automating them. |
MajorLift
marked this pull request as draft
July 31, 2026 16:00
…d protecting Two halves. INTRODUCED lists added lines that send, store, or log off-device, tagged by the sensitivity of the payload near the call. WORSENED lists removed consent gates, sanitisers, sampling gates, and validations — and that half is the reason the script exists, because a deleted protection adds no code and so is invisible to anything that scans what a diff introduces. Verified against two real PRs. On extension#42519 it ranks the compliance call first of sixteen egress sites, tagged `identifier` — the same site found by hand, found here without being told where to look. On extension#43869 it reports five removed consent gates including `if (!canSubmitAnalytics(...))`. Two defects the runs exposed, both of which had it reporting nothing: - Payload sensitivity was read from the egress line alone, but a call and its argument sit on different lines — `submitRequestToBackground<T>(` then `[addresses]`. Now read from a window around the call site. - `\baddress\b` does not match `addresses`, nor `\btoken\b` match `tokenList`. Identifiers appear pluralised and camel-cased far more often than bare, so a closing word boundary missed the entire payload on real call sites. Guards removed and re-added in the same diff are excluded as refactors. No verdict is offered: whether a flow is acceptable depends on disclosure, jurisdiction, and intent, and a screening check a user can decline screens nobody — so an absent consent gate is not automatically a defect. Anything sensitive belongs in a private tracker rather than a public comment.
MajorLift
force-pushed
the
jongsun/add/privacy-egress-diligence
branch
from
August 3, 2026 11:49
ee529d3 to
477009f
Compare
It reviews what a field carries and not whether the send is permitted, and a reader who runs it has covered half the question while believing otherwise. An unstated scope on a review is the same defect as an unstated scope on a search: the negative reads wider than the thing that produced it. The other axis lives in the private repo, because naming the conditions under which a control does not run is a different kind of document from describing the control.
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
1 participant
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.
Summary
privacy-egress-diligencetosecurity— review for changes to what user data leaves the device.Why this surface
app/scripts/constants/sentry-state.tsdecides what egresses. Measured onmetamask-extensionat the current head:true— the real value is copied and sent to Sentry.A file governing data egress, changed casually, reviewed by whoever happened to review the feature.
Shape
Deliberately the same as
lavamoat-policy-diligence: detection is mechanical, judgement is not.maskObject(shared/lib/object.utils.ts) makes the risk one-way — unlisted fields degrade to atypeofstring, so only a field promoted totruewidens egress, andgit difffinds those exactly.The trap it names: "the field is in the mask, so someone decided it was fine." The mask is the decision. Presence proves authorship, not review.
So the deliverable is what each field holds at runtime — a mask path cannot distinguish
selectedTabfromselectedAddress— sorted into:falseor a non-identifying substituteIt also covers the sibling pipes a PR widens at the same time: new MetaMetrics/Segment properties, and error strings interpolating runtime values, both of which leak regardless of the mask.
No accept/reject verdict. Whether a field is acceptable to collect belongs to privacy and legal; a confident reviewer "this is fine" is exactly the artifact that lets an unreviewed field through.
Notes
securityrather than a newprivacydomain — a one-skill domain is premature, and the diligence pattern already lives there. Say the word if you'd rather it were separate.instrumentation(feat(analytics): add Sentry, MetaMetrics, Segment, and Tempo skills #76), which covers whether an event is correctly identified and consent-gated. This covers whether its payload is sendable.CHANGELOG.mdentry: skill-only, no CLI change.