oscerd opened a new pull request, #27119:
URL: https://github.com/apache/camel/pull/27119
## What
`GoogleCloudStorageConsumer.evaluateFileExpression()` confined the resolved
local download path to the configured `downloadFileName` directory only when
`downloadFileName` contained no `$`:
```java
boolean confineToDirectory = !downloadFileName.contains("$");
```
The comment justified the skip as "when the configuration already contains
an expression the local path is built by the route author, who is trusted".
That is only half true. The *template* is written by the route author, but
`${file:name}` interpolates `GoogleCloudStorageConstants.FILE_NAME`, which the
consumer sets from `blob.getName()` a few lines earlier — so it carries the
remote object name, which is untrusted input.
A configuration such as `downloadFileName=/tmp/downloads/${file:name}` — a
natural way to write it — therefore resolved an attacker-influenced object name
into the local path with no containment check at all.
## How
Compute the **static directory prefix** of the configured
`downloadFileName`: the part before the first expression token, trimmed back to
the last path separator. Trimming matters — `/tmp/down${file:name}` has `/tmp`
as its directory prefix, not `/tmp/down`, which is a partial path segment. When
such a prefix exists, the evaluated result is confined to it using the same
`GoogleCloudStorageFileNameHelper.assertWithinDirectory` check (lexical
normalise + symlink-aware real-path comparison) that the plain-directory branch
already used.
A `downloadFileName` that is fully dynamic with no static prefix (e.g.
`${file:name}`) keeps its current behaviour: the route author configured no
directory, so there is nothing to confine to. That case is now explicit in the
code rather than implied by the `$` check.
Object names keep resolving into sub-directories of the download directory,
since GCS object names commonly use `/` as a pseudo-directory separator.
## Notes for reviewers
- **One existing test changed its assertion.**
`routeAuthorSuppliedExpressionIsNotConfined` pinned that
`downloadFileName=target/${file:name}` with object name `../escape.txt` yields
`target/../escape.txt` — that is exactly the gap being closed, so it was
replaced by `fullyDynamicExpressionIsNotConfined` (`${file:name}`, no static
prefix), where the old behaviour genuinely does survive. Flagging it because it
is a deliberate assertion flip, not a rename.
- **Runtime-visible behaviour change**, hence the upgrade-guide entry: a
deployment relying on object names escaping the configured directory now gets
`IllegalArgumentException`.
- `/${file:name}` yields `/` — containment against the filesystem root,
which accepts anything absolute. Semantically correct (the author did configure
the root) but effectively a no-op check. Pinned by a test.
- The `$` detection remains a plain `contains("$")` / `indexOf('$')` rather
than a file-language parser. A literal `$` that is not an expression token
(e.g. `/tmp/pri$ce`) is treated as an expression start. That mis-detection
already exists on `main` — the old `!contains("$")` had the same blind spot and
skipped the check entirely — and this change makes it *more* restrictive rather
than less, so the detection was left as-is.
- Windows separators are handled defensively (`max(lastIndexOf('/'),
lastIndexOf('\\'))`), though the surrounding code still hard-codes `/` when
appending `${file:name}` on the plain-directory branch (pre-existing).
## Testing
`mvn test` in `components/camel-google/camel-google-storage`: **39 run, 0
failures, 0 errors, 0 skipped**. New cases cover the expression branch with a
plain object name, a nested object name, `../` at the start of the key, `../`
nested inside the key, and the fully dynamic configuration; plus 5 unit tests
for `staticDirectoryPrefix` itself.
https://issues.apache.org/jira/browse/CAMEL-25163
---
_Claude Code on behalf of @oscerd_
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
