Skip to content

Fix crash parsing a mount directive with an empty key.#1896

Open
anxkhn wants to merge 1 commit into
apple:mainfrom
anxkhn:patch-5
Open

Fix crash parsing a mount directive with an empty key.#1896
anxkhn wants to merge 1 commit into
apple:mainfrom
anxkhn:patch-5

Conversation

@anxkhn

@anxkhn anxkhn commented Jul 5, 2026

Copy link
Copy Markdown

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Motivation and Context

Parser.mount(_:) splits each comma-separated part of a mount spec on = and then
immediately reads keyVal[0]. Swift's split(separator:) omits empty subsequences by
default, so a part made up solely of = characters produces an empty array and the
keyVal[0] read traps with a fatal Index out of range, crashing the process on user
input.

This is reachable straight from the CLI. For example:

container run --mount "=" ...
container run --mount "src=/x,=,type=bind" ...

The existing parts.count == 0 guard only catches an empty or all-comma string; it does
not catch a =-only segment.

The fix guards the first element and throws a
ContainerizationError(.invalidArgument, ...) so the malformed input is rejected the same
way other bad mount directives already are, instead of aborting the process. The change is
a single guarded read; the error message shape matches the sibling .invalidArgument
throws already in mount(_:).

This is the same class of user-input crash fix as #1612 ("Fix potential integer math crash
on PublishPort."), which also hardened Parser.swift and added a regression test.

Testing

  • Tested locally
  • Added/updated tests
  • Added/updated docs

Added testMountEmptyDirectiveSegment in Tests/ContainerAPIClientTests/ParserTest.swift,
asserting that "=", "==", and "type=bind,=,dst=/foo" each throw a
ContainerizationError whose description contains "invalid mount directive". Reverting only
the guard makes the new test trap with Fatal error: Index out of range (proving the crash);
with the guard it passes. swift format reports no drift, swift format lint --strict
passes, and the full ContainerAPIClientTests suite (184 tests) is green.


The diff (for reference; already committed locally on patch-5, unsigned)

Sources/Services/ContainerAPIService/Client/Parser.swift (+4/-1):

             let keyVal = part.split(separator: "=", maxSplits: 2)
-            var key = String(keyVal[0])
+            guard let rawKey = keyVal.first else {
+                throw ContainerizationError(.invalidArgument, message: "invalid mount directive '\(part)' in \(mount)")
+            }
+            var key = String(rawKey)

Tests/ContainerAPIClientTests/ParserTest.swift (+17): new @Test testMountEmptyDirectiveSegment().

Parser.mount split each comma-separated part on '=' and immediately read
keyVal[0]. Swift's split omits empty subsequences by default, so a part
consisting solely of '=' characters (for example `--mount "="` or a
`=` segment inside a longer spec) produced an empty array and trapped
with a fatal 'Index out of range', crashing the process on user input.

Guard the first element and throw a ContainerizationError(.invalidArgument)
so malformed input is rejected like other invalid mount directives. Add a
regression test covering "=", "==", and an embedded empty segment.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant