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. + * + *
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)."); } } } diff --git a/src/main/java/dev/marcospaulo/canalhandia/MiniMax.java b/src/main/java/dev/marcospaulo/canalhandia/MiniMax.java index 0431658..cffb24f 100644 --- a/src/main/java/dev/marcospaulo/canalhandia/MiniMax.java +++ b/src/main/java/dev/marcospaulo/canalhandia/MiniMax.java @@ -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. + * + *
Every step checks the type 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();
}
}
diff --git a/src/test/java/dev/marcospaulo/canalhandia/AiKeyTest.java b/src/test/java/dev/marcospaulo/canalhandia/AiKeyTest.java
index bae5bfe..e3ba408 100644
--- a/src/test/java/dev/marcospaulo/canalhandia/AiKeyTest.java
+++ b/src/test/java/dev/marcospaulo/canalhandia/AiKeyTest.java
@@ -42,6 +42,20 @@ class AiKeyTest {
assertEquals("sk-abc", Ai.cleanKey("sk-" + (char) 0 + "abc" + (char) 0x7F));
}
+ /**
+ * The case a deny-list of low control characters misses. Notepad and
+ * PowerShell {@code Out-File} put a U+FEFF byte order mark at the head of
+ * the file; the JDK rejects every char above U+00FF, quoting the header
+ * value back as it does.
+ */
+ @Test
+ void stripsAByteOrderMarkAndAnythingElseNonAscii() {
+ assertEquals("sk-abc123", Ai.cleanKey((char) 0xFEFF + "sk-abc123"));
+ assertEquals("sk-abc", Ai.cleanKey("sk-" + (char) 0x00E7 + "abc" + (char) 0x2013));
+ assertEquals("sk-abc", Ai.cleanKey("sk-" + (char) 0x00A0 + "abc"));
+ assertEquals("sk-abc", Ai.cleanKey("sk-" + (char) 0x0100 + "abc"));
+ }
+
@Test
void nothingUsableIsNull() {
assertNull(Ai.cleanKey(null));
@@ -49,10 +63,6 @@ class AiKeyTest {
assertNull(Ai.cleanKey(" " + LF + " "));
}
- /**
- * The contract that matters: whatever comes out is something
- * {@link HttpFetcher} will accept, so the JDK never gets to quote it back.
- */
/**
* The contract that matters, asserted end to end: anything {@code cleanKey}
* hands back is something {@link HttpFetcher} accepts, so a real key can
@@ -67,6 +77,8 @@ class AiKeyTest {
"sk" + (char) 9 + "-x",
"sk-x" + CR + LF + "# nota",
LF + "sk-y",
+ (char) 0xFEFF + "sk-bom",
+ "sk-" + (char) 0x00E7 + (char) 0x0100 + (char) 0xFFFD + "z",
};
for (String raw : messy) {
String key = Ai.cleanKey(raw);
diff --git a/src/test/java/dev/marcospaulo/canalhandia/HttpFetcherTest.java b/src/test/java/dev/marcospaulo/canalhandia/HttpFetcherTest.java
index f6274f9..c3a01cb 100644
--- a/src/test/java/dev/marcospaulo/canalhandia/HttpFetcherTest.java
+++ b/src/test/java/dev/marcospaulo/canalhandia/HttpFetcherTest.java
@@ -28,7 +28,7 @@ class HttpFetcherTest {
() -> new HttpFetcher(5).postJson(URL, "{}", key));
assertFalse(thrown.getMessage().contains("segredo"),
"key leaked into the exception: " + thrown.getMessage());
- assertTrue(thrown.getMessage().contains("controle"), thrown.getMessage());
+ assertTrue(thrown.getMessage().contains("imprimível"), thrown.getMessage());
}
@Test
@@ -42,10 +42,44 @@ class HttpFetcherTest {
}
}
+ /**
+ * The message assertions matter here: {@code UnknownHostException} is an
+ * {@code IOException}, so a bare {@code assertThrows} would still pass if
+ * the guard regressed — while quietly doing real DNS I/O.
+ */
@Test
void rejectsAnEmptyOrNullBearer() {
- assertThrows(IOException.class, () -> new HttpFetcher(5).postJson(URL, "{}", ""));
- assertThrows(IOException.class, () -> new HttpFetcher(5).postJson(URL, "{}", null));
+ for (String bearer : new String[]{"", null}) {
+ IOException thrown = assertThrows(IOException.class,
+ () -> new HttpFetcher(5).postJson(URL, "{}", bearer));
+ assertTrue(thrown.getMessage().contains("vazia"),
+ "guard did not fire; this may have hit the network: " + thrown);
+ }
+ }
+
+ /**
+ * A key file saved by Notepad or PowerShell {@code Out-File} starts with a
+ * U+FEFF byte order mark. The JDK rejects every char above U+00FF, so a
+ * deny-list of low control characters alone lets this through to the
+ * validator that quotes the value back.
+ */
+ @Test
+ void rejectsAByteOrderMarkAndOtherNonAsciiCharacters() {
+ // Written as code points, not literals: a real BOM in this source
+ // file would be invisible to whoever next reads the test.
+ for (char c : new char[]{0xFEFF, 0x00A0, 0x00E7, 0x2013, 0xFFFD, 0x0100, 0x20}) {
+ String key = c + "sk-segredo";
+ IOException thrown = assertThrows(IOException.class,
+ () -> new HttpFetcher(5).postJson(URL, "{}", key),
+ "accepted a bearer containing U+" + Integer.toHexString(c));
+ assertFalse(thrown.getMessage().contains("segredo"), thrown.getMessage());
+ assertTrue(thrown.getMessage().contains("imprimível"), thrown.getMessage());
+ }
+ }
+
+ @Test
+ void acceptsAnOrdinaryPrintableAsciiKey() {
+ assertDoesNotThrow(() -> HttpFetcher.checkBearer("sk-Abc123_-.~+/=:"));
}
/**
diff --git a/src/test/java/dev/marcospaulo/canalhandia/MiniMaxTest.java b/src/test/java/dev/marcospaulo/canalhandia/MiniMaxTest.java
index b8d1fd3..d0455b1 100644
--- a/src/test/java/dev/marcospaulo/canalhandia/MiniMaxTest.java
+++ b/src/test/java/dev/marcospaulo/canalhandia/MiniMaxTest.java
@@ -1,5 +1,6 @@
package dev.marcospaulo.canalhandia;
+import com.google.gson.JsonArray;
import com.google.gson.JsonObject;
import com.google.gson.JsonParser;
import org.junit.jupiter.api.Test;
@@ -99,6 +100,14 @@ class MiniMaxTest {
JsonObject body = JsonParser.parseString(recorder.json).getAsJsonObject();
assertEquals(0.0, body.get("temperature").getAsDouble(),
"term selection must be deterministic");
+ assertEquals(500, body.get("max_tokens").getAsInt());
+ // A regression that sent only the system prompt would still produce a
+ // tool call — just an unguided one — and every other test would pass.
+ JsonArray messages = body.getAsJsonArray("messages");
+ assertEquals(2, messages.size());
+ assertEquals("user", messages.get(1).getAsJsonObject().get("role").getAsString());
+ assertEquals("onde acho diamante?",
+ messages.get(1).getAsJsonObject().get("content").getAsString());
JsonObject function = body.getAsJsonArray("tools").get(0).getAsJsonObject()
.getAsJsonObject("function");
assertEquals("buscar_wiki", function.get("name").getAsString());
@@ -261,6 +270,59 @@ class MiniMaxTest {
assertFalse(warnings.get(0).contains("segredo"), warnings.get(0));
}
+ /**
+ * JSON that parses but has the wrong shape. Gson throws unchecked on these,
+ * and {@code message()} runs outside {@code post()}'s try, so without type
+ * checks they escape to the async Bukkit worker as a bare stack trace
+ * rather than the null the API promises.
+ */
+ @Test
+ void malformedButParseableResponsesReturnNullRatherThanThrowing() {
+ String[] shapes = {
+ "{\"choices\":[\"texto\"]}",
+ "{\"base_resp\":\"texto\"}",
+ "{\"choices\":[{\"message\":\"texto\"}]}",
+ "{\"base_resp\":{\"status_code\":\"nao-e-numero\"},\"choices\":[]}",
+ "{\"choices\":{\"nao\":\"array\"}}",
+ "[]",
+ "\"apenas uma string\"",
+ "null",
+ };
+ for (String shape : shapes) {
+ MiniMax api = new MiniMax(replying(shape), null, w -> {
+ });
+ assertNull(api.answer("k", "m", List.of(), 1200, 0.3), shape);
+ assertNull(api.searchTerm("k", "m", "oi"), shape);
+ }
+ }
+
+ /** Some OpenAI-compatible servers return content as an array of parts. */
+ @Test
+ void nonStringContentIsNullRatherThanThrowing() {
+ MiniMax api = new MiniMax(replying(
+ "{\"choices\":[{\"message\":{\"content\":[{\"type\":\"text\",\"text\":\"oi\"}]}}]}"));
+ assertNull(api.answer("k", "m", List.of(), 1200, 0.3));
+ }
+
+ @Test
+ void theFailureWarningNamesTheHostSoAWrongRegionIsDiagnosable() {
+ List