Skip to content

Commit 0534b06

Browse files
committed
Finalize PATH_TRAVERSAL_IN suppression after deep-check (settled false positive)
Deep-checked the two PATH_TRAVERSAL_IN sites flagged by findsecbugs: LlamaLoader (native-lib path from the lib.path / java.library.path / tmpdir JVM properties) and OfflineModelGuard.check (a read-only Files.exists on the configured model path). In both the tainted input is the operator's own process configuration set at launch, not untrusted input crossing a privilege boundary, and there is no allowed-root to validate against (pointing at an arbitrary GGUF/library anywhere on disk is the whole point). So it is a settled false positive for a JNI library and no code fix is appropriate. Consolidate the two suppression blocks into one finalized <Match> over all three classes with the reviewed rationale, drop the "provisional/under review" language, and close the deep-check item in TODO.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0137c1LhUbNvW3kt4eF9Kqyb
1 parent 65bbf02 commit 0534b06

2 files changed

Lines changed: 30 additions & 51 deletions

File tree

TODO.md

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,6 @@ cross-cutting initiative.
1313

1414
## Open — jllama-specific
1515

16-
### PATH_TRAVERSAL_IN suppressions — deep-check whether they can be resolved (open)
17-
18-
Two `PATH_TRAVERSAL_IN` suppressions live in `spotbugs-exclude.xml`: the long-standing
19-
`LlamaLoader` (native-lib path resolved from three operator-controlled inputs) and the new
20-
`OfflineModelGuard` / `ModelParameters` (`--model` GGUF path), added when SpotBugs moved from
21-
the publish-only `verify` phase to the fast early `code-style` CI gate (so the finding now reds
22-
every PR, not just a release). Both are currently justified as "operator-supplied path, no
23-
meaningful allowed-root." **Deep-check whether a genuine fix is feasible** — e.g. canonicalize +
24-
validate, reject `..` traversal where an expected root exists, or otherwise narrow the sink —
25-
instead of suppressing. If it is, replace the suppression with code + a regression test; if it
26-
genuinely is not, keep the suppression and record the finalized rationale here (and on the
27-
`LlamaLoader` block). See [`../workspace/policies/spotbugs-suppressions.md`](../workspace/policies/spotbugs-suppressions.md).
28-
2916
### PIT gate not hermetic — `value.ContentPart.audioFile(Path)` (open)
3017

3118
The PIT mutation gate reaches 100% **only when the audio test fixture is present**. Without it the

spotbugs-exclude.xml

Lines changed: 30 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -123,47 +123,39 @@ SPDX-License-Identifier: MIT
123123
</Match>
124124

125125
<!--
126-
LlamaLoader is the native-library bootstrap. It resolves the path
127-
to libjllama.{so,dylib,dll} from three operator-controlled inputs:
128-
129-
1. -Dnet.ladenthin.llama.lib.path=<dir> (line 94)
130-
2. java.library.path entries (line 119)
131-
3. java.io.tmpdir + hardcoded basename (lines 133, 171, 215)
132-
133-
findsecbugs PATH_TRAVERSAL_IN flags every non-literal argument to
134-
Paths.get, treating "user input" syntactically as "any non-literal
135-
string". The threat-model reality is different: all three sources
136-
are JVM properties set at process launch by whoever started the
137-
process. An attacker who can set JVM properties has already won;
138-
there is no untrusted end-user input reaching these paths.
139-
140-
Canonicalize-and-restrict-to-root mitigation is not applicable
141-
because the whole purpose of the .lib.path property is to let the
142-
operator point at any directory containing the native library;
143-
there is no meaningful "allowed root" to validate against.
144-
-->
145-
<Match>
146-
<Class name="net.ladenthin.llama.loader.LlamaLoader"/>
147-
<Bug pattern="PATH_TRAVERSAL_IN"/>
148-
</Match>
149-
150-
<!--
151-
PROVISIONAL (under review; see java-llama.cpp TODO.md
152-
"PATH_TRAVERSAL_IN suppressions deep-check"). Same operator-supplied
153-
model-path case as the LlamaLoader suppression above.
154-
OfflineModelGuard.check() does Files.exists(Paths.get(model)) where the
155-
model path comes from ModelParameters.getModel() (the configured local
156-
model file) to verify the GGUF exists before native load; findsecbugs
157-
PATH_TRAVERSAL_IN taints that path. The model path is configured by
158-
whoever launches the process (CLI flag or builder), not untrusted
159-
end-user input, and the whole point of the setting is to let the
160-
operator point at an arbitrary GGUF on disk, so there is no meaningful
161-
allowed-root to validate against. Suppressed pending a deep check of
162-
whether a real resolution (canonicalize / restrict) is feasible for
163-
this and the LlamaLoader case.
126+
PATH_TRAVERSAL_IN (reviewed 2026-06-26): confirmed a false positive for
127+
this JNI library, so this suppression is permanent and no code fix is
128+
appropriate. Two operator-controlled path sites are flagged:
129+
130+
1. LlamaLoader (native-library bootstrap) resolves
131+
libjllama.{so,dylib,dll} from three JVM-launch inputs:
132+
a. the net.ladenthin.llama.lib.path system property (a directory)
133+
b. java.library.path entries
134+
c. java.io.tmpdir plus a hard-coded basename
135+
2. OfflineModelGuard.check() does Files.exists(Paths.get(model)),
136+
where model is ModelParameters.getModel() (the configured local
137+
model-file path), to fail fast when the offline flag is set and the
138+
local model is absent. This is a read-only existence check; the
139+
path is not opened or written here.
140+
141+
findsecbugs taints every non-literal Paths.get argument (System
142+
properties and CLI/builder config count as taint sources), but all of
143+
these inputs are the operator's own process configuration set at launch,
144+
not untrusted end-user input crossing a privilege boundary. An attacker
145+
who can set the model path, the lib.path property, or java.library.path
146+
already controls the JVM; there is no traversal boundary to cross.
147+
148+
Why no fix: the canonicalize-and-restrict-to-an-allowed-root remediation
149+
does not apply. Pointing at an arbitrary GGUF or library directory
150+
anywhere on disk is the entire purpose of these settings, so there is no
151+
meaningful allowed root, and a parent-directory-rejecting check would
152+
break the legitimate relative-path case. An embedder that exposes the
153+
model path to untrusted remote users must validate it before calling this
154+
library; that lies outside the library's API contract.
164155
-->
165156
<Match>
166157
<Or>
158+
<Class name="net.ladenthin.llama.loader.LlamaLoader"/>
167159
<Class name="net.ladenthin.llama.loader.OfflineModelGuard"/>
168160
<Class name="net.ladenthin.llama.parameters.ModelParameters"/>
169161
</Or>

0 commit comments

Comments
 (0)