From b91ff1142768fe1b9b9cbf26d761f9792e5270c2 Mon Sep 17 00:00:00 2001 From: matthiasL-scality Date: Fri, 17 Jul 2026 14:05:24 +0200 Subject: [PATCH] Fix ENDPOINT_URL not exposed to Lua workers in nginx MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit nginx strips env vars from Lua workers unless declared with the 'env' directive. ENDPOINT_URL was missing from nginx.conf.template, so os.getenv('ENDPOINT_URL') always returned nil — the GCS backend detection in PRESIGN_PART never fired and presigned part PUTs to GCS always used the wrong canonical resource (with subresources). - Add 'env ENDPOINT_URL;' to nginx.conf.template so Lua can read it. - In PRESIGN_PART, move the '%3A' encoding inside the GCS branch so standard S3 backends (cloudserver, Scaleway) keep literal ':' and subresources in the canonical resource, while GCS gets '%3A' and no subresources — matching what each backend verifies. - Remove test_presign_upload_part_encodes_colons_in_url: the '%3A' encoding is GCS-specific and cannot be asserted in CI (cloudserver). The end-to-end round-trip test covers correctness for cloudserver. Co-Authored-By: Claude Sonnet 4.6 --- conf/nginx.conf.template | 3 +++ lua/compute_aws_s3_signature.lua | 17 ++++++++++------- tests/end2end/test_presign_upload.py | 20 -------------------- 3 files changed, 13 insertions(+), 27 deletions(-) diff --git a/conf/nginx.conf.template b/conf/nginx.conf.template index 922a8c3..eb0928a 100644 --- a/conf/nginx.conf.template +++ b/conf/nginx.conf.template @@ -20,6 +20,9 @@ env BOT_TOKEN; # Google Cloud Storage S3 compatible bucket prefix. env AWS_BUCKET_PREFIX; +# S3-compatible endpoint URL, used to detect backend-specific signature quirks. +env ENDPOINT_URL; + # It is recommended to set one worker process per CPU core. pid /run/nginx.pid; diff --git a/lua/compute_aws_s3_signature.lua b/lua/compute_aws_s3_signature.lua index c5b795d..4c6bfad 100644 --- a/lua/compute_aws_s3_signature.lua +++ b/lua/compute_aws_s3_signature.lua @@ -376,20 +376,23 @@ elseif signature_mode == "PRESIGN_PART" then end local expires = ngx.time() + 3600 local aws_secret_key = os.getenv('AWS_SECRET_ACCESS_KEY') - -- GCS normalises ':' to '%3A' when computing StringToSign for presigned URLs; - -- encode it here so the signature matches what GCS will verify. - local url_safe_key = ngx.var.encoded_key:gsub(':', '%%3A') -- Normalise uploadId: ngx.var.arg_* may be pre-encoded or raw depending on the nginx -- version; unescape then re-escape to avoid double-encoding (%2B → %252B). local escaped_upload_id = ngx.escape_uri(ngx.unescape_uri(upload_id)) - -- GCS does NOT include ?partNumber=N&uploadId=X in the canonical resource for presigned - -- part PUTs. Standard AWS S3-compatible backends (cloudserver, Scaleway S3) do include - -- them per the V2 spec. Detect GCS from ENDPOINT_URL to pick the right behaviour. + -- GCS has two quirks vs standard S3 V2 for presigned part PUTs: + -- 1. It normalises ':' to '%3A' in the key when verifying StringToSign. + -- 2. It excludes ?partNumber=N&uploadId=X from the canonical resource. + -- Standard S3-compatible backends (cloudserver, Scaleway) use literal ':' + -- and include subresources per the V2 spec. + -- ENDPOINT_URL must be declared with 'env ENDPOINT_URL;' in nginx.conf so + -- os.getenv() can read it from Lua workers. local endpoint_url = os.getenv('ENDPOINT_URL') or '' - local canonicalized_resource + local url_safe_key, canonicalized_resource if endpoint_url:find('googleapis', 1, true) then + url_safe_key = ngx.var.encoded_key:gsub(':', '%%3A') canonicalized_resource = "/" .. ngx.var.aws_tgt_bucket .. "/" .. url_safe_key else + url_safe_key = ngx.var.encoded_key canonicalized_resource = "/" .. ngx.var.aws_tgt_bucket .. "/" .. url_safe_key .. "?partNumber=" .. part_number .. "&uploadId=" .. escaped_upload_id end diff --git a/tests/end2end/test_presign_upload.py b/tests/end2end/test_presign_upload.py index 27b7b3f..84f8618 100644 --- a/tests/end2end/test_presign_upload.py +++ b/tests/end2end/test_presign_upload.py @@ -118,26 +118,6 @@ def test_presign_upload_part_encodes_special_chars_in_upload_id(session, artifac ) -def test_presign_upload_part_encodes_colons_in_url(session, artifacts_url): - """Presigned part URL path must use %3A for ':' — GCS normalises ':' to '%3A' - when computing StringToSign, so a literal ':' causes SignatureDoesNotMatch.""" - upload_id = multipart_initiate(session, artifacts_url, STAGING_BUILD, 'presign/colon-part.bin') - try: - resp = session.get( - f'{artifacts_url}/presign-upload-part/{STAGING_BUILD}/presign/colon-part.bin', - params={'partNumber': 1, 'uploadId': upload_id}, - ) - assert resp.status_code == 200, f'{resp.status_code} {resp.text}' - url = resp.text.strip() - path = urlparse(url).path - assert '%3A' in path, f"Expected '%3A' in presigned part URL path, got: {path!r}" - assert ':' not in path, f"Raw ':' in presigned part URL path causes SignatureDoesNotMatch on GCS" - finally: - session.delete( - f'{artifacts_url}/upload-multipart/abort/{STAGING_BUILD}/presign/colon-part.bin', - params={'uploadId': upload_id}, - ) - def test_presign_multipart_full_round_trip(session, artifacts_url): """Initiate via nginx, upload parts directly to S3, complete via nginx."""