decision: RULES.md states three things the code contradicts #211

Open
opened 2026-08-11 14:33:49 +02:00 by clawbot · 9 comments
Collaborator

Three divergences found by separate workers while verifying documentation against source. RULES.md is owner-only and says the code must change to match it rather than the reverse, so each needs your call on which side gives.

1. External services count. The External Communication checklist says the extension contacts "exactly three external services: configured RPC endpoint, CoinDesk price API, and Blockscout block-explorer API". The code contacts five — src/shared/phishingDomains.js fetches raw.githubusercontent.com, and src/shared/etherscanLabels.js fetches etherscan.io.

2. Password scope. "Password only required for signing operations" is untrue. src/popup/views/deleteWallet.js and the private-key export in src/popup/views/addressDetail.js both require it, and #161 adds a third.

3. Approval persistence. "TX and sign approvals persist across popup close/reopen (toolbar popup)" no longer describes the code: those approvals use windows.create(), and the windows.onRemoved listener resolves them with EIP-1193 code 4001. A stale comment in src/background/index.js in the port-disconnect branch still says "Keep pending — user can reopen the toolbar popup", describing behaviour that no longer exists.

Options

For each: (a) update RULES.md to describe the shipped behaviour, or (b) change the code to match RULES.md.

Recommendation

  • 1 — (a). The two extra services are deliberate features (phishing blocklist, address labels); the checklist is stale. Worth restating as a maintained list rather than a count, so adding a service forces a doc change.
  • 2 — (a). Password-gating destructive and secret-revealing actions is correct and should stay; the rule is simply narrower than practice.
  • 3 — (b) for the comment, (a) for the rule. The stale comment in src/background/index.js is just wrong and should be deleted regardless. Whether approvals SHOULD survive popup close is a real product question: rejecting on window close is the safer default for a wallet, so I would update the rule to match — but that is your call, not mine.

Tell me which way on each and I will open the PR. Nothing is blocked on this.

Three divergences found by separate workers while verifying documentation against source. `RULES.md` is owner-only and says the code must change to match it rather than the reverse, so each needs your call on which side gives. **1. External services count.** The External Communication checklist says the extension contacts "exactly three external services: configured RPC endpoint, CoinDesk price API, and Blockscout block-explorer API". The code contacts five — `src/shared/phishingDomains.js` fetches `raw.githubusercontent.com`, and `src/shared/etherscanLabels.js` fetches `etherscan.io`. **2. Password scope.** "Password only required for signing operations" is untrue. `src/popup/views/deleteWallet.js` and the private-key export in `src/popup/views/addressDetail.js` both require it, and https://git.eeqj.de/sneak/AutistMask/issues/161 adds a third. **3. Approval persistence.** "TX and sign approvals persist across popup close/reopen (toolbar popup)" no longer describes the code: those approvals use `windows.create()`, and the `windows.onRemoved` listener resolves them with EIP-1193 code 4001. A stale comment in `src/background/index.js` in the port-disconnect branch still says "Keep pending — user can reopen the toolbar popup", describing behaviour that no longer exists. ## Options For each: **(a)** update `RULES.md` to describe the shipped behaviour, or **(b)** change the code to match `RULES.md`. ## Recommendation - **1 — (a).** The two extra services are deliberate features (phishing blocklist, address labels); the checklist is stale. Worth restating as a maintained list rather than a count, so adding a service forces a doc change. - **2 — (a).** Password-gating destructive and secret-revealing actions is correct and should stay; the rule is simply narrower than practice. - **3 — (b) for the comment, (a) for the rule.** The stale comment in `src/background/index.js` is just wrong and should be deleted regardless. Whether approvals SHOULD survive popup close is a real product question: rejecting on window close is the safer default for a wallet, so I would update the rule to match — but that is your call, not mine. Tell me which way on each and I will open the PR. Nothing is blocked on this.
sneak was assigned by clawbot 2026-08-11 14:33:49 +02:00
Author
Collaborator

Re-checked against the tree. RULES.md says README.md governs where the two disagree, and README.md already describes the code on all three points (and on a fourth, below). So what is left is the wording of RULES.md, which is yours. The stale code comment in item 3 is now #505, which the fleet fixes.

Ask: will you make these four RULES.md edits? Recommendation: yes, as written.

  • External Communication, first line: "Extension contacts only the services README.md External Services lists: the configured RPC endpoint, the CoinDesk price API, the configured Blockscout API, and Etherscan address labels". The phishing list is now bundled at build time and fetches nothing.
  • External Communication, "No user-specific data sent except to the configured RPC endpoint": "User addresses go only to the configured RPC endpoint and the configured Blockscout API; Etherscan receives only the recipient address being confirmed".
  • Secrets & Storage, "Password only required for signing operations": "Password required for signing, deleting a wallet, and showing a recovery phrase or private key".
  • Approval Flow, "TX and sign approvals persist across popup close/reopen (toolbar popup)": "A transaction or signing approval window closed without an answer is rejected with EIP-1193 code 4001".

Model: opus-5-5

Re-checked against the tree. `RULES.md` says `README.md` governs where the two disagree, and `README.md` already describes the code on all three points (and on a fourth, below). So what is left is the wording of `RULES.md`, which is yours. The stale code comment in item 3 is now https://git.eeqj.de/sneak/AutistMask/issues/505, which the fleet fixes. Ask: will you make these four `RULES.md` edits? Recommendation: yes, as written. - External Communication, first line: "Extension contacts only the services README.md External Services lists: the configured RPC endpoint, the CoinDesk price API, the configured Blockscout API, and Etherscan address labels". The phishing list is now bundled at build time and fetches nothing. - External Communication, "No user-specific data sent except to the configured RPC endpoint": "User addresses go only to the configured RPC endpoint and the configured Blockscout API; Etherscan receives only the recipient address being confirmed". - Secrets & Storage, "Password only required for signing operations": "Password required for signing, deleting a wallet, and showing a recovery phrase or private key". - Approval Flow, "TX and sign approvals persist across popup close/reopen (toolbar popup)": "A transaction or signing approval window closed without an answer is rejected with EIP-1193 code 4001". Model: opus-5-5
Owner

we don’t want to send user data to anything but the rpc endpoint, why are we contacting blockscout or etherscan? this is a major privacy leak bug and use of these must default to off and be opt-in in prefs or during onboarding or both.

we don’t want to send user data to anything but the rpc endpoint, why are we contacting blockscout or etherscan? this is a major privacy leak bug and use of these must default to off and be opt-in in prefs or during onboarding or both.
Author
Collaborator

Your ruling is #668: Blockscout and Etherscan off by default, opt-in in Settings and on Welcome. #671 builds it and waits for review.

With that ruling, the first two RULES.md edits above change. The wording for them waits until PR 671 lands, so it can describe the code.

Ask: will you make the other two RULES.md edits now? Recommendation: yes, as written.

  • Secrets & Storage, "Password only required for signing operations": "Password required for signing, deleting a wallet, and showing a recovery phrase or private key".
  • Approval Flow, "TX and sign approvals persist across popup close/reopen (toolbar popup)": "A transaction or signing approval window closed without an answer is rejected with EIP-1193 code 4001".

Model: opus-5-5

Your ruling is https://git.eeqj.de/sneak/AutistMask/issues/668: Blockscout and Etherscan off by default, opt-in in Settings and on Welcome. https://git.eeqj.de/sneak/AutistMask/pulls/671 builds it and waits for review. With that ruling, the first two `RULES.md` edits above change. The wording for them waits until PR 671 lands, so it can describe the code. Ask: will you make the other two `RULES.md` edits now? Recommendation: yes, as written. - Secrets & Storage, "Password only required for signing operations": "Password required for signing, deleting a wallet, and showing a recovery phrase or private key". - Approval Flow, "TX and sign approvals persist across popup close/reopen (toolbar popup)": "A transaction or signing approval window closed without an answer is rejected with EIP-1193 code 4001". Model: opus-5-5
Owner

yes to the first one. why are we changing the approval flow? browsers frequently close the popup when clicked away, such as to access a password manager to get the signing passphrase. why do we support cancelling with anything other than the cancel button?

yes to the first one. why are we changing the approval flow? browsers frequently close the popup when clicked away, such as to access a password manager to get the signing passphrase. why do we support cancelling with anything other than the cancel button?
Author
Collaborator

Recorded:

  • Password: you approved the wording. RULES.md is your file, so the edit is yours: in Secrets & Storage, replace "Password only required for signing operations" with "Password required for signing, deleting a wallet, and showing a recovery phrase or private key".
  • Approval flow: we were not changing it on purpose. The code already rejects a request when its approval window closes, and my wording described that. Your rule is the right one, and the code is the bug: #673. A closed window leaves the request pending, and only Cancel rejects it. RULES.md already says this and stays as it is.

The last two RULES.md lines, on what the wallet contacts, wait for #671. I will propose their wording once it lands.

Model: opus-5-5

Recorded: - Password: you approved the wording. `RULES.md` is your file, so the edit is yours: in Secrets & Storage, replace "Password only required for signing operations" with "Password required for signing, deleting a wallet, and showing a recovery phrase or private key". - Approval flow: we were not changing it on purpose. The code already rejects a request when its approval window closes, and my wording described that. Your rule is the right one, and the code is the bug: https://git.eeqj.de/sneak/AutistMask/issues/673. A closed window leaves the request pending, and only Cancel rejects it. `RULES.md` already says this and stays as it is. The last two `RULES.md` lines, on what the wallet contacts, wait for https://git.eeqj.de/sneak/AutistMask/pulls/671. I will propose their wording once it lands. Model: opus-5-5
sneak was unassigned by clawbot 2026-10-24 08:22:24 +02:00
clawbot self-assigned this 2026-10-24 08:22:24 +02:00
clawbot removed their assignment 2026-10-24 08:22:51 +02:00
sneak was assigned by clawbot 2026-10-24 08:22:51 +02:00
Author
Collaborator

#671 has landed on next. Proposed wording for the two External Communication lines in RULES.md, which are yours to edit:

  • "Extension contacts exactly three external services: configured RPC endpoint, CoinDesk price API, and Blockscout block-explorer API" becomes "Until the user changes a setting, the extension contacts only the configured RPC endpoint and the CoinDesk price API, which is sent token symbols only".
  • "No user-specific data sent except to the configured RPC endpoint" becomes "No user-specific data sent except to the configured RPC endpoint, unless the user turns on "Use Blockscout" or "Check recipients on Etherscan", both off by default".

The CoinDesk part changes again if #525 replaces it with on-chain prices.

Model: opus-5-5

https://git.eeqj.de/sneak/AutistMask/pulls/671 has landed on `next`. Proposed wording for the two External Communication lines in `RULES.md`, which are yours to edit: - "Extension contacts exactly three external services: configured RPC endpoint, CoinDesk price API, and Blockscout block-explorer API" becomes "Until the user changes a setting, the extension contacts only the configured RPC endpoint and the CoinDesk price API, which is sent token symbols only". - "No user-specific data sent except to the configured RPC endpoint" becomes "No user-specific data sent except to the configured RPC endpoint, unless the user turns on "Use Blockscout" or "Check recipients on Etherscan", both off by default". The CoinDesk part changes again if https://git.eeqj.de/sneak/AutistMask/issues/525 replaces it with on-chain prices. Model: opus-5-5
Author
Collaborator

#681 has landed: prices now come from Chainlink's feeds through the RPC endpoint, and CoinDesk is gone. That changes the first line proposed at #211 (comment). Both proposed External Communication lines in RULES.md now read:

  • "Extension contacts exactly three external services: configured RPC endpoint, CoinDesk price API, and Blockscout block-explorer API" becomes "Until the user changes a setting, the extension contacts only the configured RPC endpoint".
  • "No user-specific data sent except to the configured RPC endpoint" becomes "No user-specific data sent except to the configured RPC endpoint, unless the user turns on "Use Blockscout" or "Check recipients on Etherscan", both off by default".

Model: opus-5-5

https://git.eeqj.de/sneak/AutistMask/pulls/681 has landed: prices now come from Chainlink's feeds through the RPC endpoint, and CoinDesk is gone. That changes the first line proposed at https://git.eeqj.de/sneak/AutistMask/issues/211#issuecomment-156676. Both proposed External Communication lines in `RULES.md` now read: - "Extension contacts exactly three external services: configured RPC endpoint, CoinDesk price API, and Blockscout block-explorer API" becomes "Until the user changes a setting, the extension contacts only the configured RPC endpoint". - "No user-specific data sent except to the configured RPC endpoint" becomes "No user-specific data sent except to the configured RPC endpoint, unless the user turns on "Use Blockscout" or "Check recipients on Etherscan", both off by default". Model: opus-5-5
Author
Collaborator

The approved password edit is now a task on his sysadmin tracker: sneak/sysadmin#3. The two External Communication lines proposed at #211 (comment) still wait for his yes here.

Model: opus-5-5

The approved password edit is now a task on his sysadmin tracker: https://git.eeqj.de/sneak/sysadmin/issues/3. The two External Communication lines proposed at https://git.eeqj.de/sneak/AutistMask/issues/211#issuecomment-157738 still wait for his yes here. Model: opus-5-5
Author
Collaborator

sneak: the password edit belongs here, not on the sysadmin tracker (sneak/sysadmin#3 (comment)). Both asks are here, assigned to him: make the password edit he approved (#211 (comment)), and approve or change the two External Communication lines (#211 (comment)).

Model: opus-5-5

sneak: the password edit belongs here, not on the sysadmin tracker (https://git.eeqj.de/sneak/sysadmin/issues/3#issuecomment-169770). Both asks are here, assigned to him: make the password edit he approved (https://git.eeqj.de/sneak/AutistMask/issues/211#issuecomment-154475), and approve or change the two External Communication lines (https://git.eeqj.de/sneak/AutistMask/issues/211#issuecomment-157738). Model: opus-5-5
Sign in to join this conversation.
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/AutistMask#211