Skip to content

Commit c08261d

Browse files
alukachclaude
andauthored
fix: bump multistore to 0.6.4 (multipart SignatureDoesNotMatch on keys with =) (#181)
Fixes #180. Multipart uploads to keys containing `=` (Hive-style partition paths like `by_country/country_iso=ETH/ETH.pmtiles`) fail with `SignatureDoesNotMatch`: multistore 0.6.3's `build_backend_url` splices the raw key into the backend URL for raw-signed multipart operations, while AWS/MinIO reconstruct the canonical URI by strict-encoding the decoded path — so the signatures diverge. Fixed upstream in developmentseed/multistore#105, released as 0.6.4. ## History 1. **Commit 1 (test only)** — pinned the expected SigV4 strict encoding locally; CI failed on it against 0.6.3, demonstrating the regression (red). 2. **Commit 2 (dependency bump)** — bumps all multistore crates to 0.6.4; CI green. 3. **Deconflicted with main after #183**, and the local pins were **dropped in favor of the upstream tests**: the 0.6.4 release itself ships seven equivalent-or-stronger tests in the multipart module (including the exact `country_iso=ETH` case, upload-id encoding, and literal-`%3D` double-encoding). Keeping a local copy would test the dependency, not this repo. 4. **Worker-level multipart coverage added** (tests/test_writes.py), layered by what each environment can execute: - anonymous `CreateMultipartUpload` on a hive key → 403 at the gate — hermetic, every run (verified against a local 0.6.4 worker); - authenticated create on the probe's unassumable role → bounded, parseable fail-closed error — runs on same-repo CI (exercises multipart routing through the write-authz + federation seam); - **full round-trip** (create → 5 MiB + 1 KiB parts → complete → read-back → delete) on `by_country/country_iso=ETH/…` — the proxy-level #180 guarantee. Skips until the write-probe infra exists; runs against any provisioned worker via env vars. ## Activating the round-trip (follow-up, tracked with #183's write-probe work) Probe bucket + IAM role, a dedicated CI signing key whose public JWKS is served at an issuer URL the role's OIDC provider trusts (not staging's key), the stub serving the real bucket/role instead of placeholders, and ci.yml exporting `CI_WRITE_PROBE_ROLE_ARN`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Behavior changes shipped by the bump (review-verified against the vendored sources) - **Special-character key addressing changes for the presigned CRUD path.** 0.6.3's `Path::from` percent-encoded `* % ~ #`-class characters into the *stored* object name; 0.6.4's `Path::parse` is byte-faithful. Any object PUT through the proxy since writes shipped (2026-06-23) with such a key was stored under a mangled name and becomes unreachable by its original key after this deploys. Exposure is a ~3-week window of simple presigned PUTs; worth a quick bucket scan for `%`-containing names before/after deploy. - **Degenerate keys now 400 instead of being silently normalized.** 0.6.4 adds `validate_key` on every keyed operation (reads included): leading/trailing slashes (folder-marker probes), empty `//` segments, `.`/`..` segments, and control bytes return 400 `InvalidRequest` where 0.6.3 normalized them (`GET a//b` served `a/b`; `PUT dir/` wrote `dir`). Upstream frames this as deliberate strictness; noting it here since it's invisible in this diff. (A claimed quick-xml 0.37→0.41 serialization drift was refuted by byte-level A/B comparison — output is identical; backend XML parsing goes through object_store's separately-pinned quick-xml, unchanged by this PR.) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent a811d78 commit c08261d

4 files changed

Lines changed: 160 additions & 41 deletions

File tree

Cargo.lock

Lines changed: 17 additions & 17 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,10 @@ path = "tests/fixtures.rs"
3636

3737
[dependencies]
3838
# Multistore
39-
multistore = { version = "0.6.3", features = ["azure"] }
40-
multistore-oidc-provider = "0.6.3"
41-
multistore-path-mapping = "0.6.3"
42-
multistore-sts = "0.6.3"
39+
multistore = { version = "0.6.4", features = ["azure"] }
40+
multistore-oidc-provider = "0.6.4"
41+
multistore-path-mapping = "0.6.4"
42+
multistore-sts = "0.6.4"
4343

4444
# Serialization
4545
serde = { version = "1", features = ["derive"] }
@@ -61,7 +61,7 @@ tracing = "0.1"
6161
# Pulled in transitively via object_store -> rand. getrandom doesn't support
6262
# wasm32-unknown-unknown unless the `wasm_js` backend is enabled, so opt in here.
6363
getrandom = { version = "0.4", features = ["wasm_js"] }
64-
multistore-cf-workers = { version = "0.6.3", features = ["azure"] }
64+
multistore-cf-workers = { version = "0.6.4", features = ["azure"] }
6565
# On wasm32-unknown-unknown reqwest selects its browser `fetch` backend by
6666
# target arch (not a cargo feature), so the default TLS/backend features are
6767
# unnecessary here. We only use `Client::new()`, `.send()` and `.text()`, none

tests/stub_api.py

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
"""
2323

2424
import json
25+
import os
2526
from http.server import BaseHTTPRequestHandler, HTTPServer
2627
from pathlib import Path
2728

@@ -57,6 +58,17 @@ def _fixture(name):
5758

5859
WRITE_PRODUCT_JSON = _fixture("product_write_probe")
5960

61+
WRITE_CONNECTION_JSON = _fixture("data_connection_write_probe")
62+
# Activation plumbing: the same CI_WRITE_PROBE_* variables that un-skip
63+
# test_multipart_roundtrip_hive_partition replace the fixture placeholders
64+
# here, so the test gate and the served connection can't drift apart.
65+
_details = WRITE_CONNECTION_JSON["details"]
66+
_details["bucket"] = os.environ.get("CI_WRITE_PROBE_BUCKET", _details["bucket"])
67+
_details["region"] = os.environ.get("CI_WRITE_PROBE_REGION", _details["region"])
68+
WRITE_CONNECTION_JSON["authentication"]["role_arn"] = os.environ.get(
69+
"CI_WRITE_PROBE_ROLE_ARN", WRITE_CONNECTION_JSON["authentication"]["role_arn"]
70+
)
71+
6072
# ── Failure probes ─────────────────────────────────────────────────
6173
# Synthetic products that make the control plane misbehave on purpose, so the
6274
# proxy's fail-closed error mapping is exercised in CI (the original incident
@@ -81,9 +93,7 @@ def _fixture(name):
8193
f"/api/v1/products/{WRITE_ACCOUNT}": {"products": [WRITE_PRODUCT_JSON]},
8294
f"/api/v1/products/{WRITE_ACCOUNT}/{WRITE_PRODUCT}": WRITE_PRODUCT_JSON,
8395
f"/api/v1/products/{WRITE_ACCOUNT}/{WRITE_PRODUCT}/permissions": ["read", "write"],
84-
f"/api/v1/data-connections/{WRITE_CONNECTION}": _fixture(
85-
"data_connection_write_probe"
86-
),
96+
f"/api/v1/data-connections/{WRITE_CONNECTION}": WRITE_CONNECTION_JSON,
8797
}
8898

8999
# Import-time drift guards: routes are keyed on the constants above, bodies

tests/test_writes.py

Lines changed: 125 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@
2525
smoke tests (tests/test_federation.py, wired into staging.yml).
2626
"""
2727

28+
import functools
2829
import os
30+
import uuid
2931
import xml.etree.ElementTree as ET
3032

3133
import pytest
@@ -63,8 +65,13 @@ def sts_exchange(token):
6365
)
6466

6567

68+
@functools.lru_cache(maxsize=1)
6669
def exchange_token():
67-
"""POST /.sts and return (access_key_id, secret_access_key, session_token)."""
70+
"""POST /.sts and return (access_key_id, secret_access_key, session_token).
71+
72+
Cached: one real exchange serves every positive-path test instead of ~8
73+
identical OIDC verifications per run (the negatives call sts_exchange
74+
directly and stay uncached)."""
6875
resp = sts_exchange(ID_TOKEN)
6976
assert resp.status_code == 200, f"/.sts exchange failed ({resp.status_code}): {resp.text[:300]}"
7077
# Match by local name to stay independent of the response's xmlns.
@@ -75,9 +82,10 @@ def exchange_token():
7582
return fields["AccessKeyId"], fields["SecretAccessKey"], fields["SessionToken"]
7683

7784

78-
def s3_client(session_token=None):
85+
def s3_client(session_token=None, **config_kwargs):
7986
"""Credentialed S3 client via /.sts; `session_token` overrides the sealed
80-
token so tests can present corrupted ones."""
87+
token so tests can present corrupted ones. `config_kwargs` merge into the
88+
botocore Config (e.g. retries)."""
8189
import boto3 # deferred: only the credentialed tests need it
8290
from botocore.config import Config
8391

@@ -89,16 +97,102 @@ def s3_client(session_token=None):
8997
aws_secret_access_key=secret_key,
9098
aws_session_token=session_token or real_token,
9199
region_name="us-east-1",
92-
config=Config(s3={"addressing_style": "path"}),
100+
config=Config(s3={"addressing_style": "path"}, **config_kwargs),
93101
)
94102

95103

96-
def test_anonymous_write_to_federated_product_denied():
97-
"""No credentials -> denied at the gate, before any backend/STS call."""
98-
resp = requests.put(f"{PROXY_URL}/{WRITE_ACCOUNT}/{WRITE_PRODUCT}/denied.txt")
104+
# Hive-style partition key (`country_iso=ETH`): the raw-signed multipart path
105+
# where the 0.6.3 encoding bug (#180) lived. Only the round-trip test below
106+
# reaches that encoding seam — the denial and fail-closed layers die at the
107+
# authz gate / federation first; their hive keys just keep the paths uniform.
108+
HIVE_KEY_DIR = "by_country/country_iso=ETH"
109+
110+
111+
@pytest.mark.parametrize(
112+
"method,path",
113+
[
114+
("put", "denied.txt"),
115+
("post", f"{HIVE_KEY_DIR}/denied.pmtiles?uploads"),
116+
],
117+
ids=["put", "multipart-create"],
118+
)
119+
def test_anonymous_write_to_federated_product_denied(method, path):
120+
"""No credentials -> denied at the gate, before any backend/STS call.
121+
Every write op shares the gate; multipart create is one more of them."""
122+
resp = getattr(requests, method)(
123+
f"{PROXY_URL}/{WRITE_ACCOUNT}/{WRITE_PRODUCT}/{path}"
124+
)
99125
assert resp.status_code == 403
100126

101127

128+
needs_probe_infra = pytest.mark.skipif(
129+
not os.environ.get("CI_WRITE_PROBE_ROLE_ARN"),
130+
reason="write-probe AWS infra not configured (set CI_WRITE_PROBE_ROLE_ARN)",
131+
)
132+
133+
134+
@needs_token
135+
@needs_probe_infra
136+
def test_multipart_roundtrip_hive_partition():
137+
"""The proxy-level #180 guarantee: multipart create/upload/complete on a
138+
hive-partitioned key, read back and verified — the only test that reaches
139+
the raw-signed backend-URL encoding seam end-to-end.
140+
141+
Activation (see #183's follow-ups): the probe bucket + IAM role, a worker
142+
signing key whose issuer the role's OIDC provider trusts (a dedicated CI
143+
key — not staging's), and ci.yml exporting the CI_WRITE_PROBE_* vars to
144+
both the stub and pytest. The stub derives its served bucket/role from
145+
the same variables (see stub_api.py), so this gate and the served config
146+
can't drift apart.
147+
"""
148+
client = s3_client()
149+
key = (
150+
f"{WRITE_PRODUCT}/{HIVE_KEY_DIR}/"
151+
f"gh-{os.environ.get('GITHUB_RUN_ID', 'local')}-{uuid.uuid4().hex}.pmtiles"
152+
)
153+
bodies = (b"a" * (5 * 1024 * 1024), b"b" * 1024) # 5 MiB min non-final part
154+
155+
upload_id = client.create_multipart_upload(Bucket=WRITE_ACCOUNT, Key=key)[
156+
"UploadId"
157+
]
158+
try:
159+
parts = []
160+
for n, body in enumerate(bodies, 1):
161+
resp = client.upload_part(
162+
Bucket=WRITE_ACCOUNT, Key=key, UploadId=upload_id,
163+
PartNumber=n, Body=body,
164+
)
165+
# Carry the Checksum* members through. Not required today (no
166+
# ChecksumAlgorithm was declared at create, so S3 validates only
167+
# PartNumber+ETag), but the moment one is declared — e.g. by
168+
# switching to boto3's transfer manager — S3 rejects a Complete
169+
# that omits per-part checksums. Harmless now, load-bearing then.
170+
parts.append(
171+
{"PartNumber": n, "ETag": resp["ETag"]}
172+
| {k: v for k, v in resp.items() if k.startswith("Checksum")}
173+
)
174+
client.complete_multipart_upload(
175+
Bucket=WRITE_ACCOUNT, Key=key, UploadId=upload_id,
176+
MultipartUpload={"Parts": parts},
177+
)
178+
got = client.get_object(Bucket=WRITE_ACCOUNT, Key=key)["Body"].read()
179+
assert got == b"".join(bodies)
180+
finally:
181+
# Best effort, order-independent: delete covers a completed object
182+
# (even one completed server-side after a client-side error), abort
183+
# covers an unfinished upload; the bucket lifecycle rule backstops.
184+
for cleanup in (
185+
lambda: client.delete_object(Bucket=WRITE_ACCOUNT, Key=key),
186+
lambda: client.abort_multipart_upload(
187+
Bucket=WRITE_ACCOUNT, Key=key, UploadId=upload_id
188+
),
189+
):
190+
try:
191+
cleanup()
192+
except Exception:
193+
pass
194+
195+
102196
@needs_token
103197
def test_sts_exchange_issues_credentials():
104198
"""The proxy verifies the GitHub OIDC token (issuer discovery, aud check)
@@ -186,21 +280,36 @@ def test_signed_request_with_special_char_key_verifies():
186280

187281

188282
@needs_token
189-
def test_federated_write_fails_closed():
283+
@pytest.mark.parametrize("op", ["put_object", "create_multipart_upload"])
284+
def test_federated_write_fails_closed(op):
190285
"""The write probe's role is unassumable by design (placeholder ARN,
191286
throwaway signing key): backend STS federation must fail with a bounded,
192287
parseable S3 error — never a hang, never a silent success. This is the
193-
incident class that motivated the stub, kept permanently exercised."""
288+
incident class that motivated the stub, kept permanently exercised.
289+
Federation errors before any backend URL is built, so this pins the
290+
fail-closed seam for every write op; key encoding is the round-trip
291+
test's job."""
194292
import botocore.exceptions
195293

196-
client = s3_client()
294+
# max_attempts=1: botocore's default mode retries the 502/503 this
295+
# produces, multiplying live AWS STS round-trips for no extra signal.
296+
client = s3_client(retries={"max_attempts": 1})
297+
kwargs = {
298+
"Bucket": WRITE_ACCOUNT,
299+
"Key": f"{WRITE_PRODUCT}/{HIVE_KEY_DIR}/fail-closed.pmtiles",
300+
}
301+
if op == "put_object":
302+
kwargs["Body"] = b"x"
197303
with pytest.raises(botocore.exceptions.ClientError) as exc:
198-
client.put_object(
199-
Bucket=WRITE_ACCOUNT, Key=f"{WRITE_PRODUCT}/fail-closed.txt", Body=b"x"
200-
)
201-
# The exact code depends on how AWS STS rejects the assertion; what
202-
# matters is that boto3 could parse an S3-shaped error at all.
203-
assert exc.value.response["Error"].get("Code"), "unparseable error response"
304+
getattr(client, op)(**kwargs)
305+
code = exc.value.response["Error"].get("Code")
306+
assert code, "unparseable error response"
307+
# Federation failure maps only to AccessDenied / BackendAuthenticationFailed
308+
# / ServiceUnavailable / InternalError; the inbound-auth codes would mean
309+
# verification broke before federation was even attempted.
310+
assert code not in {"SignatureDoesNotMatch", "InvalidRequest"}, (
311+
f"{code}: rejected before the federation seam this test pins"
312+
)
204313

205314

206315
@needs_token

0 commit comments

Comments
 (0)