Add credentials refresh in more locations

[J-Meyers]

This is very difficult to test since you must wait for the credentials to timeout, however there were a variety of locations and scenarios that previously did not automatically refresh the credentials.

After the first S3FileHandle was Initialized then many operations like GLOBs would never have an opportunity to refresh the handle.

This adds in autorefresh around all calls, the core of this is the ExecuteWithRefresh and keeping around the FileOpener. ExecuteWithRefresh is used to just wrap the old code exactly as it was with a mechanism to automatically refresh the credentials and then run it again.

The previous refreshing in S3FileHandle::Initialize is now removed since it is now managed at each of the lower level function calls.

Should close:
duckdb/duckdb-aws#26
duckdb/duckdb-aws#64
duckdb/duckdb-aws#97

If you have any advice on how to create an automated test for this I'd appreciate it

[carlopi]

@J-Meyers: Thanks for the PR, just an heads up that it might take a bit for this to get properly through review process, but we'll get to it.

Testing is indeed not trivial, unsure what's the way forward, but I'll think of something

[szalai1]

When can we expect this to be merged? How can I help to test this? @carlopi

[mst]

This also bites DuckLake's ability to auto-refresh. The chain:

1. DuckLake passes file_size + dummy etag and last_modified in extended_info: https://github.com/duckdb/ducklake/blob/c80b9155c8ff260778b6a5bc726e50d65edc84dc/src/storage/ducklake_multi_file_list.cpp#L195-L198

   		extended_info->options["validate_external_file_cache"] = Value::BOOLEAN(false);
   		// etag / last modified time can be set to dummy values
   		extended_info->options["etag"] = Value("");
   		extended_info->options["last_modified"] = Value::TIMESTAMP(timestamp_t(0));

   this seems to be on purpose to avoid unnecessary HEAD requests (https://github.com/duckdb/ducklake/blob/c80b9155c8ff260778b6a5bc726e50d65edc84dc/src/storage/ducklake_delete_filter.cpp#L23)

2. this makes HTTPFileHandle ctor flip initialized = true ....
   

   duckdb-httpfs/src/httpfs.cpp

   Lines 400 to 404 in 13e18b3

   	if (lm_entry != info.end() && etag_entry != info.end() && fs_entry != info.end()) {
   	// we found all relevant entries (last_modified, etag and file size)
   	// skip head request
   	initialized = true;
   	}

3. ...and makes L829 early-return from LoadFileInfo()

   duckdb-httpfs/src/httpfs.cpp

   Lines 827 to 840 in 13e18b3

   	if (initialized || force_full_download) {
   	// already initialized or we specifically do not want to perform a head request and just run a direct download
   	return;
   	}
   	// In write_overwrite_mode we dgaf about the size, so no head request is needed
   	if (write_overwrite_mode) {
   	length = 0;
   	initialized = true;
   	return;
   	}
   	auto &hfs = file_system.Cast<HTTPFileSystem>();
   	auto res = hfs.HeadRequest(*this, path, {});

Why this matters: today's only auto-refresh hook lives in S3FileHandle::Initialize's catch block -- it fires when the HEAD returns 403. By design, DuckLake (and Iceberg) skip the HEAD via prefilled extended_info to avoid an extra round-trip per file. That skip silently opts the whole read out of auto-refresh: Initialize() completes against stale creds, and the first auth check happens in a range read, which has no equivalent hook. STS expiry mid-session then propagates raw ExpiredToken to the user, even with refresh='auto' configured on the secret.

Reproduction

INSTALL postgres; LOAD postgres;
INSTALL ducklake; LOAD ducklake;
FORCE INSTALL aws FROM core_nightly; LOAD aws;
INSTALL httpfs; LOAD httpfs;

CREATE SECRET aws_s3 (
    TYPE S3, PROVIDER credential_chain, CHAIN 'web_identity',
    ASSUME_ROLE_ARN '<role>',
    WEB_IDENTITY_TOKEN_FILE '<jwt-file>',
    REGION 'eu-west-1'
);
ATTACH 'ducklake:postgres:host=<pg> port=5432 dbname=<db>' AS x;

Step	Action	Result	secret key_id
1	SELECT * FROM x.main.t1 LIMIT 1	OK	initial
2	time.sleep(65 * 60) (past STS expiry)	--	initial (unchanged)
3	SELECT * FROM x.main.t2 LIMIT 1	FAIL ExpiredToken	initial
4	SELECT * FROM read_parquet('s3://...')	OK; fires existing Initialize refresh	NEW
5	SELECT * FROM x.main.t3 LIMIT 1	OK	NEW (from step 4)

---

EDIT: same pattern on duckdb-iceberg: e.g. https://github.com/duckdb/duckdb-iceberg/blob/b0d93913629a08b91af90da9ae7e89d846cd22ec/src/planning/iceberg_multi_file_list.cpp#L1127-L1132.
EDIT 2: add more explanation why early-return from LoadFileInfo() is a problem

[mst]

refresh_get_range_test.sh

This is a small shell-driver integration test that exercises the GetRangeRequest path specifically -- i.e. the gap DuckLake / Iceberg hit via prefilled extended_info. Reproduces without either of those extensions by using enable_http_metadata_cache=true to trigger the same HEAD-skip on the second read.

How the test works: Two service accounts under one parent user. Phase A reads with svcacct A (populates http_metadata_cache), then mc admin user svcacct rm revokes it. Phase B's first I/O is a range GET (HEAD skipped) signed with the now-revoked token. With this PR the wrapper catches the 403 and refreshes via REFRESH_INFO to svcacct B; without it, the 403 propagates.

How to run

# 1. minio -- same image pinned in `scripts/minio_s3.yml`
#    (what `scripts/run_s3_test_server.sh` boots in CI via docker compose).
docker run -d --name duckdb-minio -p 9000:9000 \
    -e MINIO_ROOT_USER=duckdb_minio_admin \
    -e MINIO_ROOT_PASSWORD=duckdb_minio_admin_password \
    minio/minio:RELEASE.2021-11-03T03-36-36Z server /data
until curl -sf http://localhost:9000/minio/health/live >/dev/null; do sleep 1; done

MC="docker run --rm --network=host \
    -e MC_HOST_local=http://duckdb_minio_admin:duckdb_minio_admin_password@localhost:9000 \
    minio/mc:RELEASE.2021-11-05T10-05-06Z"
$MC admin user add local minio_duckdb_user minio_duckdb_user_password
$MC admin policy set local readwrite user=minio_duckdb_user
$MC mb local/test-bucket

# 2. build + run the test
make release
ENDPOINT=localhost:9000 URL_STYLE=path \
  DUCKDB=./build/release/duckdb \
  ./scripts/refresh_get_range_test.sh

I hope that helps moving this PR forward.

[mst]

thread safety: n threads expiring simultaneously will probably each issue their own STS call. This might require a mutex around the Credentials Refresh call and some way to detect that a refresh already happened.

[mst]

this might cause a data race -- a reading thread accessing e.g. the updated access_key_id before the secret_access_key is being written by the refreshing thread.

Might not be a big functional problem if there's the mutex around the refresh: either the reader runs with an expired token into the refresh mutex, or it does with a invalid, partially updated set of credentials and runs into the refresh mutex (assuming the exception handling for expired tokens == invalid signature).

Not sure if this is a problem, but could be solved by swapping all credential attributes atomically.

[mst]

It looks like HTTPFileSystem::PostRequest doesn't throw on 4xx [1] (and neither does Put/Delete/Head [2] [3]), so this also needs a check on 4xx. Only GetRequest does [4], and that's why the test I posted succeded...

[1] https://github.com/duckdb/duckdb-httpfs/blob/main/src/httpfs.cpp#L164
[2] https://github.com/duckdb/duckdb-httpfs/blob/main/src/httpfs.cpp#L167
[3] https://github.com/duckdb/duckdb-httpfs/blob/main/src/httpfs.cpp#L193
[4] https://github.com/duckdb/duckdb-httpfs/blob/main/src/httpfs.cpp#L242