From e21340968bb10c7003168ce17c83da6bd3362ffe Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Sat, 12 Sep 2026 02:02:19 +0000 Subject: [PATCH 1/2] fix: refuse URL userinfo on the s3:// and gs:// paths (#995) http(s) has refused userinfo by name since #706. The s3/gs path absorbed it into the BUCKET NAME and told the caller the object did not exist: http://u:p@127.0.0.1:1/x.parquet 22023 userinfo in "..." is not supported s3://u:p@mybucket/x.parquet 58P01 "..." does not exist (HTTP 404) That message is true and useless. The object did not exist BECAUSE we made u:p@mybucket a bucket name. s3://user:key@bucket/obj is a form other tools accept, so it is a mistake a user makes, and the answer sent them hunting a missing object. MEASURED ON MAIN WITH AN ENDPOINT CONFIGURED, which is the only way to reach the authority parse at all: path style HEAD /u%3Ap%40pgc-bucket/vh.parquet, Host untouched -> HTTP 404 virtual host the bucket becomes the leftmost Host label, so the whole string goes to the resolver -> could not resolve "u:p@pgc-bucket.s3.local" write export_parquet raised NO ERROR and PUT to /u%3Ap%40pgc-bucket/ui.parquet -- a bucket nobody named NOT AN SSRF, and that was checked rather than assumed: nothing here splits the authority at '@', so the userinfo never becomes basic auth and never moves the host. The write row is why this is refused rather than documented -- acceptance there sends bytes to a bucket the caller did not ask for. The guard sits BEFORE the endpoint is resolved. Placed after it, the refusal would be unreachable whenever no endpoint is configured, because the s3 branch demands one first (28000) -- and the arms could then say nothing without an object-store fixture. Eight arms, five refusals and THREE CONTROLS. The controls are not decoration: my first probe of this gap proved nothing, because with no credentials every s3 URL returned 28000, THE CLEAN ONE INCLUDED, so a userinfo refusal was indistinguishable from an unreachable object store. They hold that a clean URL still reaches the endpoint demand, that a malformed URL still gets the bucket/key refusal, and that an '@' in the KEY is left alone -- the false positive this guard could plausibly have had. red -> green: 5 arms 28000 -> 22023, 3 controls PASS on both trees derived gate list = the 11 registered suites naming an s3:// or gs:// URL all 11 green on pg18a (183 checks, 0 FAIL) and on pg19a build preflight pg15a/16a/17a/18a/19a: rc=0, 0 warnings each Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- CHANGELOG.md | 35 ++++++++++++++++++++++ objstore/columnar_objstore_module.c | 34 +++++++++++++++++++++ test/objstore_userinfo.sh | 46 +++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d57dfad3..83a6cda9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2054,6 +2054,41 @@ true until the next version shipped. The blobs stay reachable in the repository's history -- removing a file from the tree does not unwrite it, and rewriting `main` is not something a stray artifact justifies. What this stops is the tree carrying them, and the next `git add -A` re-adding them. +- `s3://` and `gs://` now refuse URL userinfo, as `http(s)://` has since #706. They were + absorbing it into the bucket name and reporting a missing object (#995). + + http://u:p@127.0.0.1:1/x.parquet 22023 userinfo in "..." is not supported + s3://u:p@mybucket/x.parquet 58P01 "..." does not exist (HTTP 404) <- before + s3://u:p@mybucket/x.parquet 22023 userinfo in "..." is not supported <- after + + The old message was true and useless: the object did not exist *because* `u:p@mybucket` + had become a bucket name. `s3://user:key@bucket/obj` is a form other tools accept, so it + is a mistake a user makes, and the answer sent them looking for a missing object. + + **Measured on main with an endpoint configured**, which is the only way to reach the + authority parse at all: + + path style HEAD /u%3Ap%40pgc-bucket/vh.parquet, Host untouched -> HTTP 404 + virtual host the bucket becomes the leftmost Host label -> could not resolve + "u:p@pgc-bucket.s3.local" + write export_parquet raised NO ERROR and PUT to + /u%3Ap%40pgc-bucket/ui.parquet -- a bucket nobody named + + **Not an SSRF, and that was checked rather than assumed.** Nothing splits the authority + at `@`, so the userinfo never becomes basic auth and never moves the host. The write row + is why this is refused rather than documented: acceptance there sends bytes to a bucket + the caller did not ask for. + + The guard sits **before the endpoint is resolved**. Placed after it, the refusal would be + unreachable whenever no endpoint is configured, because the s3 branch demands one first + -- and the arms could then say nothing without an object-store fixture. + + Eight arms in `test/objstore_userinfo.sh`, five of them refusals and **three of them + controls**, because the first probe of this gap proved nothing: with no credentials every + s3 URL returned `28000`, the clean one included, so a userinfo refusal was + indistinguishable from an unreachable object store. The controls hold that a clean URL + still reaches the endpoint demand, that a malformed URL still gets the bucket/key + refusal, and that an `@` in the KEY is left alone. ## [1.0-alpha3] - 2026-09-02 diff --git a/objstore/columnar_objstore_module.c b/objstore/columnar_objstore_module.c index d257431b..3db1403f 100644 --- a/objstore/columnar_objstore_module.c +++ b/objstore/columnar_objstore_module.c @@ -1342,6 +1342,40 @@ os_resolve_s3(PgColumnarObjHandle *h, const char *url, errmsg("columnar: \"%s\" is not %s://bucket/key", url, isGs ? "gs" : "s3"))); + /* + * Userinfo is refused here, the same as os_open and http_request do for + * http(s) (#706). This path never did, and the failure was quiet rather than + * open: the '@' is not special to a bucket name, so `s3://u:p@bucket/key` + * parsed, the userinfo became part of the BUCKET, and the caller was told the + * object does not exist. + * + * Measured on main with an endpoint configured (#995): + * + * path style HEAD /u%3Ap%40pgc-bucket/vh.parquet, Host untouched + * -> "does not exist (HTTP 404)" + * virtual host the bucket is the leftmost Host label, so the whole string + * goes to the resolver -> "could not resolve + * \"u:p@pgc-bucket.s3.local\"" + * write export_parquet raised NO ERROR and PUT to + * /u%3Ap%40pgc-bucket/ui.parquet -- a bucket the caller + * never named + * + * So this is a wrong-reason defect and not an SSRF: nothing here splits the + * authority at '@', and the userinfo never becomes basic auth or moves the + * host. The write row is why it is worth refusing rather than documenting -- + * acceptance there sends bytes somewhere nobody asked for. + * + * BEFORE THE ENDPOINT IS RESOLVED, deliberately. Placed after it, the refusal + * would be unreachable whenever no endpoint is configured, because the s3 + * branch demands one first (28000) -- and the arms for this in + * test/objstore_userinfo.sh would then need an object-store fixture to say + * anything at all. + */ + if (memchr(bucket, '@', slash - bucket) != NULL) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("columnar: userinfo in \"%s\" is not supported", url))); + if (cfg != NULL && cfg->endpoint != NULL) ep = cfg->endpoint; else diff --git a/test/objstore_userinfo.sh b/test/objstore_userinfo.sh index e30cc48b..5f9bf84d 100755 --- a/test/objstore_userinfo.sh +++ b/test/objstore_userinfo.sh @@ -63,6 +63,52 @@ check "and the export_parquet message names userinfo" \ check "export_arrow refuses userinfo through the same handle (22023)" \ "$(sqlstate_of "SELECT pgcolumnar.export_arrow('ex', 'http://u@127.0.0.1:1/x.arrow')")" "22023" +# --- the same gap on the s3/gs path, which was never closed (#995) ----------- +# +# Measured on main with an endpoint configured: the userinfo is percent-encoded +# INTO THE BUCKET NAME -- `HEAD /u%3Ap%40pgc-bucket/vh.parquet`, Host untouched -- +# and the caller is told the object does not exist, which is true and useless. The +# object does not exist because we made `u:p@pgc-bucket` a bucket name. Under +# virtual-host addressing the same string becomes the leftmost label of the +# hostname and fails at DNS. +# +# Nothing splits the authority at '@', so this is a wrong-reason defect rather +# than an SSRF. The write path is the half with a consequence: `export_parquet` +# raised NO error and PUT to a bucket the caller never named. +# +# NO OBJECT-STORE SERVER IS NEEDED. The guard is in the bucket parse, which runs +# BEFORE the endpoint is resolved, so it fires whether or not one is configured -- +# which is also why these arms can live in this suite beside the http ones. +S3UI="s3://u:p@mybucket/x.parquet" +check "read_parquet refuses userinfo in an s3:// bucket authority (22023)" \ + "$(sqlstate_of "SELECT * FROM pgcolumnar.read_parquet('$S3UI') AS t(v int)")" "22023" +check "and the s3 read message names userinfo" \ + "$(msg_of "SELECT * FROM pgcolumnar.read_parquet('$S3UI') AS t(v int)")" "1" +check "a gs:// bucket authority is refused by the same guard" \ + "$(sqlstate_of "SELECT * FROM pgcolumnar.read_parquet('gs://u@mybucket/x.parquet') AS t(v int)")" "22023" +check "export_parquet refuses it on the WRITE path too, where acceptance writes to a bucket nobody named" \ + "$(sqlstate_of "SELECT pgcolumnar.export_parquet('ex', '$S3UI')")" "22023" +check "and the s3 write message names userinfo" \ + "$(msg_of "SELECT pgcolumnar.export_parquet('ex', '$S3UI')")" "1" + +# THE CONTROL, without which the five arms above are worth nothing. My first probe +# of this gap proved exactly nothing: with no credentials every s3 URL returned +# 28000, the CLEAN ONE INCLUDED, so a userinfo refusal was indistinguishable from +# an unreachable object store. A clean URL must still reach the endpoint demand. +check "control: a clean s3:// URL is not refused as userinfo, it demands an endpoint" \ + "$(sqlstate_of "SELECT * FROM pgcolumnar.read_parquet('s3://mybucket/x.parquet') AS t(v int)")" "28000" +check "control: and that refusal names AWS_ENDPOINT_URL rather than userinfo" \ + "$(msg_of "SELECT * FROM pgcolumnar.read_parquet('s3://mybucket/x.parquet') AS t(v int)")" "0" +check "control: a malformed s3:// URL is still the bucket/key refusal, naming no userinfo" \ + "$(msg_of "SELECT * FROM pgcolumnar.read_parquet('s3://nokey') AS t(v int)")" "0" +# The false positive this guard could plausibly have: '@' is legal in an S3 KEY, +# and refusing one would break a working read. The guard scans only up to the first +# slash, so the key is outside it -- asserted rather than trusted to the comment. +check "control: an '@' in the KEY is not userinfo, and is not refused as it" \ + "$(msg_of "SELECT * FROM pgcolumnar.read_parquet('s3://mybucket/my@file.parquet') AS t(v int)")" "0" +check "control: and such a URL still reaches the endpoint demand, so it was not refused earlier" \ + "$(sqlstate_of "SELECT * FROM pgcolumnar.read_parquet('s3://mybucket/my@file.parquet') AS t(v int)")" "28000" + # --- the allow-list still does its own job on a clean URL -------------------- # The guard must not have swallowed the 42501 class: a userinfo-free URL to a # non-allowed endpoint is still the allow-list's refusal. From 3df6e674d19941da35dbc53173344e62b350517d Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Fri, 11 Sep 2026 21:26:38 -0600 Subject: [PATCH 2/2] test: the control names AWS_ENDPOINT_URL, and the arm count is twelve (#995) Two review findings from @jdatcmd, plus one of my own mistakes. 1. THE CONTROL ASSERTED AN ABSENCE. It read: check "control: and that refusal names AWS_ENDPOINT_URL rather than userinfo" \ "$(msg_of "...s3://mybucket/x.parquet...")" "0" and msg_of is `grep -c 'userinfo'`, so it said nothing at all about AWS_ENDPOINT_URL. Its name claimed more than its value expression checked -- the same defect as my #988 reason-detector matching the test's own name. It matters because 28000 is not unique to the endpoint demand: ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION is raised at three sites in os_resolve_s3 -- a missing endpoint, a missing credential, and the authorization refusal. A control seeing 28000 and no "userinfo" has pinned nothing. So there is a positive matcher now, and three controls where there was one: names AWS_ENDPOINT_URL 1 <- pins WHICH 28000 does not name userinfo 0 <- the new guard did not fire the gs:// refusal names AWS_ACCESS_KEY_ID 1 <- same 28000, DIFFERENT cause The third is worth a check rather than a comment: gs:// defaults its endpoint to the interop host, so it never reaches the endpoint demand and its 28000 is the credential demand. A reviewer reading the body concluded the gs arm "cannot have been 28000 on main"; it can, for the other reason, and an arm that names the variable settles it without anyone re-deriving it. THE MATCHER READS THE ERROR MESSAGE, NOT THE WHOLE OUTPUT. My first version grepped everything psql printed and returned 2, because the HINT also names the variable ("Set AWS_ENDPOINT_URL and restart"). Expecting 2 would have pinned the arm to how many places the hint mentions it. Removal proof per control, since a positive matcher that cannot fail is worse than the absence check it replaced: point it at a name nothing emits -> exactly 1 FAIL, the matching arm; restore -> 0 FAIL. 2. THE ARM COUNT. The body said eight, then ten; it is TWELVE. Counted with `grep -cE '^check '` across the revisions rather than retyped, which is how it drifted twice: main 7 check calls branch 19 check calls -> 12 added: 5 refusals, 7 controls 3. A CREDIT THAT WOULD NOT RESOLVE. A source comment read "Reported by @pgcolumnar-9b" -- a session name, not a GitHub account. An @ that resolves to nothing reads as a person who has left. It names the reviewing account now. objstore_userinfo 19 checks 0 FAIL | shellcheck -S error -s bash 0 | bash -n clean Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- CHANGELOG.md | 21 ++++++++++++++++----- test/objstore_userinfo.sh | 25 ++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 83a6cda9..e74f47e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2083,13 +2083,24 @@ true until the next version shipped. unreachable whenever no endpoint is configured, because the s3 branch demands one first -- and the arms could then say nothing without an object-store fixture. - Eight arms in `test/objstore_userinfo.sh`, five of them refusals and **three of them - controls**, because the first probe of this gap proved nothing: with no credentials every - s3 URL returned `28000`, the clean one included, so a userinfo refusal was - indistinguishable from an unreachable object store. The controls hold that a clean URL - still reaches the endpoint demand, that a malformed URL still gets the bucket/key + **Twelve arms** added to `test/objstore_userinfo.sh` -- five refusals and **seven + controls**, taking the file from 7 checks to 19 -- because + the first probe of this gap proved nothing: with no credentials every s3 URL returned + `28000`, the clean one included, so a userinfo refusal was indistinguishable from an + unreachable object store. + + The controls hold that a clean URL still reaches the endpoint demand **and that the + message names `AWS_ENDPOINT_URL`**, that a malformed URL still gets the bucket/key refusal, and that an `@` in the KEY is left alone. + That positive matcher matters more than it looks: `28000` is raised by **three** different + demands in `os_resolve_s3` -- a missing endpoint, a missing credential, and the + authorization refusal -- so a control that sees `28000` and no `userinfo` has pinned + nothing about which one fired. The `gs://` control makes the point concretely: its `28000` + is the **credential** demand, because `gs` defaults its endpoint to the interop host and + never reaches the endpoint demand at all. Same code, different cause, and only a matcher + that names the variable can tell them apart. + ## [1.0-alpha3] - 2026-09-02 ### Added diff --git a/test/objstore_userinfo.sh b/test/objstore_userinfo.sh index 5f9bf84d..b0effc9c 100755 --- a/test/objstore_userinfo.sh +++ b/test/objstore_userinfo.sh @@ -45,6 +45,20 @@ msg_of() { -d "$PGC_DB" -qtA -c "$1" 2>&1 | grep -c 'userinfo' } +# msg_of answers only "does the message say userinfo", so a control built on it asserts an +# ABSENCE. That is not enough here: 28000 is raised by THREE different demands in +# os_resolve_s3 -- a missing endpoint, a missing credential, and the allow-list's own +# authorization refusal -- so a control that sees 28000 and no `userinfo` has not pinned +# WHICH refusal fired. Reported by @jdatcmd in review on #997. +msg_has() { # msg_has SQL PATTERN -> count of matches in the ERROR MESSAGE alone + # The message alone, not the whole output: the HINT beside it also names the + # variable ("Set AWS_ENDPOINT_URL and restart"), so counting every line gives 2 + # and the arm would be pinned to how many places the hint mentions it rather + # than to which refusal fired. + env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ + -d "$PGC_DB" -qtA -c "$1" 2>&1 | sed -n 's/^ERROR: //p' | grep -c -- "$2" +} + URL="http://u:p@127.0.0.1:1/x.parquet" # --- premise: the read path refuses userinfo with the parse guard ------------ @@ -97,8 +111,17 @@ check "and the s3 write message names userinfo" \ # an unreachable object store. A clean URL must still reach the endpoint demand. check "control: a clean s3:// URL is not refused as userinfo, it demands an endpoint" \ "$(sqlstate_of "SELECT * FROM pgcolumnar.read_parquet('s3://mybucket/x.parquet') AS t(v int)")" "28000" -check "control: and that refusal names AWS_ENDPOINT_URL rather than userinfo" \ +check "control: and that refusal NAMES AWS_ENDPOINT_URL, pinning which 28000 fired" \ + "$(msg_has "SELECT * FROM pgcolumnar.read_parquet('s3://mybucket/x.parquet') AS t(v int)" \ + 'AWS_ENDPOINT_URL')" "1" +check "control: and it does not name userinfo, so the new guard did not fire on it" \ "$(msg_of "SELECT * FROM pgcolumnar.read_parquet('s3://mybucket/x.parquet') AS t(v int)")" "0" +# The gs arm's 28000 is a DIFFERENT demand from the s3 arm's, which is the whole reason the +# two controls above are needed: gs defaults its endpoint to the interop host and then +# demands a credential, so it never reaches the endpoint demand at all. +check "control: the gs:// refusal names a CREDENTIAL, not the endpoint -- same 28000, other cause" \ + "$(msg_has "SELECT * FROM pgcolumnar.read_parquet('gs://mybucket/x.parquet') AS t(v int)" \ + 'AWS_ACCESS_KEY_ID')" "1" check "control: a malformed s3:// URL is still the bucket/key refusal, naming no userinfo" \ "$(msg_of "SELECT * FROM pgcolumnar.read_parquet('s3://nokey') AS t(v int)")" "0" # The false positive this guard could plausibly have: '@' is legal in an S3 KEY,