Skip to content

[Detail Bug] Access token issuance can lose the one-shot secret due to unsafe automatic retries #133

Description

@detail-app

Detail Bug Report

https://app.detail.dev/org_89d327b3-b883-4365-b6a3-46b6701342a9/bugs/bug_35a7bcb4-0a3f-41e5-b06e-fc0e3125a544

Introduced in #1 by @quettabit on Apr 7, 2026

Summary

  • Context: issue_access_token is the account-level operation that mints a new access token and returns its server-generated secret string to the caller. That secret is delivered only in the 201 response body and is never retrievable again from any endpoint.
  • Bug: issue_access_token is routed through the general request retrier (self._retrier, gated by the method-agnostic http_retry_on) even though POST /access-tokens has no idempotency/replay mechanism. On a transient failure that occurs after the server has committed the token but before the client receives the 201, the retrier re-POSTs the same id; the server reports the duplicate id as a 409, and the originally-issued secret — which lives only in the lost 201 body — is permanently lost.
  • Actual vs. expected: The caller ends up with a token they cannot use (no secret) and cannot read (no endpoint returns it), surfaced as a 409 error that reads as "the id was already taken before my call" when in fact their own first, successful call created the token. The operation should not be retried, or should be retried only when the error proves no server-side mutation could have occurred.
  • Impact: Over any network exhibiting a transient failure in the post-commit/pre-receipt window, a user issuing an access token is left with an orphaned, valid-but-unusable token whose secret is lost. The 409 error the caller sees does not honestly convey "your call may have succeeded." Recovery requires manual cleanup: list_access_tokens to confirm the orphaned id, revoke_access_token(id), and re-issue under a brand-new id.

Code with Bug

In src/s2_sdk/_ops.py, issue_access_token is retried like any other request but has no idempotency token:

@fallible
async def issue_access_token(
    self,
    id: str,
    *,
    scope: types.AccessTokenScope,
    expires_at: datetime | None = None,
    auto_prefix_streams: bool = False,
) -> str:
    ...
    validate_access_token_id(id)
    json = access_token_info_to_json(id, scope, auto_prefix_streams, expires_at)
    response = await self._retrier(                       # <-- BUG 🔴 retries a non-idempotent POST with no idempotency token
        self._account_client.unary_request,
        "POST",
        "/v1/access-tokens",
        json=json,
    )
    return response.json()["access_token"]

The generic retry predicate retries on any TransportError and on multiple 4xx/5xx statuses, regardless of method or endpoint:

def http_retry_on(e: Exception) -> bool:
    if isinstance(e, S2ServerError):
        if e.status_code in (408, 429, 500, 502, 503, 504):
            return True
        if e.status_code == 409 and e.code == "transaction_conflict":
            return True
    if isinstance(e, TransportError):
        return True
    return False

Explanation

  • POST /access-tokens returns the token secret only once (in the 201 response body). GET /access-tokens returns only metadata (id, scope, expires_at, auto_prefix_streams); there is no endpoint that can retrieve the secret later.
  • Because issue_access_token is executed under the general retrier, a transient failure after the server commits but before the client receives the 201 triggers a duplicate POST with the same id. The server responds with 409 for the duplicate, and the SDK surfaces that error—leaving behind an access token that exists server-side but whose secret was lost with the original 201.
  • The SDK cannot distinguish “server committed but response lost” from “server never committed,” but it retries unconditionally. This makes the irreversible case (lost one-shot secret) possible.

Codebase Inconsistency

Other create-POST operations in the same file add s2-request-token to make retries safe via server-side dedup, but issue_access_token does not:

response = await self._retrier(
    self._account_client.unary_request,
    "POST",
    "/v1/basins",
    json=json,
    headers={"s2-request-token": _s2_request_token()},   # <-- makes retries safe
)
response = await self._retrier(
    self._client.unary_request,
    "POST",
    "/v1/streams",
    json=json,
    headers={"s2-request-token": _s2_request_token()},   # <-- makes retries safe
)

Recommended Fix

Route issue_access_token through a retrier that only retries when the error indicates no side effects could have occurred (reuse existing has_no_side_effects in src/s2_sdk/_retrier.py), instead of the general http_retry_on retrier.

History

This bug was introduced in commit 3dc9795. PR #1 ("feat: add initial version of s2-sdk") landed the first complete SDK, and within that single change the author carefully equipped create_basin/create_stream with headers={"s2-request-token": _s2_request_token()} for retry-safety but routed issue_access_token through the same general self._retrier without any idempotency token. The bug has remained unchanged since that first commit; commit c18d589 only added validate_access_token_id(id) and did not affect retry behavior.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

detail-bugbug flagged by https://detail.dev/

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions