nsivabalan opened a new pull request, #19342:
URL: https://github.com/apache/hudi/pull/19342
### Describe the issue this Pull Request addresses
A community user reported that Hudi does not sanitize partition-path values,
allowing a directory-traversal write outside a table's declared base path,
driven purely by the content of one ingested record field.
Hudi's built-in key generators compute a record's partition path directly
from the configured partition field's raw value. Escaping of that value only
happens when `hoodie.datasource.write.partitionpath.urlencode=true`, which is
opt-in and disabled by default, and even that escaping never rejects `..`. The
unsanitized value is then joined onto the base path
(`FSUtils.constructAbsolutePath` → `StoragePath`), which resolves `../`
segments with `java.net.URI.resolve` semantics, so a partition value like
`../../../../tmp/evil` on a single record causes
`HoodieWriteHandle.makeNewPath` to create a partition directory and write
base/log files completely outside the table base path.
In multi-tenant or shared-storage deployments where one writer's credentials
span many tables, a lower-trust data producer (e.g. an upstream Kafka producer
feeding a Hudi Streamer job) can influence the partition field and write
Hudi-format files into other teams' or tenants' table directories, causing
metadata pollution and potential corruption. This was reproduced end-to-end
against the released `release-1.2.0` artifacts using the real write client (no
mocks).
### Summary and Changelog
Rejects partition paths containing directory-traversal (`..`) segments so a
record can no longer write Hudi files outside the table base path. Two
complementary layers:
1. **Compute layer** — `PartitionPathEncodeUtils.hasPathTraversal` /
`validateNoPathTraversal` (new) reject any whole `..` path segment,
**independent of the `urlencode` setting**. Wired into the three independent
partition-path implementations so all writers are covered:
- `KeyGenUtils.getPartitionPath` / `getRecordPartitionPath` — Avro/record
path (Spark Datasource, Spark SQL, structured streaming, Hudi Streamer, Java
client, and the Flink record path).
- `CustomAvroKeyGenerator.getPartitionPath` — combined custom path.
- `PartitionPathFormatterBase.combine` — Spark row-writer / bulk_insert
path.
- `RowDataKeyGen.getPartitionPath` — Flink row-writer path.
2. **Write layer (defense-in-depth)** —
`FSUtils.constructAbsolutePathAndValidate` / `validatePartitionPathContained`
(new) verify the resolved absolute path stays contained under the table base
path before any directory is created. Wired into
`HoodieWriteHandle.makeNewPath`/`makeNewFilePath`/`createLogWriter`,
`BaseCreateHandle`, `HoodieAppendHandle`, `HoodieRowCreateHandle` (Spark) and
`HoodieRowDataCreateHandle` (Flink). This catches the bulk_insert path that
reads the partition value back from the `_hoodie_partition_path` meta field,
which bypasses the key generator. The shared read-path
`FSUtils.constructAbsolutePath` is intentionally left unchanged so
query/rollback/hive-sync continue to operate over already-existing tables.
Only a whole `..` segment causes an escape (per RFC 3986 URI resolution,
verified against `java.net.URI.resolve`), so legitimate values such as
`2024-01-01`, `region=us-west-2`, and hive-style `rider=../evil` (which
resolves to `base/rider=../evil`, contained) are still accepted.
Tests:
- `TestPartitionPathEncodeUtils` (new) — validator allow/reject cases.
- `TestFSUtils` — containment cases including a sibling-prefix escape
(`../table-evil`).
- `TestKeyGenUtils#testGetPartitionPathRejectsPathTraversal` — single- and
multi-field keygen paths.
-
`TestHoodieJavaWriteClientInsert#testInsertRejectsPathTraversalPartitionPath` —
end-to-end: reproduces the PoC, asserts the write fails and no directory is
created outside the base path.
### Impact
Writers now fail fast with a clear error when a partition path contains a
`..` traversal segment. Any existing pipeline that legitimately produced a
partition value that is exactly `..` (or contains a `..` segment) without
url-encoding would now be rejected; such values were never safe as directory
names. Read/query, rollback, and sync paths are unchanged.
### Risk Level
low
The change adds validation at write time and at partition-path computation.
It does not alter the resolved path for any legitimate partition value
(verified against `java.net.URI.resolve` semantics and covered by unit tests
for allowed values), and it leaves the shared read-path `constructAbsolutePath`
untouched. New unit and end-to-end tests cover both the allowed and rejected
cases across the Avro, Spark-row, and Flink-row paths.
### Documentation Update
none
### Contributor's checklist
- [ ] Read through [contributor's
guide](https://hudi.apache.org/contribute/how-to-contribute)
- [ ] Change Logs and Impact were stated clearly
- [ ] Adequate tests were added if applicable
- [ ] CI passed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
