Skip to content

fix(sdk): expectDid pins the RFC 9421 signer; strict sig1 parsing; owner JSON depth limit - #195

Merged
WillCybertron merged 2 commits into
mainfrom
fix/sdk-expect-did
Nov 12, 2026
Merged

WillCybertron merged 2 commits into
mainfrom
fix/sdk-expect-did

Conversation

@WillCybertron

Copy link
Copy Markdown
Contributor

Day 6 review gate, SDK part (oneentity docs/mobile/review/GATE-REPORT.md C1, H1, H2, M1 and the SDK lows). Ships in 1.1.0: nothing is staged yet, so the versions stay and CHANGELOG.md gains the entries.

What

  • Signer pin. verifyRequest / verify_request and verifyResponse / verify_response take expectDid / expect_did (one DID or a list). Any other keyid, a did:key included, fails with the new code unexpected_signer before any key is read or fetched. A did:web DID in the pin needs no allowDidWeb. The docs say allowDidWeb alone is not a pin: three products read it as one and accepted any did:key as the account service.
  • Strict sig1 parsing. Signature-Input and Signature must each hold exactly one sig1 member, and a repeated parameter fails with missing_headers (TS and Python).
  • Depth parity. Owner key logs, approvals and payload summaries refuse JSON nested deeper than 64 containers, in TS, Python, Swift and Kotlin, so no SDK accepts what another refuses.
  • Python hardening. parse_json refuses NaN and Infinity, and input too deep for the parser, with InvalidInputError; _der_to_raw refuses a truncated sequence. Hostile input is a verifier failure (a 422), never another exception (a 500).
  • Swift, found while fixing. The owner walker's lazy.compactMap { }.first ran its closure twice per level on the way to an error: 2^depth calls, so a deep input hung the verifier. Plain loops now.
  • Vectors: expect_did in http-signature.json, duplicate-param and two-sig1 cases, keylog-1/depth-limit.json and too-deep.json, approval NaN and depth cases, a 64-deep payload and a refused 65-deep one. The invalid http vectors now carry their own names instead of their base case's.

Decisions

  • Swift and Kotlin get the depth rule but no expectDid: they have no RFC 9421 verifier, which is a server-side profile that no app receives.
  • The pin is checked before the keyid's form, so a did:key under a did:web pin answers unexpected_signer, and nothing is fetched for a signer outside the pin.

Test plan

  • Pre-push verify.sh green through the shared heavy-job gate (TS, Python, Kotlin 125, Swift 38).
  • Each new guard removed in turn (pin, duplicate parameter, second sig1, depth, NaN, RecursionError, DER bounds) fails its tests.
  • Swift with the old walker restored ran over six minutes on the new vectors without finishing (stopped by hand); with the loops the suite takes 0.13 s.

…ner JSON depth limit

Day 6 review gate (C1, H1, H2, M1 and the SDK lows):
- verifyRequest / verify_request and verifyResponse / verify_response take
  expectDid / expect_did (one DID or a list): any other keyid, a did:key
  included, fails with the new code unexpected_signer before any key is
  read or fetched. allowDidWeb alone is not a pin, and the docs say so.
  Vectors: expect_did in test-vectors/http-signature.json.
- Signature-Input and Signature must hold exactly one sig1 member, and a
  repeated parameter fails with missing_headers. Vectors: duplicate-param,
  two-sig1-inputs, two-sig1-signatures. Invalid http vectors now carry
  their own names instead of their base case's.
- Owner key logs, approvals and payload summaries refuse JSON nested
  deeper than 64 containers, in all four SDKs. Vectors: keylog-1
  depth-limit and too-deep, approval clientDataJSON depth and NaN cases,
  a 64-deep payload and a 65-deep refused one.
- Python: parse_json refuses NaN and Infinity, and input too deep for the
  parser, with InvalidInputError; _der_to_raw refuses a truncated sequence.
  Hostile input is a verifier failure, never another exception.
- Swift: the owner walker used lazy.compactMap.first, which runs its
  closure twice per level on the way to an error: 2^depth calls, a hang
  on a deep input. Plain loops now.
@chatgpt-codex-connector

Copy link
Copy Markdown
Codex Review Summary: ❌ Failed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ❌ Failed 2026-11-12T09:33:26.479611Z
We couldn’t complete this review. Comment @codex review to try again.
9690959 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@WillCybertron
WillCybertron merged commit c41d526 into main Nov 12, 2026
1 check passed
@WillCybertron
WillCybertron deleted the fix/sdk-expect-did branch November 12, 2026 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant