Design: credential and endpoint resolution — robust, flexible, ready-to-go

[thorwhalen]

Splitting this out of #11 because it is the root cause of three of the worst bugs in v0.1.9, and because the obvious fix is migration-unsafe.

What's broken today

Verified against the current release:

1. Explicit endpoint_url is silently dropped when env credentials exist — base.py:82 calls get_client("environment variables", **session_kwargs) without forwarding it. A store configured for R2/MinIO/Supabase talks to AWS.
2. Explicit credentials are overridden by env — base.py:71 does {**session_kwargs, **aws_credentials}, so env wins over what the caller passed.
3. url_for presigns with SigV2 (url_for produces SigV2 presigned URLs, which modern S3 and every major S3-compatible provider reject #10) — because signature_version is never set explicitly.

All three share one cause: credential/endpoint resolution is spread across three functions instead of living in one connection object.

Two facts that make a naive fix dangerous

AWS_ENDPOINT_URL_S3 silently outranks everything. It is service-specific so it beats the generic AWS_ENDPOINT_URL, and it is not in botocore's BOTOCORE_DEFAUT_SESSION_VARIABLES — it is resolved in the endpoint-provider layer, so it is invisible to naive introspection. It is also why CI is currently green on a data-misrouting bug: the test environment re-supplies the endpoint the code throws away.

Consequence nobody would guess: v0's effective precedence when env credentials exist is AWS_ENDPOINT_URL_S3 > AWS_ENDPOINT_URL > explicit kwarg — and "explicit wins" inverts the top of it. Any deployment relying on that env var to override a stale hard-coded endpoint silently redirects on upgrade. Upgrading must not move anyone's data target without telling them.

Credential refresh is easy to destroy. boto3 clients hold a Credentials object and call get_frozen_credentials() per request, so SSO/STS/IMDS refresh works — if you let the chain resolve. v0 defeats this twice: by merging env keys into session kwargs, and by building a raw client from a credential snapshot that can never refresh (and which drops profile_name entirely).

The part I think is uncontroversial

Make resolution a pure function of (spec, environ, aws_config) returning a record where every field carries its provenance:

resolve(spec, env) -> Resolution(
    endpoint_url      = ('https://…',  source='preset:r2'),
    region_name       = ('auto',       source='preset:r2'),
    signature_version = ('s3v4',       source='default'),
    credentials       = (<spec>,       source='profile:prod'),
)

No I/O, so tier-1 tests can cover the ladder exhaustively with a fake env dict — which is what makes a precedence rule trustworthy rather than aspirational. s3dol.diagnose() prints this table (never the secret). The connection stays a frozen dataclass carrying a spec, with the client as a lazy cached_property, so it is picklable (today stores raise PicklingError, and botocore.UNSIGNED is itself unpicklable so it must never enter the dataclass).

The four questions

Q1 — what wins?

• (a) Explicit kwargs always win, full stop. Clean; silently redirects anyone relying on AWS_ENDPOINT_URL_S3.
• (b) Explicit wins, but warn once when an env var that would have won under v0 is present and differs. Migration-safe; one release of noise.
• (c) A strict=True mode that raises on any ambiguity, off by default.

Lean: (b) as default + (c) available, warning removed in v2.

Q2 — what shape is credentials=?

Options: raw kwargs (v0's mistake — defeats refresh); a profile name; a botocore.Session; a callable credential provider returning {key, secret, token, expires_at} (obstore's shape, and the one that composes with SSO); or polymorphic "the thing or a spec for it", à la azuredol.AzureConnection.from_anything.

Lean: polymorphic accept, normalize to a session/provider internally, never to frozen keys. The rule to encode: never construct a client from a credential snapshot.

Q3 — how much does a preset get to set?

• Should a preset be allowed to carry credentials? Proposed no — presets are public, shareable, committable config.
• Should AWS_ENDPOINT_URL_S3 outrank a preset the user explicitly named? Proposed no — an explicitly named preset is an explicit argument.

Q4 — where does anon live, and what does 'auto' mean?

It must be on the top-level factory (s3_store(bucket, anon=True)), not only on the connection, or the open-data notebook case requires learning Layer A — and reading a public bucket is currently impossible in s3dol at all.

'auto' needs a precise meaning. Proposed: "try unsigned if no credentials resolve at all" — explicitly not "retry unsigned after AccessDenied", because an expired token would then silently downgrade to a different, public view of the data. Note requester-pays buckets forbid anonymous access outright, so a silent fallback confuses there regardless.

Also: url_for must raise when the resolved signer is UNSIGNED — botocore otherwise returns a plain unsigned URL with no error.

Sequencing (already in ADR-0007 §5, repeated because it's the risky part)

Because merging auto-publishes to PyPI and version numbers burn permanently:

0. Ship s3dol.diagnose() in a non-breaking 0.1.x patch. Have each dependent run it in their real environment and record the answer.
1. Only then flip the resolution order — and ship the endpoint/credential fix separately from the naming/API change, so a dependent whose data target moves can bisect it to one change.

Design context: ADR-0003, ADR-0007 §4–5, and state-of-play §8.

[thorwhalen]

Resolved — ADR-0012, merged in #18

Three of this discussion's premises did not survive measurement. All checked on botocore
1.43.68, offline, isolated from ~/.aws.

1. "The obvious fix inverts v0's precedence" — no

botocore already ranks explicit > AWS_ENDPOINT_URL_S3 > AWS_ENDPOINT_URL > config file > resolver. v0 behaves otherwise because it drops the argument:
_find_default_credentials(endpoint_url=…) calls get_client("environment variables", **session_kwargs) and never forwards it.

v1 re-ranks nothing; it stops discarding an argument. That deletes the entire "own a
precedence ladder" work item — and with it a conformance test that would have guarded a botocore
class documented as "private and subject to abrupt breaking changes". Q1 was largely a
non-question.

2. The SigV2 scope is wrong in #10 and in ADR-0003 §4

Region-gated, not universal — 12 v2-capable region strings in endpoints.json — and not
caused by a custom endpoint: us-east-1 + MinIO → SigV2, eu-south-2 + MinIO → SigV4. Region
is the discriminator. (I got this wrong once in the other direction first and corrected it on
#10; the corrected measurement is what ADR-0012 carries.)

3. Config(ignore_configured_endpoint_urls=True) must never be passed

A no-op when an explicit endpoint is set; when one is not, it retargets an
AWS_ENDPOINT_URL_S3 user to https://s3.amazonaws.com. Two independent drafts of this design
passed it unconditionally — exactly the silent data-target move the whole exercise exists to
prevent.

The answers

Q1 — what wins. Explicit, because botocore already says so. The migration warning is a
differential through v1's own resolver (v0_mangle → resolve twice), not a frozen
simulation of v0 — imprecision cancels and the value-gap false positives disappear. No
strict= kwarg: filterwarnings('error', category=S3DolWarning) is strict mode with zero API.

Q2 — the shape of credentials=. Polymorphic accept, normalised to a picklable
CredentialProvider object, injected by replacing botocore's credential_provider
component. #15's "a botocore.Session" is impossible — measured, nothing live pickles: not
boto3.Session, not botocore.Session, not clients, not refreshable credentials, not the real
chain's providers. A frozen dataclass holding only a profile name round-trips and rebuilds
live, still-refreshable credentials.

Q3 — what a preset may set. No credential slot, and the exclusion is structural rather
than a check: proxies/proxies_config/client_context_params are off the allowlist and values
are scalars-only. signature_version and client_kwargs leave Preset for the connection.
addressing_style gets no default so every row must state it. pinned is derived and gated on
verified, making it a ratchet that turns on with evidence.

Q4 — anon. 'auto' is deferred to v1.x; v1 ships anon: bool. The reason is the shape
of the evidence: the guard count went 4 → 6 → 7 across three review rounds, each finding
another silent-credential-downgrade path. 'auto' is the only path where s3dol could silently
read the public view of data the caller believed was authenticated. Widening later is fully
compatible, and anon=bool(os.getenv(...)) serves the honest case today.
Also: url_for returns None when anonymous — amending ADR-0003 §5, because raising turns
lacing's documented streaming fallback into a 500 on a public bucket.

Q0 (the one this discussion didn't have) — preset vs env. Partly already settled: ADR-0007 §4
documents explicit > preset > AWS_ENDPOINT_URL_S3. A preset is the caller's explicit
endpoint spelled by name. The guard is a two-tier host check: matching no pattern warns
(a proxy, a CNAME and a gateway all look like that); matching a different provider's pattern
raises.

Other measured facts now in the ADR

• profile_name + any explicit credential kwarg silently discards the profile's
  credentials. A token-only kwarg leaves Credentials(None, None, 'TOK').
• Reading .access_key on a refreshable credential triggers a network refresh — it crashed
  one probe with a connect timeout to STS. diagnose() must never touch it.
• repr() of a botocore provider chain dumps the entire os.environ. A str subclass is
  not sufficient redaction — it leaks through json, concatenation and join.
• botocore does not read AWS_REGION, only AWS_DEFAULT_REGION.
• Omitting a Config key ≠ passing None: Config(s3={'addressing_style': None}) overrides
  a profile's value; omitting preserves it.
• Anonymous HEAD under moto returns 403 on a present key and 404 on an absent one, so every
  key reads as absent — anon + deny_means_absent is banned.
• v0 also nulls an explicitly-passed aws_session_token and changes an explicitly-passed
  region_name — two behaviour changes neither this discussion nor ADR-0007 recorded.

Method, and what it cost

probe → design → refute → repair → integrate. All five first-round designs were refuted with
kill-shot findings, and the integration pass found and resolved 19 cross-question
contradictions (the five answers share one dataclass and one ladder). Four genuine tradeoffs
were escalated and decided: detection scope, host-mismatch loudness, 'auto', and the live-client
escape hatch.

Next

Both open design questions are now closed; implementation is unblocked. First step is unchanged
and deliberately boring — ADR-0007 §5 step 0: ship s3dol.diagnose() in a non-breaking 0.1.x
patch and have lacing, reelee and http_cosmo_prep run it in their real environments
before anything about resolution changes.