Skip to content

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

Description

@paulcaron16k

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.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions