What changed. The typed-data signing screen shows a red "TOKEN PERMISSION" warning when the type being signed is Permit or one of Permit2's signature types, naming the spender and each token's symbol, address and amount (Unlimited at the field's largest value).
ethers ignores the page's primaryType and signs the one struct in types nothing else refers to. The screen now shows that type; typed data whose stated type is missing or differs gets an error line and a disabled Sign button, and Sign refuses it again before decrypting.
What a reader would trip over. Whether to warn depends on the type name ethers signs. The spender, tokens and amounts, and whether a Permit is EIP-2612's or DAI's, come only from fields that type declares, because ethers drops every other key; the exception is a Permit's token, which is the domain's verifyingContract. Anything missing reads Unknown (an NFT permit has no amount); the domain, type and message lines still follow. Typed data that cannot be read at all shows as raw text, and is refused.
Disclosures
Deviation: no deadline or expiry in the warning; reason on the issue.
Judgement call: DAI's older Permit and Permit2's batch and witness transfer types are recognised too.
Judgement call: the refusal is on the sign screen, with Reject available, not in the background.
Judgement call: object values in the domain now show as JSON, as message values already did.
Unchanged: the message lines still list every key the page sent, signed or not.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/AutistMask/issues/400.
**What changed.** The typed-data signing screen shows a red "TOKEN PERMISSION" warning when the type being signed is `Permit` or one of Permit2's signature types, naming the spender and each token's symbol, address and amount (`Unlimited` at the field's largest value).
ethers ignores the page's `primaryType` and signs the one struct in `types` nothing else refers to. The screen now shows that type; typed data whose stated type is missing or differs gets an error line and a disabled Sign button, and Sign refuses it again before decrypting.
**What a reader would trip over.** Whether to warn depends on the type name ethers signs. The spender, tokens and amounts, and whether a `Permit` is EIP-2612's or DAI's, come only from fields that type declares, because ethers drops every other key; the exception is a `Permit`'s token, which is the domain's `verifyingContract`. Anything missing reads `Unknown` (an NFT permit has no amount); the domain, type and message lines still follow. Typed data that cannot be read at all shows as raw text, and is refused.
**Disclosures**
- Deviation: no deadline or expiry in the warning; reason on the issue.
- Judgement call: DAI's older `Permit` and Permit2's batch and witness transfer types are recognised too.
- Judgement call: the refusal is on the sign screen, with Reject available, not in the background.
- Judgement call: object values in the domain now show as JSON, as message values already did.
- Unchanged: the message lines still list every key the page sent, signed or not.
Model: opus-5-5
src/popup/views/approval.js line 458: the amount in the warning can differ from the amount being signed. The code tells DAI's permit apart from EIP-2612's by whether the page's message has an allowed key. But ethers signs only the fields that the Permit type declares and ignores any other keys. So a page can send an EIP-2612 Permit for the largest amount and add "allowed": false to the message. That signs exactly the same as the plain unlimited permit, yet the warning shows 0.0000 USDC instead of Unlimited. The spender at line 510 has the same problem. It is read from the message even when the signed type has no spender field, so a decoy key gets named as the spender. Acceptable: decide which permit it is, and read the spender and amount, only from the fields the signed type declares in types. Add a test for the extra-key case.
src/popup/views/approval.js lines 540 and 564: some typed data has a type named Permit without the EIP-2612 or DAI fields. For that data the warning code throws, and the whole message falls back to raw JSON text. There is no warning, and the domain, type and message lines that next shows today are gone too, while Sign stays enabled. ethers signs such data, and real contracts accept it: the permit for Uniswap v3 position NFTs (spender, tokenId, nonce, deadline) is one example. This contradicts the PR body, which says ethers would refuse to sign such data. It also contradicts the README, which promises the warning for any typed data whose primary type is Permit. Acceptable: when the permission fields cannot be read, keep the warning (with whatever can be read) and the normal lines, or refuse the request. The README and the PR body must match what the code does.
tests/typedDataPermit.test.js: no test covers the refusal itself. The tests call typedDataRefusal() directly, so the sign screen could stop disabling Sign, or the Sign handler could stop refusing, and no test would fail. Acceptable: a test that shows a request with a mismatched primary type on the sign screen and checks that Sign is disabled and the error is shown. A second test clicks Sign and checks that nothing is decrypted or signed. These can drive the screen the way tests/deleteWalletLostPassword.test.js drives its own.
Model: opus-5-5
FAIL
1. `src/popup/views/approval.js` line 458: the amount in the warning can differ from the amount being signed. The code tells DAI's permit apart from EIP-2612's by whether the page's message has an `allowed` key. But ethers signs only the fields that the `Permit` type declares and ignores any other keys. So a page can send an EIP-2612 `Permit` for the largest amount and add `"allowed": false` to the message. That signs exactly the same as the plain unlimited permit, yet the warning shows `0.0000 USDC` instead of `Unlimited`. The spender at line 510 has the same problem. It is read from the message even when the signed type has no `spender` field, so a decoy key gets named as the spender. Acceptable: decide which permit it is, and read the spender and amount, only from the fields the signed type declares in `types`. Add a test for the extra-key case.
2. `src/popup/views/approval.js` lines 540 and 564: some typed data has a type named `Permit` without the EIP-2612 or DAI fields. For that data the warning code throws, and the whole message falls back to raw JSON text. There is no warning, and the domain, type and message lines that `next` shows today are gone too, while Sign stays enabled. ethers signs such data, and real contracts accept it: the permit for Uniswap v3 position NFTs (`spender`, `tokenId`, `nonce`, `deadline`) is one example. This contradicts the PR body, which says ethers would refuse to sign such data. It also contradicts the README, which promises the warning for any typed data whose primary type is `Permit`. Acceptable: when the permission fields cannot be read, keep the warning (with whatever can be read) and the normal lines, or refuse the request. The README and the PR body must match what the code does.
3. `tests/typedDataPermit.test.js`: no test covers the refusal itself. The tests call `typedDataRefusal()` directly, so the sign screen could stop disabling Sign, or the Sign handler could stop refusing, and no test would fail. Acceptable: a test that shows a request with a mismatched primary type on the sign screen and checks that Sign is disabled and the error is shown. A second test clicks Sign and checks that nothing is decrypted or signed. These can drive the screen the way `tests/deleteWalletLostPassword.test.js` drives its own.
Model: opus-5-5
The spender, tokens and amounts, and whether a Permit is EIP-2612's or DAI's, are now read only from fields the signed type declares in types; new tests add an undeclared allowed key and an undeclared spender key.
The warning can no longer fail: whatever the declared fields do not give reads Unknown, the domain, type and message lines stay, and only typed data that cannot be read at all (which is refused) is shown as raw text; a new test uses the Uniswap v3 position permit, and the README and PR body now say exactly this.
New tests open the sign screen through show() against a DOM stub in the shape of tests/deleteWalletLostPassword.test.js: a matching type leaves Sign enabled, a mismatched one shows the error with Sign disabled, and clicking Sign anyway calls no decrypt and sends no sign response.
How I checked each new test fails on what it guards: the tests for findings 1 and 2 failed against the previous head of this branch, and each sign-screen test failed with its own refusal check removed by hand.
Model: opus-5-5
1. The spender, tokens and amounts, and whether a `Permit` is EIP-2612's or DAI's, are now read only from fields the signed type declares in `types`; new tests add an undeclared `allowed` key and an undeclared `spender` key.
2. The warning can no longer fail: whatever the declared fields do not give reads `Unknown`, the domain, type and message lines stay, and only typed data that cannot be read at all (which is refused) is shown as raw text; a new test uses the Uniswap v3 position permit, and the README and PR body now say exactly this.
3. New tests open the sign screen through `show()` against a DOM stub in the shape of `tests/deleteWalletLostPassword.test.js`: a matching type leaves Sign enabled, a mismatched one shows the error with Sign disabled, and clicking Sign anyway calls no decrypt and sends no sign response.
How I checked each new test fails on what it guards: the tests for findings 1 and 2 failed against the previous head of this branch, and each sign-screen test failed with its own refusal check removed by hand.
Model: opus-5-5
tests/typedDataPermit.test.js: of Permit2's six signature types, only PermitSingle is tested, and no test has more than one token. Taking PermitBatch and the four transfer types out of the warning (src/popup/views/approval.js lines 522 and 531-534) leaves every test passing. So does showing a batch's list of tokens as one unknown token (line 461). The README promises the warning for all of Permit2's signature types, with a line for each token. Acceptable: a test with a PermitBatch of two tokens that checks both tokens and both amounts appear in the warning. Also a test that PermitTransferFrom, PermitWitnessTransferFrom, PermitBatchTransferFrom and PermitBatchWitnessTransferFrom each show the warning, naming their spender and token.
README.md lines 1813-1814, the TODO.md entry (line 56) and the PR body say that everything the warning names is read only from the fields the signed type declares. But a Permit's token is the domain's verifyingContract (src/popup/views/approval.js line 517), and no field of Permit declares it. Acceptable: all three say that the token of a Permit comes from the domain's verifyingContract.
Model: opus-5-5
FAIL
1. `tests/typedDataPermit.test.js`: of Permit2's six signature types, only `PermitSingle` is tested, and no test has more than one token. Taking `PermitBatch` and the four transfer types out of the warning (`src/popup/views/approval.js` lines 522 and 531-534) leaves every test passing. So does showing a batch's list of tokens as one unknown token (line 461). The README promises the warning for all of Permit2's signature types, with a line for each token. Acceptable: a test with a `PermitBatch` of two tokens that checks both tokens and both amounts appear in the warning. Also a test that `PermitTransferFrom`, `PermitWitnessTransferFrom`, `PermitBatchTransferFrom` and `PermitBatchWitnessTransferFrom` each show the warning, naming their spender and token.
2. `README.md` lines 1813-1814, the `TODO.md` entry (line 56) and the PR body say that everything the warning names is read only from the fields the signed type declares. But a `Permit`'s token is the domain's `verifyingContract` (`src/popup/views/approval.js` line 517), and no field of `Permit` declares it. Acceptable: all three say that the token of a `Permit` comes from the domain's `verifyingContract`.
Model: opus-5-5
State for the next manager: branch issue-400-typed-data-permit-warning, last pushed commit eaaf159. Left: a second rework fixing both findings of #414 (comment) (tests for a two-token PermitBatch and for each of the four Permit2 transfer types, each checked to fail without the code it covers; README.md, the TODO.md entry and the PR body saying a Permit's token comes from the domain's verifyingContract), then a fresh independent review. No rework is running; one was started and stopped before it pushed anything.
Model: opus-5-5
State for the next manager: branch `issue-400-typed-data-permit-warning`, last pushed commit `eaaf159`. Left: a second rework fixing both findings of https://git.eeqj.de/sneak/AutistMask/pulls/414#issuecomment-117396 (tests for a two-token `PermitBatch` and for each of the four Permit2 transfer types, each checked to fail without the code it covers; `README.md`, the `TODO.md` entry and the PR body saying a `Permit`'s token comes from the domain's `verifyingContract`), then a fresh independent review. No rework is running; one was started and stopped before it pushed anything.
Model: opus-5-5
The typed-data screen listed a Permit or Permit2 signature as plain
key/value lines, like a sign-in message. It now shows a red warning for
them naming the spender and each token and amount, read only from the
fields the signed type declares (a Permit's token is the domain's
verifyingContract); anything those fields do not give reads Unknown.
The screen printed the page's primaryType, but ethers signs the type it
derives from types. It now shows that type, and refuses typed data whose
stated type is missing or differs: error line, Sign disabled, checked
again where signing starts.
Deviation: the warning names no deadline or expiry; see the issue.
Judgement call: DAI's older permit and Permit2's batch and witness
transfer types are recognised too.
Model: opus-5-5
Added a test for a two-token PermitBatch that checks both tokens and both amounts, and one each for PermitTransferFrom, PermitWitnessTransferFrom, PermitBatchTransferFrom and PermitBatchWitnessTransferFrom that checks the warning names its spender and token.
README.md, the TODO.md entry, the PR body, the commit body and the code comment above the warning now say a Permit's token comes from the domain's verifyingContract.
Each new test failed with the code it covers removed by hand (the PermitBatch case, the reading of a batch's list of tokens, the four transfer-type cases) and passed again once that code was restored.
Model: opus-5-5
1. Added a test for a two-token `PermitBatch` that checks both tokens and both amounts, and one each for `PermitTransferFrom`, `PermitWitnessTransferFrom`, `PermitBatchTransferFrom` and `PermitBatchWitnessTransferFrom` that checks the warning names its spender and token.
2. `README.md`, the `TODO.md` entry, the PR body, the commit body and the code comment above the warning now say a `Permit`'s token comes from the domain's `verifyingContract`.
Each new test failed with the code it covers removed by hand (the `PermitBatch` case, the reading of a batch's list of tokens, the four transfer-type cases) and passed again once that code was restored.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #400.
What changed. The typed-data signing screen shows a red "TOKEN PERMISSION" warning when the type being signed is
Permitor one of Permit2's signature types, naming the spender and each token's symbol, address and amount (Unlimitedat the field's largest value).ethers ignores the page's
primaryTypeand signs the one struct intypesnothing else refers to. The screen now shows that type; typed data whose stated type is missing or differs gets an error line and a disabled Sign button, and Sign refuses it again before decrypting.What a reader would trip over. Whether to warn depends on the type name ethers signs. The spender, tokens and amounts, and whether a
Permitis EIP-2612's or DAI's, come only from fields that type declares, because ethers drops every other key; the exception is aPermit's token, which is the domain'sverifyingContract. Anything missing readsUnknown(an NFT permit has no amount); the domain, type and message lines still follow. Typed data that cannot be read at all shows as raw text, and is refused.Disclosures
Permitand Permit2's batch and witness transfer types are recognised too.Model: opus-5-5
FAIL
src/popup/views/approval.jsline 458: the amount in the warning can differ from the amount being signed. The code tells DAI's permit apart from EIP-2612's by whether the page's message has anallowedkey. But ethers signs only the fields that thePermittype declares and ignores any other keys. So a page can send an EIP-2612Permitfor the largest amount and add"allowed": falseto the message. That signs exactly the same as the plain unlimited permit, yet the warning shows0.0000 USDCinstead ofUnlimited. The spender at line 510 has the same problem. It is read from the message even when the signed type has nospenderfield, so a decoy key gets named as the spender. Acceptable: decide which permit it is, and read the spender and amount, only from the fields the signed type declares intypes. Add a test for the extra-key case.src/popup/views/approval.jslines 540 and 564: some typed data has a type namedPermitwithout the EIP-2612 or DAI fields. For that data the warning code throws, and the whole message falls back to raw JSON text. There is no warning, and the domain, type and message lines thatnextshows today are gone too, while Sign stays enabled. ethers signs such data, and real contracts accept it: the permit for Uniswap v3 position NFTs (spender,tokenId,nonce,deadline) is one example. This contradicts the PR body, which says ethers would refuse to sign such data. It also contradicts the README, which promises the warning for any typed data whose primary type isPermit. Acceptable: when the permission fields cannot be read, keep the warning (with whatever can be read) and the normal lines, or refuse the request. The README and the PR body must match what the code does.tests/typedDataPermit.test.js: no test covers the refusal itself. The tests calltypedDataRefusal()directly, so the sign screen could stop disabling Sign, or the Sign handler could stop refusing, and no test would fail. Acceptable: a test that shows a request with a mismatched primary type on the sign screen and checks that Sign is disabled and the error is shown. A second test clicks Sign and checks that nothing is decrypted or signed. These can drive the screen the waytests/deleteWalletLostPassword.test.jsdrives its own.Model: opus-5-5
786613ce2dtoeaaf1593c0Permitis EIP-2612's or DAI's, are now read only from fields the signed type declares intypes; new tests add an undeclaredallowedkey and an undeclaredspenderkey.Unknown, the domain, type and message lines stay, and only typed data that cannot be read at all (which is refused) is shown as raw text; a new test uses the Uniswap v3 position permit, and the README and PR body now say exactly this.show()against a DOM stub in the shape oftests/deleteWalletLostPassword.test.js: a matching type leaves Sign enabled, a mismatched one shows the error with Sign disabled, and clicking Sign anyway calls no decrypt and sends no sign response.How I checked each new test fails on what it guards: the tests for findings 1 and 2 failed against the previous head of this branch, and each sign-screen test failed with its own refusal check removed by hand.
Model: opus-5-5
FAIL
tests/typedDataPermit.test.js: of Permit2's six signature types, onlyPermitSingleis tested, and no test has more than one token. TakingPermitBatchand the four transfer types out of the warning (src/popup/views/approval.jslines 522 and 531-534) leaves every test passing. So does showing a batch's list of tokens as one unknown token (line 461). The README promises the warning for all of Permit2's signature types, with a line for each token. Acceptable: a test with aPermitBatchof two tokens that checks both tokens and both amounts appear in the warning. Also a test thatPermitTransferFrom,PermitWitnessTransferFrom,PermitBatchTransferFromandPermitBatchWitnessTransferFromeach show the warning, naming their spender and token.README.mdlines 1813-1814, theTODO.mdentry (line 56) and the PR body say that everything the warning names is read only from the fields the signed type declares. But aPermit's token is the domain'sverifyingContract(src/popup/views/approval.jsline 517), and no field ofPermitdeclares it. Acceptable: all three say that the token of aPermitcomes from the domain'sverifyingContract.Model: opus-5-5
State for the next manager: branch
issue-400-typed-data-permit-warning, last pushed commiteaaf159. Left: a second rework fixing both findings of #414 (comment) (tests for a two-tokenPermitBatchand for each of the four Permit2 transfer types, each checked to fail without the code it covers;README.md, theTODO.mdentry and the PR body saying aPermit's token comes from the domain'sverifyingContract), then a fresh independent review. No rework is running; one was started and stopped before it pushed anything.Model: opus-5-5
eaaf1593c0to457dd18642PermitBatchthat checks both tokens and both amounts, and one each forPermitTransferFrom,PermitWitnessTransferFrom,PermitBatchTransferFromandPermitBatchWitnessTransferFromthat checks the warning names its spender and token.README.md, theTODO.mdentry, the PR body, the commit body and the code comment above the warning now say aPermit's token comes from the domain'sverifyingContract.Each new test failed with the code it covers removed by hand (the
PermitBatchcase, the reading of a batch's list of tokens, the four transfer-type cases) and passed again once that code was restored.Model: opus-5-5
PASS
Model: opus-5-5