fix: restrict API keys to printable ASCII, and harden response parsing

Fuzzing all 65,536 char values, not the 256 of the previous pass, shows
the JDK rejects 65,312 of them from a header value and every single one
echoes the value back. The previous deny-list covered 33. A key file
saved by Notepad or PowerShell Out-File carries a U+FEFF byte order
mark, which passed both filters and reached the quoting validator.

Ai.cleanKey and HttpFetcher.checkBearer now allow printable ASCII only,
an allow-list of the 94 characters a bearer token is made of, which
cannot drift out of what the JDK accepts. Demonstrated invariant:
cleanKey output ⊆ checkBearer accepts ⊆ JDK accepts, 0 violations.

Ai.call gains the same check and its failure log is now redacted; that
was the one path where the proof-of-concept leak surfaced.

MiniMax.message and answer now check JSON types before assuming them.
Gson throws unchecked on JSON that parses but has the wrong shape, and
both run outside post()'s try, so {"choices":["str"]} escaped to an
async Bukkit worker as a stack trace instead of the promised null.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3jSvSTrG4qsniLSr6TpC6
This commit is contained in:
marcos
2026-08-05 16:18:32 +00:00
parent 88fe9c9695
commit c5d8f44bf2
6 changed files with 187 additions and 21 deletions
@@ -90,11 +90,18 @@ final class Ai {
StringBuilder out = new StringBuilder(line.length());
for (int i = 0; i < line.length(); i++) {
char c = line.charAt(i);
if (c >= 0x20 && c != 0x7F) {
// Printable ASCII only, which is all a bearer token contains.
// A deny-list of low controls is not enough: the JDK also
// rejects every char above U+00FF, so a U+FEFF byte order mark
// — what Notepad and PowerShell Out-File put at the head of a
// file — would survive and produce a header the JDK quotes
// back, key included. See HttpFetcher.checkBearer for the
// invariant this half must satisfy.
if (c > 0x20 && c < 0x7F) {
out.append(c);
}
}
String key = out.toString().trim();
String key = out.toString();
if (!key.isEmpty()) {
return key;
}
@@ -102,6 +109,12 @@ final class Ai {
return null;
}
/** Logs a warning with any occurrence of the key removed. */
private void warnWithout(String key, String message) {
plugin.getLogger().warning(
key == null || key.isEmpty() ? message : message.replace(key, "***"));
}
private String apiKey() {
String fromEnv = System.getenv(KEY_ENV);
if (fromEnv != null && !fromEnv.isBlank()) {
@@ -172,7 +185,11 @@ final class Ai {
try {
answer = call(key, prompt, settings);
} catch (Exception e) {
plugin.getLogger().warning("Falha na chamada à IA: " + e);
// Redacted: this catches everything call() can throw, including
// the JDK's header validation, whose message quotes the whole
// Authorization value back. cleanKey should make that
// impossible; this is the backstop if it ever does not.
warnWithout(key, "Falha na chamada à IA: " + e);
answer = null;
}
String finalAnswer = answer;
@@ -215,6 +232,11 @@ final class Ai {
body.addProperty("temperature", settings.aiTemperature());
// No "tools" and no "tool_choice": the model is given nothing it could call.
// Rejects a malformed key before the JDK's own validator can quote it
// back into an exception message. Task 10 retires this whole method in
// favour of MiniMax, which reaches the same check through HttpFetcher.
HttpFetcher.checkBearer(key);
HttpRequest request = HttpRequest.newBuilder(URI.create(settings.aiUrl()))
.timeout(Duration.ofSeconds(settings.aiTimeoutSeconds()))
.header("Content-Type", "application/json")
@@ -49,6 +49,10 @@ final class HttpFetcher implements Fetcher {
* <p>Checking here rather than at the call site means no future caller has
* to remember to redact. The message deliberately names only the position
* of the offending character, never any part of the key.
*
* <p>The invariant, demonstrated by fuzzing all 65,536 char values:
* {@code Ai.cleanKey output ⊆ checkBearer accepts ⊆ JDK accepts}. Widening
* either of the first two without rechecking the third reopens the leak.
*/
static void checkBearer(String bearer) throws IOException {
if (bearer == null || bearer.isEmpty()) {
@@ -56,10 +60,18 @@ final class HttpFetcher implements Fetcher {
}
for (int i = 0; i < bearer.length(); i++) {
char c = bearer.charAt(i);
if (c < 0x20 || c == 0x7F) {
throw new IOException("Chave de API inválida: caractere de controle na posição "
// Printable ASCII only. Restricting to what a bearer token is
// actually made of is the only range that is safe by construction:
// the JDK rejects every char above U+00FF as well as the low
// controls — 65,312 of the 65,536 values — so an allow-list of the
// 94 printable ones cannot drift out of what it accepts. A key
// saved by Notepad or PowerShell Out-File carries a U+FEFF byte
// order mark, which a deny-list of low controls alone lets through.
if (c < 0x21 || c > 0x7E) {
throw new IOException("Chave de API inválida: caractere não imprimível na posição "
+ i + " (de " + bearer.length() + "). "
+ "Verifique se o arquivo da chave tem uma única linha, sem quebra de linha.");
+ "Verifique se o arquivo da chave tem uma única linha, "
+ "sem quebra de linha e sem marca de ordem de byte (BOM).");
}
}
}
@@ -159,7 +159,11 @@ final class MiniMax {
return null;
}
JsonElement content = message.get("content");
if (content == null || content.isJsonNull() || content.getAsString().isBlank()) {
// isJsonPrimitive before getAsString: on an object or array that call
// throws UnsupportedOperationException, and this runs outside post()'s
// try. Some OpenAI-compatible servers return content as an array of
// parts rather than a string.
if (content == null || !content.isJsonPrimitive() || content.getAsString().isBlank()) {
warn.accept("IA: resposta vazia com max_tokens=" + maxTokens
+ " (o raciocínio do modelo pode ter consumido o orçamento).");
return null;
@@ -200,7 +204,10 @@ final class MiniMax {
warn.accept("IA: chamada interrompida.");
return null;
} catch (Exception e) {
redacted(key, "IA: falha na chamada: " + e);
// The host is named because MiniMax's regional endpoints reject
// each other's keys: without it a wrong-host misconfiguration and
// an expired key produce the same log line.
redacted(key, "IA: falha na chamada a " + url + ": " + e);
return null;
}
}
@@ -219,7 +226,16 @@ final class MiniMax {
warn.accept(key == null || key.isEmpty() ? message : message.replace(key, "***"));
}
/** The first choice's message, or null if the response reported a failure. */
/**
* The first choice's message, or null if the response reported a failure.
*
* <p>Every step checks the <em>type</em> it is about to assume, not just
* the presence of the key. This is called outside {@link #post}'s try, and
* Gson's {@code getAsJsonObject}/{@code getAsInt} throw unchecked on JSON
* that parses but has the wrong shape — {@code {"base_resp":"texto"}} or
* {@code {"choices":["texto"]}} — which would escape to the async worker as
* a bare stack trace instead of the null this method promises.
*/
private JsonObject message(JsonObject root) {
if (root == null) {
return null;
@@ -227,9 +243,12 @@ final class MiniMax {
// HTTP 200 does not mean success here: MiniMax reports application
// errors — bad key, no balance, rate limit — with a 200 and a non-zero
// status_code, so checking the HTTP status alone misses all of them.
if (root.has("base_resp")) {
JsonObject base = root.getAsJsonObject("base_resp");
if (base.has("status_code") && base.get("status_code").getAsInt() != 0) {
JsonElement baseResp = root.get("base_resp");
if (baseResp != null && baseResp.isJsonObject()) {
JsonObject base = baseResp.getAsJsonObject();
JsonElement status = base.get("status_code");
if (status != null && status.isJsonPrimitive() && status.getAsJsonPrimitive().isNumber()
&& status.getAsInt() != 0) {
warn.accept("IA: recusada, base_resp " + AiText.forLog(base.toString()));
return null;
}
@@ -239,7 +258,12 @@ final class MiniMax {
warn.accept("IA: resposta sem \"choices\": " + AiText.forLog(root.toString()));
return null;
}
JsonObject first = choices.getAsJsonArray().get(0).getAsJsonObject();
return first.has("message") ? first.getAsJsonObject("message") : null;
JsonElement first = choices.getAsJsonArray().get(0);
JsonElement message = first.isJsonObject() ? first.getAsJsonObject().get("message") : null;
if (message == null || !message.isJsonObject()) {
warn.accept("IA: choices com formato inesperado: " + AiText.forLog(root.toString()));
return null;
}
return message.getAsJsonObject();
}
}