From 1a7c8e122b98be9dab30a041cc6daca94287117b Mon Sep 17 00:00:00 2001 From: He-Pin Date: Sat, 4 Jul 2026 15:45:49 +0800 Subject: [PATCH] fix: keep multi outputs under target directory Motivation: The CLI documents --multi as writing multiple files to the requested directory. The previous output-path construction used os.RelPath on Jsonnet object keys, so keys such as ../escape.txt or a/../../escape.txt could escape the selected multi directory, while absolute keys could throw raw JVM path exceptions. Modification: Route multi output names through a dedicated helper that strips leading POSIX separators for absolute-looking keys and joins the result as an os.SubPath under the requested multi directory. os.SubPath rejects parent traversal that would escape the output root, and the CLI now maps those failures to user-facing openat errors. Add JVM CLI regressions for direct escapes, nested escapes, valid in-directory normalization, and absolute-key normalization. Result: Multi output file names now stay under the requested directory. Escaping paths fail cleanly without writing outside the target, valid normalized paths still work, and absolute-looking keys are treated as output-directory-relative paths. References: - README.md documents --multi as writing multiple files to the directory. --- .../sjsonnet/SjsonnetMainBase.scala | 16 +++++- .../test/src-jvm/sjsonnet/MainTests.scala | 53 +++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/sjsonnet/src-jvm-native/sjsonnet/SjsonnetMainBase.scala b/sjsonnet/src-jvm-native/sjsonnet/SjsonnetMainBase.scala index 4b97d65e..b28e8b27 100644 --- a/sjsonnet/src-jvm-native/sjsonnet/SjsonnetMainBase.scala +++ b/sjsonnet/src-jvm-native/sjsonnet/SjsonnetMainBase.scala @@ -421,6 +421,20 @@ object SjsonnetMainBase { case e: Error => Left(Error.formatError(e, maxTrace)) } + private def multiOutputPath(multiPath: String, fileName: String): Either[String, os.FilePath] = + Try { + // Multi output names must stay under the requested output directory. + val normalizedFileName = fileName.dropWhile(_ == '/') + (os.FilePath(multiPath) / os.SubPath(normalizedFileName)).asInstanceOf[os.FilePath] + }.toEither.left.map { + case e: IllegalArgumentException + if Option(e.getMessage).exists(_.contains("ups must be zero")) => + s"openat $fileName: path escapes from parent" + case e: InvalidPathException => s"openat $fileName: ${e.getMessage}" + case e: IllegalArgumentException => s"openat $fileName: ${e.getMessage}" + case e => e.toString + } + private def writeFile( config: Config, f: os.Path, @@ -692,7 +706,7 @@ object SjsonnetMainBase { ujson.transform(v, renderer) writer.toString } - relPath = (os.FilePath(multiPath) / os.RelPath(f)).asInstanceOf[os.FilePath] + relPath <- multiOutputPath(multiPath, f) _ <- writeFile(config, relPath.resolveFrom(wd), rendered, trailingNewline) } yield relPath } diff --git a/sjsonnet/test/src-jvm/sjsonnet/MainTests.scala b/sjsonnet/test/src-jvm/sjsonnet/MainTests.scala index 1c01cfb1..2d7caab2 100644 --- a/sjsonnet/test/src-jvm/sjsonnet/MainTests.scala +++ b/sjsonnet/test/src-jvm/sjsonnet/MainTests.scala @@ -299,6 +299,59 @@ object MainTests extends TestSuite { assert(os.read(multiDest / "world") == expectedWorld + "\n") } + test("multiRejectsOutputPathsEscapingParent") { + val root = os.temp.dir() + val multiDest = root / "out" + val (res, out, err) = + runMain("""{"../escape.txt": "owned"}""", "--exec", "--multi", multiDest, "--string") + assert(res == 1) + assert(out.isEmpty) + assert(err.contains("path escapes from parent")) + assert(!err.contains("IllegalArgumentException")) + assert(!os.exists(root / "escape.txt")) + } + + test("multiRejectsNestedOutputPathsEscapingParent") { + val root = os.temp.dir() + val multiDest = root / "out" + val (res, out, err) = + runMain("""{"a/../../escape.txt": "owned"}""", "--exec", "--multi", multiDest, "--string") + assert(res == 1) + assert(out.isEmpty) + assert(err.contains("path escapes from parent")) + assert(!err.contains("IllegalArgumentException")) + assert(!os.exists(root / "escape.txt")) + } + + test("multiAllowsNormalizedOutputPathsInsideParent") { + val root = os.temp.dir() + val multiDest = root / "out" + os.makeDir.all(multiDest) + val (res, out, err) = + runMain("""{"dir/../ok.txt": "ok"}""", "--exec", "--multi", multiDest, "--string") + assert((res, out, err) == ((0, s"$multiDest/ok.txt\n", ""))) + assert(os.read(multiDest / "ok.txt") == "ok\n") + } + + test("multiNormalizesAbsoluteOutputPathsInsideParent") { + val root = os.temp.dir() + val multiDest = root / "out" + val externalPath = root / "external" / "abs.txt" + val expectedPath = multiDest / os.SubPath(externalPath.toString.dropWhile(_ == '/')) + val (res, out, err) = + runMain( + "{" + ujson.write(externalPath.toString) + ": \"inside\"}", + "--exec", + "--multi", + multiDest, + "--string", + "--create-output-dirs" + ) + assert((res, out, err) == ((0, s"$expectedPath\n", ""))) + assert(os.read(expectedPath) == "inside\n") + assert(!os.exists(externalPath)) + } + test("missingExtStrFileErrorsAsImport") { checkCliStderrGolden( "cli_missing_ext_str_file.jsonnet",