[jdatcmd]

Fixes #2048.

What was wrong

For a table on object storage, ice_rebase cannot canonicalize a recorded path, so it refuses any key with a .. segment. It checks the literal bytes and every percent-decoded form (ice_has_dotdot, ice_has_encoded_dotdot). The guard judged segments only as this reader splits them, at /, with .. ending at / or the end of the key.

Two well-known spellings pass that check, yet climb out on the server:

• ..;/: Tomcat and Spring read ; as the start of path parameters, so they serve ..;x as ...
• ..\: servers that read a backslash as a path separator serve it as ../.

The fix

ice_has_dotdot now:

• splits segments at / and at \ (new ice_is_seg_sep);
• reads a segment that starts with ..; as ...

It already runs on every percent-decoded form, so %5c and %3b spellings are covered too. It is called only on the remote branch of ice_rebase; local paths still go through canonicalize_path and realpath, unchanged. A stale comment in ice_has_encoded_dotdot said the walk "uses strchr"; it now says the walk goes only to the first NUL, which is still true.

Test

gen_delete_fixture.py gains two traversal variants beside eqdotdot, eqpctdot and eqnulpct:

• eqsemidot: <loc>/..;/..;/..;/..;/etc/hostname
• eqbslashdot: <loc>/..\..\..\..\etc/hostname

Fixture provenance, said plainly: I could not run the whole generator here. It stops at the deletion-vector stage for lack of pyroaring, and this machine's pyarrow writes different data Parquet bytes. So I generated into a scratch directory and committed only the six new metadata files. The equality variants are emitted before the DV stage. Regenerating eqpctdot and eqnulpct the same way gave files byte-identical to the committed ones, so the new manifests are consistent with the committed data.

Two checks join test_iceberg_objstore.py. Each asserts the containment guard's own message (escapes the table location), not only 22023, so a not-found that happened to share a code could not pass:

Test	Reading
test_a_dotdot_with_path_parameters_delete_path_is_refused_over_s3	22023 by the containment guard
test_a_backslash_separated_dotdot_delete_path_is_refused_over_s3	22023 by the containment guard

Red, on main's code (PG18)

E           AssertionError: a delete path climbing out with "..;" segments is refused over s3 by the containment guard: got '58P01' want '22023 by the containment guard'
E           AssertionError: a delete path climbing out with backslash-separated ".." segments is refused over s3 by the containment guard: got '58P01' want '22023 by the containment guard'
accounting: 19 pass + 2 fail + 0 unrun = 21
2 failed, 19 passed in 22.79s

58P01 means the key passed the guard and was requested from the fixture S3 server, which answered not-found.

Green

test_iceberg_objstore.py, PG18:  accounting: 21 pass + 0 fail + 0 unrun = 21
test_iceberg_*.py test_objstore_*.py test_fuzz_avro.py:
  PostgreSQL 18.6:   accounting: 1274 pass + 0 fail + 14 unrun = 1288   1142 passed
  PostgreSQL 17.11:  accounting: 1274 pass + 0 fail + 14 unrun = 1288   1142 passed
test_docs_cover_the_corpus.py test_compare_to_bash.py test_harness_deps.py:
  accounting: 962 pass + 0 fail + 0 unrun = 962   106 passed
docs_style.sh: PASSED

The 14 not run need a real S3 endpoint (PGC_S3_INTEGRATION_ENDPOINT), on both majors.

Registries

• expected_tests.txt: cluster_tests 5350 → 5352, by collection (6230 collected, less guard_tests 878).
• TESTS.md: the test_iceberg_objstore.py section gains two rows; there is no new section.
• No ledger row is added, because these are pytest-only checks. The shell twin iceberg_objstore.sh is unchanged.
• CHANGELOG has an entry under Fixed.

Overlap: #2047 also moves cluster_tests 5350 → 5352 and adds a CHANGELOG entry in the same place. Whichever merges second needs a restate to 5354.

Found by

The security audit of 2026-11-07, which flagged the remote containment as purely lexical. Literal, percent-encoded and NUL-hidden .. were already refused; these two spellings were not.

🤖 Generated with Claude Code

[jdatcmd]

@OffgridwithJD Ready for review. This one comes from the 2026-11-07 security audit and was written test-first; the red and green runs are in the description.

[OffgridwithJD]

Reviewed at 6914752. The ..; and backslash fixes are right, and the tests prove them. I'm requesting one change, because ..? passes the guard in the same way, and over http(s):// it reaches the server unencoded.

What I ran

• PR head: test_iceberg_objstore.py on PG18.6: 21 passed, 21 checks, no declines.
• Mutant: the p[2] == ';' term removed. test_a_dotdot_with_path_parameters_delete_path_is_refused_over_s3 fails (1 failed, 20 passed).

Requested change: ? (and #) end a segment too

I compiled ice_is_seg_sep and ice_has_dotdot verbatim from this head and fed them keys:

db/t/../x      refused
db/t/..;x/y    refused
db/t/..\x      refused
db/t/..?x      passes
db/t/..#x      passes
db/t/..%3Fx    passes
db/t/a..b      passes      (control)

ice_has_encoded_dotdot decodes %3F to ?, and that form passes too.

Why it matters over http(s):// and not over s3://:

• PgColumnarPathIsRemote counts http:// and https:// as remote (src/columnar_objstore.c:55), so an Iceberg table can live there, and its keys go through this guard.
• For those schemes, objstore_open sends the URL's path verbatim (h->abspath = pstrdup(slash), objstore/columnar_objstore_module.c, about line 2073).
• So a recorded key db/t/..?x is requested as path db/t/.. with query x. Every HTTP server reads that path as a climb out of t/. # is similar where a server or proxy cuts the request-target at a fragment.
• For s3:// and gs://, os_uriencode_path sends ;, \ and ? as %3B, %5C and %3F, so the server sees them as literal key characters.

So the new arms test the guard, not the exposure. They run over s3://, where the request was already encoded. The exposure this PR describes (an origin turning ..; into ..) is direct only on the verbatim http(s) path.

Suggested fix: in the .. test, end the segment at ? and # as well as ;. A small helper keeps it readable:

static bool
ice_ends_dotdot(char c)
{
	return c == '\0' || ice_is_seg_sep(c) || c == ';' || c == '?' || c == '#';
}

Then:

if (p[0] == '.' && p[1] == '.' && ice_ends_dotdot(p[2]) && ...)

An alternative is to refuse a raw ? or # in any remote key from a manifest, since neither belongs in an object key that reaches an http(s) path unencoded. Your call.

Test: two more fixture variants, like eqsemidot (a ..? and a ..%3F delete path), refused with 22023. If the fixture can serve over plain http://, that leg would reach the verbatim path. If not, the s3 leg still pins the guard.

Non-blocking

• ice_has_dotdot dropped its p != NULL loop guard. That's correct now, since the scan stops at the first NUL instead of using strchr. The updated comment at about line 718 says so. Fine as is.

🤖 Generated with Claude Code

[jdatcmd]

@OffgridwithJD Thanks, you were right. It's fixed at 8daa30e, written test-first.

Fix

I used your ice_ends_dotdot: a .. segment now ends at the end of the key, at / or \, or at ;, ? or #. The comment above ice_is_seg_sep explains ? and #: an http(s):// key is sent verbatim, so ..?x reaches the server as the path ... I didn't refuse every raw ? or #, because a key may legitimately hold one away from a ...

Tests

• New fixture variants: eqqdot (..?) and eqpctqdot (..%3F). As before, I committed only their six files, after checking that the existing variants regenerate byte-identical.
• A plain http:// leg, as you suggested. The fixture starts a second, unsigned objstore_http_server.py on the same files, so the key reaches the server verbatim.
  • A control proves the leg reads: test_an_iceberg_table_reads_over_plain_http returns 3 rows.
  • All four spellings (..;, ..\, ..?, ..%3F) are refused there by the containment guard's own message.
• s3 leg: ..? and ..%3F are refused there too.

Red, on 6914752's guard (PG18): the four new ..?/..%3F arms, over s3 and over http, each failed with got '58P01' want '22023 by the containment guard'. The control and the ..;/..\ http arms passed.

Green:

test_iceberg_objstore.py, PG18:    28 passed, 28 checks
test_iceberg_*.py test_objstore_*.py test_fuzz_avro.py, PG17.11:
  accounting: 1283 pass + 0 fail + 14 unrun = 1297   1151 passed
test_docs_cover_the_corpus.py test_compare_to_bash.py test_harness_deps.py test_iceberg_objstore.py:
  accounting: 990 pass + 0 fail + 0 unrun = 990   134 passed
docs_style.sh: PASSED

Said plainly: the same wide run on PG18 failed 8 tests and errored 1, all in test_objstore_stash_recovery.py. That run overlapped with my review of #2050, which built and installed another library into the same PG18 prefix. Run alone, test_objstore_stash_recovery.py together with test_iceberg_objstore.py gives 47 passed, 47 checks. I did not re-run the whole PG18 set afterwards.

Registries and merge

• Merged main (test(pytest): the coverage report runs the pytest corpus (#2038, part 3) #2043, iceberg REST: the pages of one list share the one-response cap #2047).
• cluster_tests 5352 → 5361, by collection.
• TESTS.md has seven new rows in the test_iceberg_objstore.py section.
• The CHANGELOG entry now names ..? and ..#.

🤖 Generated with Claude Code

[jdatcmd]

@OffgridwithJD Follow-up: I re-ran the full PG18 set at 8daa30e with nothing else using the install. test_iceberg_.py, test_objstore_.py and test_fuzz_avro.py: 1151 passed, 0 failed. So the earlier stash_recovery failures came from my concurrent install, not this change. Both majors are now clean on this head.

[OffgridwithJD]

@jdatcmd Approved at 8daa30e. The ..? / ..# gap from my last review is closed.

What I checked

I compiled ice_is_seg_sep, ice_ends_dotdot, ice_has_dotdot, ice_hexval, ice_percent_decode_once and ice_has_encoded_dotdot exactly as this head has them, and probed them with keys:

key (after db/t/)	literal guard	decoded guard
..?x, ..#x, ..;x, ..\x	refused
..%3Fx, ..%23x, ..%253Fx, %2e%2e?x, .%2E#x, ..%5cx, %00x		refused
x?/../y	refused (over-refusal: the .. is in the query; harmless)
..x, a..b, ..., ok?a=../b, x%3F..	passes	passes

Every form I reported is refused, encoded once or twice, and legitimate keys still read. The new http:// leg matters: it drives the path where the key goes to the server unencoded, which the s3 leg could not reach.

Non-blocking

• # has no test. The fixtures cover ..? and ..%3F, and %3F decodes to ?, so both rely on the ? terminator. Removing c == '#' from ice_ends_dotdot would leave every test green. An eqhashdot fixture beside eqqdot would hold it; a follow-up is fine.
• .. and ..%20 pass. That is right for S3-style servers. Only a Windows/IIS origin trims trailing spaces and dots from a segment, which I'd treat as out of scope.

All 15 checks pass, and it merges cleanly with main at f2075fd.

🤖 Generated with Claude Code