fix(core): prevent path traversal in DummyPayloadStorage

[Denimworld12]

🎟️ Tracking

Fixes #1639 (CodeQL alert #4, java/path-injection), sub-issue of #1010.

Approach taken from @nthmost-orkes on the issue:

the unvalidated new File(payloadDir, path) appears at two call sites — upload() at line 68 and download() at line 95 — so the canonical-path guard needs to cover both (best done in a shared resolve/validate helper, as #723 did for MockExternalPayloadStorage).

📔 Objective

DummyPayloadStorage resolved the caller-supplied path against payloadDir with no validation, at both call sites:

public void upload(String path, InputStream payload, long payloadSize) {
    File file = new File(payloadDir, path);
    ...
public InputStream download(String path) {
    return new FileInputStream(new File(payloadDir, path));

A ../-style path could therefore read or write outside the payload directory. This is the default payload storage when no external storage is configured, so it is reachable in dev and default deployments.

This adds a private validateAndResolvePath mirroring the one merged in #723: normalize with Paths.get(path).normalize(), reject a normalized path that still contains .., then verify the canonical path remains within payloadDir, throwing SecurityException on escape. Both methods route through it.

Following #723, both call sites catch SecurityException rather than letting it propagate, so download() keeps returning null on failure and the existing contract is unchanged. upload() also gained the getParentFile().mkdirs() from that same fix, which is what makes legal nested paths work.

Validation

DummyPayloadStorageTest gains three cases — traversal rejected on read, traversal rejected on write, and a nested path still round-tripping. 6/6 pass in :conductor-core:test, and :conductor-core:spotlessApply reports no changes.

I also checked the helper's logic in isolation against the case the .. string check cannot see on its own — a symlink inside payloadDir pointing outside it. The canonical-path comparison rejects it, so both halves of the guard are load-bearing. Absolute paths such as /etc/passwd are contained rather than rejected, since File(parent, child) treats an absolute child as relative and resolves it under payloadDir.

The sibling implementations in the same package already raise constant messages with no path values embedded, so nothing here logs the rejected path beyond the existing LOGGER.error call sites.

[nthmost-orkes]

Pushed one commit on top:

• The upload-traversal test now asserts on disk that no file lands outside payloadDir. Before, it only checked download() returned null — but download rejects any ../ path on its own, so the test passed even if upload had written the escaped file. Added a package-private getPayloadDir() seam so the test can look where a ../ write would actually go.
• Restructured upload() so the payload stream is always closed, including when validation rejects the path (previously it leaked on that branch, since the close sat in an inner finally that rejection skipped).

Guard logic itself is unchanged and still mirrors #723. LGTM.