amadan
Pin the fact that RequireFresh is not the same as a real assertion
passkey.SignInRecovery mints its session with AuthTime: time.Now(), so redeeming a recovery code produces a session that satisfies RequireFresh at any duration. That is right where it is used today — somebody who just proved possession of a recovery code should be able to replace the set — but §5.2 says recovery is "sign-in only — step-up still demands a real assertion", and RequireFresh does not tell the two apart. This matters one step out, not here: §14's owner write is specified to sit behind step-up. Built on RequireFresh it would accept a recovery code instead, and nothing else in the tree would notice. Recorded on codesFreshness and asserted by a test, so the fact cannot change silently and whoever builds that write finds it before they need it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 files changed,
+63
−0
internal/home/passkey.go+13 −0internal/home/passkey_test.go+50 −0
Intent
Human
<task-notification> <task-id>a7c5da444aa32f80b</task-id> <tool-use-id>toolu_01VHUjHb76QrLpe9PWu7YVda</tool-use-id> <output-file>/home/paulca/.cache/tmp/claude-1001/-home-paulca-amadan-net-oficina-home/d63f5ac7-6b42-48c5-a13d-34e6faf8e44c/tasks/a7c5da444aa32f80b.output</output-file> <status>failed</status> <summary>Agent "Review tasks 5-7" failed: Agent terminated early due to an API error: You've hit your session limit · resets 12:10pm (Europe/Dublin) (error type rate_limit, HTTP 429, request id req_011CeocH6o6jaMTef511JkV1, model sent to the API: claude-fable-5-1)</summary> <note>A task-notification fires each time this agent stops with no live background children of its own. The user can send it another message and resume it, so the same task-id may notify more than once.</note> <result>The persisted files exceeded the output cap. I'll read each source file separately, in halves where needed. All independent.</result> </task-notification>
Human
continue
diff --git a/internal/home/passkey.go b/internal/home/passkey.go| index 5bb2d66..f0ba80b 100644 |
| --- a/internal/home/passkey.go |
| +++ b/internal/home/passkey.go |
| @@ -25,6 +25,19 @@ const confirmPath = "/passkey/confirm" |
| // Short, because the page shows ten codes once. Long enough to exceed a |
| // passkey ceremony round trip, or enrolling and then being asked for |
| // your codes would bounce straight back to a step-up. |
| +// |
| +// A FACT ABOUT RequireFresh THAT A LATER STEP MUST NOT ASSUME AWAY: |
| +// freshness is not the same as "a real assertion". passkey.SignInRecovery |
| +// mints its session with AuthTime: time.Now(), so redeeming a recovery |
| +// code produces a session that satisfies RequireFresh at any duration. |
| +// That is right here — somebody who just proved possession of a recovery |
| +// code should be able to replace the set — but §5.2 says recovery is |
| +// "sign-in only — step-up still demands a real assertion", and |
| +// RequireFresh alone does not deliver that. §14's owner write is |
| +// specified to sit behind step-up; if it is built on RequireFresh it |
| +// will accept a recovery code instead. Asserted in |
| +// TestRecoveryCodeProducesAFreshSession so the fact cannot change |
| +// silently. |
| const codesFreshness = 5 * time.Minute |
| // passkeyConfig is the config the app builds. |
diff --git a/internal/home/passkey_test.go b/internal/home/passkey_test.go| index 2f719d8..dcd1562 100644 |
| --- a/internal/home/passkey_test.go |
| +++ b/internal/home/passkey_test.go |
| @@ -259,3 +259,53 @@ func TestRecoveryCodesPageNeedsAFreshSession(t *testing.T) { |
| t.Error("a day-old session still reads the recovery codes page") |
| } |
| } |
| + |
| +// A recovery code produces a session that satisfies RequireFresh, |
| +// because passkey.SignInRecovery mints one with AuthTime: time.Now(). |
| +// |
| +// This test does not object to that — replacing the code set right after |
| +// spending one is the sensible behaviour. It pins the fact, because §5.2 |
| +// says recovery is "sign-in only — step-up still demands a real |
| +// assertion" and RequireFresh does not distinguish the two. §14's owner |
| +// write is specified to sit behind step-up; built on RequireFresh it |
| +// would take a recovery code instead, and nothing else in the tree would |
| +// notice. |
| +func TestRecoveryCodeProducesAFreshSession(t *testing.T) { |
| + app := newTestApp(t) |
| + const addr = "person@example.invalid" |
| + |
| + cookie := signInFully(t, app, addr) |
| + enrol(t, app, cookie) |
| + codes, err := app.passkeys.RegenerateRecoveryCodes(subjectOf(t, app, cookie)) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + signIn(t, app, addr) |
| + gated := redeem(t, app, capturedLink(t, app)) |
| + |
| + form := url.Values{"code": {codes[0]}} |
| + req := httptest.NewRequest(http.MethodPost, "/passkey/signin/recovery", strings.NewReader(form.Encode())) |
| + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") |
| + req.Header.Set("Origin", Origin) |
| + for _, c := range (&http.Response{Header: gated.Header()}).Cookies() { |
| + req.AddCookie(c) |
| + } |
| + w := httptest.NewRecorder() |
| + app.Router().ServeHTTP(w, req) |
| + |
| + recovered := sessionCookie(w) |
| + if recovered == nil { |
| + t.Fatalf("the recovery code did not sign anybody in (status %d)", w.Code) |
| + } |
| + |
| + // The RequireFresh-guarded page opens to it. |
| + page := httptest.NewRequest(http.MethodGet, "/passkey/codes", nil) |
| + page.AddCookie(recovered) |
| + pw := httptest.NewRecorder() |
| + app.Router().ServeHTTP(pw, page) |
| + if pw.Code != http.StatusOK { |
| + t.Errorf("a recovery-code session got %d from the RequireFresh page; if this has "+ |
| + "changed, the note on codesFreshness needs revisiting", pw.Code) |
| + } |
| +} |