Concurrent writes to a remote-signing catalog go out unsigned and fail 403

Open
#3,896 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
1/5
Estimated time
Under an hour
Newbie friendliness
35/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Stale
Tech stack
python
Domain
cloud

Research direction

Start in pyiceberg/io/fsspec.py at _s3(), then run the two named tests in tests/io/test_fsspec.py. Done means the signer remains installed while shared-client reconfiguration occurs and both concurrency regression tests pass; PR #3783 already covers the reported fix.

Written by the indexing model from the issue text.

Description

Apache Iceberg version

main (development) — also reproduced on 0.12.0 and 0.11.1.

Please describe the bug 🐞

_s3() unregisters the S3 request signer and re-registers it on an event emitter that fsspec caches and every thread shares. A request signed in that window goes out with no Authorization header_s3() has already set config_kwargs["signature_version"] = UNSIGNED, so botocore does not sign in its place — and the store answers 403 AccessDenied.

pyiceberg/io/fsspec.py, in _s3():

fs = S3FileSystem(**s3_fs_kwargs)

for event_name, event_function in register_events.items():
    fs.s3.meta.events.unregister(event_name, unique_id=1925)   # opens the window
    fs.s3.meta.events.register_last(event_name, event_function, unique_id=1925)

FsspecFileIO.get_fs caches per thread and every table gets its own FileIO, so a short workload makes hundreds of these cycles against the one shared emitter. PyIceberg's writer is concurrent by default, so no unusual usage is needed to reach it.

Roughly 3–4% of appends fail against a remote-signing catalog, surfacing as an opaque PermissionError: Access Denied out of s3fs, several frames from its cause. Every unsigned request caught on the wire was a PUT of a manifest during commit.

Measured on Lakekeeper 0.13.1 + MinIO (path-style), 90 writes per run:

configuration append failures unsigned on wire
as shipped 2 / 4 / 5 2 / 4 / 5
signer registered once (client still shared) 0 0
PYICEBERG_MAX_WORKERS=1 0 0

The second row is the one that matters: the failures stop while the client is still shared, which separates this from a general concurrency problem. The response code is always AccessDenied and never SignatureDoesNotMatch — what an unsigned request produces, not a mis-signed one.

Suggested fix

Drop the unregister. Botocore's HierarchicalEmitter._register_section returns early for a unique_id it already holds, so re-registering an equivalent signer was already a no-op; the unregister only opens the window.

Reproduction

Two tests in tests/io/test_fsspec.py, no credentials and no network:

  • test_s3_leaves_a_signer_installed_while_reconfiguring_a_shared_client — drives _s3() and observes the emitter the instant it unregisters.
  • test_the_signer_stays_installed_while_another_thread_reconfigures_s3 — the same window seen from another thread. The window is two adjacent statements, so sampling for it blind is a coin flip (20,000 observations caught it zero times); it is held open by delaying only the re-registration, so the timing is deterministic while the defect is not manufactured.

Both fail on main and pass with the fix.

Already covered by an open PR

#3783 (for #3625) removes this unregister as part of registering the signer on an AioSession, for a different reason — a lazily created client not inheriting handlers, failing InvalidRequest. I applied that PR and ran both tests above against it: both pass, so it fixes this as well.

Worth recording rather than closing silently, because the line is wrong for two independent reasons and #3783's own test does not cover this one. If #3783 lands, these two tests are the regression cover for the concurrency window; happy to raise them against that PR instead if a maintainer prefers.

A second, independent defect in the same file — noted, not proposed here

pyiceberg/io/fsspec.py:160 applies the signing service's headers with add_header, which appends, so every header the service echoes back appears twice — 880 of 1615 sign calls ended with a duplicate of a header named in SignedHeaders. On MinIO this is latent: de-duplicating changed nothing, and it did not contribute to the 403s above. SigV4 combines repeated headers comma-separated, so a stricter store may reject them. Mentioned so it is not lost, not to widen this issue.

Willingness to contribute

I can contribute a fix to resolve this bug independently.

Dominant language
Python
Stars
1.1k
Forks
589
Avg merge
2d 4h
Merged PRs (30d)
72

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from apache/iceberg-python

All issues in apache/iceberg-python

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.