Skip to content

#11989: signature.dag decoded_or_empty fabricates declared size 0 for undecodable carriers - #12211

Closed
gunbai-bot[bot] wants to merge 160 commits into
mainfrom
step4-fix
Closed

gunbai-bot[bot] wants to merge 160 commits into
mainfrom
step4-fix

Conversation

@gunbai-bot

@gunbai-bot gunbai-bot Bot commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

Auto-opened by session-dashboard for session fierce-crane-31.
Pushing to step4-fix advances this PR.

Worker attestation

Before flipping this PR to ready for review, confirm each item:

  • Title describes the change (not the session id or branch).
  • PR body summarises what and why (replace the TODO below).
  • Tests run: name the command (e.g. npm test, cargo test) and the result.
  • If this closes a work item, the body contains a Closes #N directive.
  • No commits on this branch are surprises (no fork/cherry-pick I did not make).
  • No secrets / credentials / large binaries staged.

Summary

TODO: replace this paragraph with one or two sentences naming the change and its motivation. Reviewers read this first.

Test plan

  • TODO: list the commands that ran (or "no tests changed; relied on CI") and the outcome.

gunbc-ci-auto-heal and others added 30 commits September 21, 2026 15:24
…thn authenticator-data decoders in .dag, claimed over a real Apple-issued attestation

The first of the pure byte decoders the App Attest verifier needs (extdeps.apple.app_attest
attestation_verification_from_implementation has no supplier on main). Three extdeps modules,
each cited: extdeps.standards.rfc_8949 (strict CBOR: definite-length only, floats and reserved
additional info refuse, integers beyond 2^53 refuse before the multiply, nesting bounded, unique
text-key member read), extdeps.standards.rfc_9052 (COSE_Key EC2 P-256 ES256 -> SEC 1 point),
extdeps.standards.webauthn_authenticator_data (the §6.1 layout, whole and 37-octet prefix).

The fixture is a real attestation object and assertion Apple issued to a development build
(veehaitch/devicecheck-appattest ios-14.4.yaml at cb26211f), transcribed as octets with its
published facts; over it the decoders establish Apple's steps 4, 5, 6, 7 and 8 from real bytes.
One upstream fact the sample surfaced: Apple's assertion authenticatorData is 37 octets with the
AT flag set and nothing following, so assertions are read as the prefix, never the full layout.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ers in .dag and the pinned Apple App Attestation Root CA, claimed over the sample's real certificate chain

extdeps.standards.x690_der reads the DER profile and refuses BER's freedoms by name:
indefinite lengths, non-minimal long-form lengths, high tag numbers, unused BIT STRING bits,
non-minimal or negative INTEGERs. extdeps.standards.rfc_5280 reads Certificate/TBSCertificate/
AlgorithmIdentifier/Validity/SubjectPublicKeyInfo/Extension, refuses an outer signature
algorithm that disagrees with the signed one, looks extensions up uniquely, and renders
ECDSA-Sig-Value as fixed-width r||s for the curve verifiers. extdeps.standards.rfc_7468 reads
one PEM block. extdeps.apple.app_attest gains the root CA pinned byte for byte from apple.com.

Over the sample chain the readers establish, from real bytes: Apple's steps 2 and 3 (the leaf's
nonce extension equals SHA-256(authData || clientDataHash)); the leaf's SPKI point IS the
credential key the authData carries; leaf -> CA 1 -> pinned root join by DER Name equality;
CA 1 and the root are secp384r1 / ecdsa-with-SHA384, so the chain step needs P-384.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ther than re-matching CborValue (review 69585)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…P-256 and P-384 as parameter rows, SHA-384, and the frontier retargeted to the decided shape

extdeps.crypto.nist_prime_curve carries the a = -3 Jacobian group law, Shamir double-scalar
ladder, partial public-key validation and ECDSA verification over a SUPPLIED digest, written
once (the formulas #11645 landed for P-256, moved not rewritten). extdeps.crypto.nist_p256 is
now its parameter row plus the SHA-256-bound verifier the device signature uses; the new
extdeps.crypto.nist_p384 is the second row, consumed by Apple's chain step, whose two links are
a P-384 key signing with SHA-256 and a P-384 key signing with SHA-384. extdeps.crypto.sha2 gains
SHA-384 (the SHA-512 computation with its own initial state, truncated) over std.bitwise Word64,
its constants derived from the definition rather than transcribed.

P-256 semantics preserved by execution: the RFC 6979 sample signature verifies and the
different-message control is false through the family fold (88 s / 86 s, 56M / 55M eval steps,
peak 15.5 GiB), and n.G = infinity holds. P-384 established on real bytes: Apple's CA 1 and root
keys are on the curve, a neighbouring point is not; the two chain-link verifications are enrolled
and NOT executed here (at 17 limbs they exceed this container's memory), the same disposition as
#11645's eight P-256 rows: a dedicated native gunbc test row is their executing evidence.

gunbc.auth.approval_device_redemption ecdsa_verification_realization_frontier no longer names
the host-Rust primitive the operator refused; it names the .dag readers landed by #11970/#11975/
this change, the verifier folds still owed, and the execution route decision (natively emitted
handler once MachineWidth<N> reification lands; interpreter cost is the v1-performance row).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ture's ByteSize authority; the two guard-shadowed zero defaults (CBOR argument octets, authenticator-data octets) now refuse as located truncations

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t or a second block; OID subidentifiers refuse a leading 0x80 (X.690 §8.19.2), and the witness now asserts that refusal instead of announcing it

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…dag, Apple's real assertion signature verified by execution, and the signature-seam join

extdeps.apple.app_attest verify_attestation runs Apple's ten steps over the readers: CBOR shape
and fmt, the nonce extension against SHA-256(authData || SHA-256(clientData)), the credential
certificate's public key against the key id, RP ID, counter 0, AAGUID for the environment,
credentialId, chain names and validity at the observed instant against the pinned root, the
validation-category and bundle-version extensions, and last -- because they cost minutes -- the
two chain-link ECDSA verifications. verify_assertion runs steps 1-4, 7, 8 and carries the counter
out. Both supply the seams; nothing else may mint VerifiedAttestation / AuthenticAssertion.

Established on Apple's real objects: the attestation passes every step up to the validation
category the 2021 sample predates (AttestationValidationCategoryAbsent is the first refusal), each
earlier step reds by name when its expectation is wrong, and observing it in 2026 finds the
credential certificate expired at 2021-01-25T12:13:35Z. THE ASSERTION SIGNATURE VERIFIES under the
credential key -- after a defect found only by executing it: Apple's nonce is the signed MESSAGE
(ECDSA-with-SHA256 over the 32 nonce octets), not the digest; the first cut refused and openssl
confirmed the reading.

extdeps.crypto.signature verify_signature is the join the frontier called (b): base64url carriers
-> measured sizes -> the seam -> nist_p256; RFC 6979's sample verifies through it and is
SignatureInvalid over another message. rfc_5280 gains UTCTime/GeneralizedTime -> Timestamp and
validity at an instant. The frontier row now names the execution route as the only remainder.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-> 11, 384 -> 16), not ceiling + 1; annotations corrected. Re-measured: one P-256 verify 73-75 s / 48.5M eval steps, from 88 s / 56M at the extra limb

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…read (X.690 §8.6.2.3); every positional TBS, outer, SPKI, AlgorithmIdentifier, Extension and ECDSA-Sig-Value read asserts its ASN.1 type through der_element_expect, consuming the tag constants that were imported and unused

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ture fields (the placeholder-filled certificate has no constructor); left_pad consumes std.bignat bignat_pad_front instead of re-minting a zero fill

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…mily fold reads the octet rows), the witness's limb count corrected to 16, and the SHA-256/SHA-512 two-pipeline divergence stated on the carrier with its collapsing capability (MachineWidth<N> reification)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… the authenticator data are decoded once (decode_attestation) and carried to the refusal fold and the success mint; the assertion document is decoded once for its counter and its refusal

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…of re-declaring its fields; extnValue and Time reads assert class and form through the typed reads; an explicitly encoded critical FALSE refuses (X.690 §11.5); the never-called der_expect_universal deleted and der_element_expect_universal consumed

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…in counter, key or receipt reaches the seam), a non-unsigned validation category is Undecodable rather than category -1, chain_link_verifies is claimed at its producer (three refusing arms executed over the real leaf and CA; the two verifying links enrolled with the P-384 cost stated), and nist_p256's header says its consumers are the declared frontier; the validity fold reads the nested tbs

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…y one a POST whose body carries its authentication; the wire's header rows deleted; vectors regenerated; Swift mirror

Decision msg_5b415336 (B): gunbc serve hands a route no header but the tailnet login, and the
seed's purpose standing refuses growing it for the approval product, so the enrolment id, the
claimed time and the App Attest assertion ride in the body (ReadAuthentication; PushUpdateRequest
{ auth, push }). The assertion's client data is unchanged.

gunbc.auth.approval_device_routes: enrol (writer gate, verify_attestation over the enrolment
transcript, enrolment_admission, commit, push registration), redeem (writer gate, verify_signature
under the decision key and verify_assertion over the signing input into redeem_device_over_store),
pending / one request / enrolment readback (admit_device_read: active enrolment, +-60 s skew,
assertion over the route's framed client data; the request answer mints a MAC-keyed challenge
and both verb capabilities), push update, and the operator's code-issuing verb as a CliWire
answer. gunbc.auth.approval_broker_serve mounts the six with their own markers; gunbc.roadmap_serve
classifies all six as never mounted by the roadmap. gunbc.durable_cas_file_store cas_slot_keys
(the store's own inverse of its layout) and approval_decision_store pending_escalations list what
the phone lists. gunbc.auth.approval_app_attest_config is AwaitingOperator until the App ID prefix
and bundle version land; every verifier-needing route refuses 503 naming that until then.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e), the pinned root's SHA-256 is a data row computed by a claim over the decoded PEM, and a Certificate, TBS tail or AlgorithmIdentifier with children the profile does not name refuses instead of reading past them

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gunbc-ci-auto-heal and others added 25 commits September 23, 2026 11:17
… the rewrite)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…w (the #11981 dead-band disposition)

CI at daf2bda measured the_key_id_step_reds_on_another_key_id (420 ms) and
the_sample_assertion_passes_rp_id_and_refuses_on_absent_extensions (447 ms) between the 302 ms
enrolment margin and the 500 ms per-subject line: stale under a typed admission, over the margin
without one (v2.workflow.floor_enrolment_margin). As #11981 did for its cliff-band SHA-384
vectors, both become ordinary functions whose executing consumer is the native row
gunbc.auth.approval_device_redemption ecdsa_verification_realization_frontier, which now names
them. Their typed admissions and eval-step drop rows are removed; the verifier drop covers exactly
four identities. The pairing comment on the assertion mint and the witness state which facts lose
floor execution until native emission lands.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…s ask

device_enrol_response_at, device_redeem_response_at and device_push_update_response_at decoded
the whole JSON body before asking approval_write_admission, which depends only on the serving
process: a non-writer paid a decode for a request it was going to refuse (DESIGN section 6b, the
earliest unjustified boundary is the route's ordering, not the claim). The gate now precedes the
decode, so a non-writer is refused 503 whatever the body. The witness supplies a bare body to
the_non_writer_process_is_refused_on_the_mutating_routes and asks the malformed-body claim's
mutating routes as the writer, since decoding now follows the gate.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… head by gallop+bisect) (#12080)

* CAS file store: find the head by gallop+bisect, removing the 4096-generation read cliff

The assertion counter advances its slot one generation per admitted App Attest
assertion, and the store walked a slot linearly from generation 1, refusing past
4096: an enrolment went permanently unreadable (503) after ~4096 admitted
requests. The chain is the store's O_EXCL exclusion mechanism and is kept (no
honest in-place conditional overwrite exists on a plain filesystem); the linear
READ was the artifact. Generations are a contiguous prefix, so the head is found
in O(log n) reads, and the typed bound now sits at the Int range (2^62).

Witnesses: 4200 sequential admissions still admit; lost race refused (409).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* CAS file store: delete cas_probe_from_exact; witness and failure-mode row cite the classifier production routes through

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* Interleaving witness: an append landing mid-search yields a stale real head, and the stale writer is refused by the exclusive create

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* CAS generation line: total successor with typed exhaustion, overflow-free head search, linearizability stated

- std.durable_compare_and_set: cas_generation_next (unconditional +1) replaced by the total
  cas_generation_successor; CasStoreFailure gains CasGenerationSpaceExhausted { head } for a head at
  the Int maximum. cas_decide_readable, the file store and the fabric CAS storage refuse with it.
- gunbc.durable_cas_file_store: the gallop probes the Int maximum instead of doubling past half of it,
  so the search is total over the generation line; ProbedBeyondBound and its helpers are deleted.
  The contract is stated as linearizability with its proof, and history integrity is named as a
  separate concern the head read no longer provides.
- Consumers matching CasStoreFailure (fabric wire encode/decode, fabric storage fault text,
  capture chunk, fabric live probe) carry the new arm.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* CAS: one authority for the exhausted outcome (cas_generation_exhausted), replacing three spellings (review 70320)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* Interleaving witness observes its search boundary by real reads before the append

Generation 4 is read present and generation 6 read absent through cas_read_generation BEFORE the
concurrent append; the bisection resumes from those observed facts rather than a caller-supplied
absent bound. Moving the append before the observations reds the claim.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* Enrol the counter's five wet claims and the gallop wet claim on the local-repo wet lane

Each real-execution identity gets the triple the lane requires: a FloorRouteGapExpectation
(DirWithTemplate / NoMockResponse, its first effect), a WetScheduledClaim in local_repo_wet_schedule,
and -- for approval_assertion_counter_wet_witness_test.dag, which had none -- a LocalRepoWetLane
witness_exclusion_frontier row. durable_cas_file_store_wet_witness_test.dag already had its file row.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* The past-4096 claim supplies its history instead of re-running 4200 admissions

Its subject is head discovery and the next admission past generation 4096 (one interface, DESIGN
section 3). 4200 contiguous generations are seeded through cas_file_slot_path with the bytes an
admission writes, then one real admission is asserted. ~22 s -> 3.8 s wall; restoring a 4096 bound in
the gallop reds it. The admission path itself stays executed for real by the other claims in the file.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* floor_route_gap: describe the counter's past-4096 claim as a seeded prefix plus real reads

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: gunbc-ci-auto-heal <gunbc-ci-auto-heal@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…aled; key-id is one paired claim

Review 5291201877:
- Wall 1: every fixture of test.claim.approval_device_redemption_witness_test that returns trusted
  evidence or an admission built from it (fx_attestation, assertion_over, verdict,
  fx_admitted_enrollment, good_signature, good_assertion, admit) admits only the declarations of that
  module that use it, derived from its call sites; laundering probes import fx_attestation, verdict
  and assertion_over from an outside module and are refused at the fixture boundary.
- Wall 2: ca_authority_refusal, validation_category_refusal and bundle_version_refusal admit only
  attestation_step_refusal; a probe calling ca_authority_refusal directly is refused; the x509 CA
  claim reaches the step through ordered_step_refusal.
- Wall 3: the key-id step is one claim pairing the real key's id (admits) with another key id
  (refuses), and it and the rp_id assertion claim are floor claims again (the #11981 pattern).
  Admissions and drop membership for them are pending: both measure in the 302-500 ms band.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ty; the two App Attest claims execute

Side-chat ruling on #11989 (PR comment 5795659115), after gunbc.rung_drop
long_home_enrolment_margin_observed_only:
- v2.workflow.floor_enrolment_dead_band names EXACTLY the_key_id_step_admits_the_real_key_and_reds_on_another_key_id
  and the_sample_assertion_passes_rp_id_and_refuses_on_absent_extensions, separate from typed cost debt.
- v2.workflow.floor_enrolment_margin gains the EnrolmentExpensivenessDeadBand ground: non-blocking only for an
  exact reading with margin < cpu <= line; at or under the margin EnrolmentDeadBandStale, above the line
  EnrolmentDeadBandWrongGround, both blocking; bounds, censored and absent readings keep their blocking arms.
  Mirrored in required_floor_runner.rs (decode, declaration, standing, names, causes, detail) with a unit test.
- Model tests cover both self-staling arms, the band's two edges, bound/censored/absent, ground selection and
  the authority being exactly the two.
- gunbc.rung_drop app_attest_verifier_enrolment_dead_band_observed_only declares the drop (previous: mechanically
  preventable; temporary: mitigatable, observed-only, NOT not-executed) with a capability trigger.
- gunbc.recurring_failure_mode enrolment_dead_band_has_no_representable_standing files the general class.
- The verifier eval-step drop returns to six members; the native frontier no longer names the two claims.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…he mirror's arms to the model

gunbc.enrolment_dead_band_seed_growth enumerates the two added hand-Rust items
(floor_enrolment_dead_band_observed_identities; the seed unit test for both self-staling arms), names the
modified ones, and states the v1 purpose test: the required floor guards the v2 program, and without the
arm a witness honestly costing in (margin, line] has no standing that holds. The decision stays in
v2.workflow.floor_enrolment_margin; the Rust decodes v2.workflow.floor_enrolment_dead_band and mirrors.
test.claim.enrolment_margin_seed_mirror_witness_test builds every arm of the model's standing coproduct,
derives its wire name and blocking cause from the model, and requires each quoted in
required_floor_runner.rs, so an unmirrored arm or a drifted spelling reds.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Measured: renaming the mirror's "dead_band_stale" left the whole-file lens green on the seed unit
test's spelling. The lens now reads only impl EnrolmentMarginStanding up to fn detail; with that
scope the same rename reds.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
# Conflicts:
#	dag/gunbc/seed_growth_admission.dag
#	docs/design-rung-drops.md
…ve it its own

Review 70578: inserting it between the roster test and its /// block had moved that doc onto the
dead-band test. Test-only; no generated projection mirrors this file.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… into s4work

# Conflicts:
#	dag/gunbc/durable_cas_file_store.dag
#	dag/test/claim/durable_cas_file_store_wet_witness_test.dag
#	src/v2/workflow/floor_route_gap.dag
#	src/v2/workflow/local_repo_wet_terminal.dag
Review 70621 repeated two comments that predate #12072 and #12080: the read residual ('the counter is
not stored ... replays inside its window') and PlatformRedemptionProof's 'THE ASSERTION COUNTER IS NOT
STORED'. Reads and the push update are replay-refused by the per-enrolment counter committed before they
run; the redemption is deliberately exempt (challenge plus one-decision slot). Both comments now point
at gunbc.auth.approval_assertion_counter and approval_device_routes admit_device_read instead of
restating them.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…anding

The required floor's parse refuses a source annotation inside a declaration body (5 errors at
src/v2/workflow/floor_enrolment_margin.dag 312-316); claim_batch and gunbc run admit it, so the local
runs did not see it (gunbc.recurring_failure_mode local_acceptance_wider_than_the_acceptance_path).
Comment moved to module-item grain; no in-body annotation remains in this PR's .dag diff.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…s Absent in pattern position

Latent on main since #10929, surfaced by the required floor on #11989: in EXPRESSION position the bare
'Absent' resolved to Optional's arm, so deduce_os_install_actuated_satisfied(verdict: Absent) did not
type ('declared ObservationVerdict, produced Optional'), and the '== Absent' / '!= Absent' comparisons
were against the wrong type. Each now matches on the produced verdict, where the pattern is typed as
std.upsert_decision ObservationVerdict. Same claims: pre-install is Absent and not converged; an
unreadable lease is UnknownRefused, not Absent.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… through the attempt-store refusal

The floor refused #12000 at dff52b3 with a non-exhaustive match at
gunbc.compute.attempt_lifecycle: #12080 added the generation-exhaustion arm to
std.durable_compare_and_set CasStoreFailure, and this consumer was not updated.
It now renders a typed refusal detail naming the exhausted head.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…/ SignatureUndecodable, not Malformed at a fabricated size 0

decoded_or_empty mapped a non-base64 key or signature to [], so verify_signature reported
VerifyingKeyMalformed/SignatureMalformed { declared: 0 } -- a size never observed (review 70731,
DESIGN section 5). Deleted it; verify_signature now refuses with a typed arm naming the carrier.
Consumers of SignatureVerification all match with wildcards. Claims: non-base64 key and signature
each reach their arm, and "==" (decodes to zero octets) stays VerifyingKeyMalformed at declared 0.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
# Conflicts:
#	docs/design-rung-drops.md
Ledger-Repair-Judged: docs/design-rung-drops.md
Ledger-Rows-Repaired: docs/design-rung-drops.md microvm_shakedown_slot_row_above_fleet_row
Ledger-Rows-Repaired: docs/design-rung-drops.md network_requirement_unrepresented_after_uses_cut
Ledger-Rows-Repaired: docs/design-rung-drops.md v41_row_store_host_realized_by_sample
Ledger-Rows-Repaired: docs/design-rung-drops.md admit_callers_discarded_on_the_native_route
Heal-Candidate-Run: 35951131764
@briansrls
briansrls marked this pull request as ready for review September 24, 2026 05:47
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codex Review Summary

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

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-24T05:55:59.571507Z 3c73aea Draft marked ready
ℹ️ 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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3c73aeabab

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

// serving process, because the writer gate is consulted inside them; the three reads do not. The
// instant is probed inside each fold, once per request.
fn device_route_response(marker: ApprovalBrokerMarker, process: ApprovalServingProcess, r: DeviceRouteResponse) -> ServeHttpResponse {
broker_text_response(status: r.status, content_type: application_json_utf8, body: r.body, marker: marker, process: process)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve valid JSON in device route responses

Route device responses without passing them through broker_text_response: for the application/json media type, approval_broker_marked_body appends x-approval-broker: ... directly after r.body, producing content such as {"enrollment_id":"..."}x-approval-broker: .... The iOS client parses successful enrolment, pending, fetch, redemption, and readback responses with JSONSerialization, which refuses that trailing non-JSON text, so those newly mounted routes cannot complete successfully once the crypto realization is enabled.

Useful? React with 👍 / 👎.

XCTAssertEqual(ReadHeader.enrollment, try surface("header_enrollment"))
XCTAssertEqual(ReadHeader.requestedAt, try surface("header_requested_at"))
XCTAssertEqual(ReadHeader.assertion, try surface("header_assertion"))
XCTAssertEqual("POST", try surface("method_every_device_operation"))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Replace retired header names in the expected surface set

Update the completeness set alongside this switch to method_every_device_operation. The committed fixture now contains that method key and no header_* keys, but spelled below still expects all three removed headers and omits the method; consequently testSurfaceMatchesTheFixture deterministically fails its final set equality even though each individual route and method assertion succeeds.

Useful? React with 👍 / 👎.

@gunbai-bot

gunbai-bot Bot commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

Duplicate auto-opened on step4-fix when fierce-crane-31 closed. It's #12000's head (3c73aea); #11989 (queued) then #12000 are the landing vehicles. — sent from eager-owl-205

@gunbai-bot gunbai-bot Bot closed this Sep 24, 2026
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.

0 participants