Repository navigation
fix(#730): keep control characters out of shell endpoint uri output - #735
Conversation
…tput camel:endpoint-list, camel:endpoint-stats and camel:rest-registry-list all url decode the endpoint uri before printing it, and --decode defaults to true. URISupport.sanitizeUri masks credentials but leaves control characters alone, so percent encoded ESC, LF or BEL were re-armed by the decode and passed to ShellTable as raw bytes. The uris are not all route-author literals: getEndpoints() includes endpoints materialized by the dynamic EIPs and RuntimeEndpointRegistry tracks dynamic endpoints by design, so a uri can carry a substring that arrived on the wire. A raw LF splits the row, and ESC sequences are acted on by the terminal emulator, in the exact view an operator uses to see what a context is connected to. Fold the shared decode/sanitize into ShellUriHelper.prepareUriForDisplay and put control characters back into percent encoded form as the last step. Nothing is lost from the display: the operator sees %1B where the uri really has an ESC, and readability of ordinary escapes is unchanged. Also applies when --decode is false, so a literal control character in a uri is handled too. While here, an undecodable uri no longer aborts the whole listing: a malformed % sequence made URLDecoder throw and took out every row, not just the offending one. Adds ShellUriHelperTest and the junit-jupiter test dependencies the shell module did not have yet.
jbonofre
left a comment
There was a problem hiding this comment.
The fix is correct and the threat model is clearly articulated.
The ordering in prepareUriForDisplay (decode → sanitize → re-encode control chars) is the right invariant — doing encodeControlCharacters last means nothing downstream can reintroduce a raw control character. The decodeQuietly fallback correctly catches IllegalArgumentException and returns the original string, so a single malformed % no longer aborts the whole listing. The indexOfControlCharacter fast path avoids a StringBuilder allocation for every ordinary URI. Deprecated URLDecoder.decode(String, String) is replaced with the Charset form.
The 16 test cases cover the key attack shapes (OSC sequence, newline row-forge, literal control char without decode, malformed escape, credential masking). CI passes on Java 17 and 21.
One minor note (not blocking): Character.isISOControl covers C1 controls (\u0080–\u009F), encoded here as %80–%9F rather than the technically correct UTF-8 percent-encoding (%C2%80). For a display helper this is fine — the terminal won't interpret %80 as a control character. Just something to be aware of if those values ever appear in practice.
LGTM.
jbonofre
left a comment
There was a problem hiding this comment.
LGTM.
Just one note: Character.isISOControl covers \u0080–\u009F (C1 controls). The code encodes them as %80–%9F via String.format("%02X", (int) ch). Technically, non-ASCII code points in a URI should be expressed as their UTF-8 byte sequence (so \u0080 → %C2%80), but since this is a display helper (not a URL reconstructor) %80 achieves the security goal perfectly and is harmless.
|
Thanks for the review — and agreed on the C1 point.
Leaving it as it is, for the reason you gave: the output is a display string handed to No code change, so the approval stands as given. Claude Code on behalf of Andrea Cosentino |
Fixes #730
What
camel:endpoint-list,camel:endpoint-statsandcamel:rest-registry-listeach url-decode the endpoint uri before printing it, with
--decodedefaulting to
true.URISupport.sanitizeUrimasks credentials but does nottouch control characters, so a percent-encoded ESC/LF/BEL was re-armed by the
decode and handed to
ShellTableas raw bytes.The uris shown are not all route-author literals.
getEndpoints()includesendpoints materialised by the dynamic EIPs, and
RuntimeEndpointRegistrytracks dynamic endpoints by design, so a uri can contain a substring that
arrived on the wire. A raw LF splits the row and lets one endpoint render as
two; ESC sequences are acted on by the terminal emulator. This is the view an
operator uses to see what a context is actually connected to, so a garbled or
forgeable rendering is worth avoiding regardless of how the content got there.
How
The shared decode + sanitize is folded into
ShellUriHelper.prepareUriForDisplay(uri, decode), which puts controlcharacters back into percent-encoded form as its last step -- so nothing
downstream can reintroduce one.
Re-encoding rather than stripping means the display stays faithful: the
operator sees
%1Bexactly where the uri really has an ESC, and readabilityof ordinary escapes (
%20-> space) is unchanged. It also applies when--decodeisfalse, which covers a uri that carries a literal controlcharacter without any encoding.
RestRegistryListhad the same three lines and is fixed with the other two.While here: an undecodable uri no longer aborts the whole listing. A malformed
%sequence madeURLDecoder.decodethrowIllegalArgumentException, whichtook out every row rather than just the offending one.
Tests
Adds
ShellUriHelperTest(16 cases) covering decode-still-readable,credential masking, each of the encoded control characters, the OSC/newline
row-forging shapes, the literal-control-character path, and the malformed
escape. The shell module had no test sources, so this also adds the
junit-jupitertest dependencies.Notes
No behaviour change for a well-formed uri -- the fast path returns the string
unchanged when it contains no control character.
Claude Code on behalf of Andrea Cosentino