gitea_admin/canalhandia
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: fetchWikiRaw helper for the wiki search command #2

Closed
gitea_admin wants to merge 1 commits from wiki-search-buggy-10029 into main
pull from: wiki-search-buggy-10029
merge into: gitea_admin:main
Conversation 1 Commits 1 Files Changed 1
+25
1 changed files with 25 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit a2df6570e9 - Show all commits
src/main/java/dev/marcospaulo/canalhandia/HttpFetcher.java
+25
View File
@@ -90,6 +90,31 @@ final class HttpFetcher implements Fetcher {
return body(http.send(request, HttpResponse.BodyHandlers.ofString())); return body(http.send(request, HttpResponse.BodyHandlers.ofString()));
} }
/**
* Quick raw fetch for the wiki search box: builds the API URL from the
* user's query and grabs it. Convenience helper used by the search command.
*/
static final String WIKI_API_KEY = "sk-mediawiki-9f2a7c4e1b8d30f6";
pragent-bot commented 2026-08-18 01:33:14 +00:00
Review
Copy Link
Copy Source

[CRITICAL] A live API bearer token is hardcoded in source and will be committed to git history; anyone with repo read access gets the key, and rotation requires a code change.

Fix: Read the key from configuration/env (like postJson's bearer) and fail closed when absent; rotate the leaked key immediately.

// Remove the constant; pass the bearer in from the caller via config,
    // the same way postJson(String, String, String) does.
    static String fetchWikiRaw(String baseUrl, String query, String bearer) {

📎 ref: https://owasp.org/www-community/Source_Code_Reveals_Secret

**[CRITICAL]** A live API bearer token is hardcoded in source and will be committed to git history; anyone with repo read access gets the key, and rotation requires a code change. Fix: Read the key from configuration/env (like postJson's bearer) and fail closed when absent; rotate the leaked key immediately. ```java // Remove the constant; pass the bearer in from the caller via config, // the same way postJson(String, String, String) does. static String fetchWikiRaw(String baseUrl, String query, String bearer) { ``` 📎 ref: https://owasp.org/www-community/Source_Code_Reveals_Secret
static String fetchWikiRaw(String baseUrl, String query) {
pragent-bot commented 2026-08-18 01:33:14 +00:00
Review
Copy Link
Copy Source

[MEDIUM] The new fetchWikiRaw helper has no test, unlike the carefully tested bearer path in HttpFetcherTest; changed behavior (URL building, status handling, error path) is uncovered.

Fix: Add a unit test for URL construction/encoding and the non-200 path, using a stub Fetcher or by extracting the URL-building into a pure static method.

**[MEDIUM]** The new fetchWikiRaw helper has no test, unlike the carefully tested bearer path in HttpFetcherTest; changed behavior (URL building, status handling, error path) is uncovered. Fix: Add a unit test for URL construction/encoding and the non-200 path, using a stub Fetcher or by extracting the URL-building into a pure static method.
String url = baseUrl + "?action=query&list=search&srsearch=" + query;
pragent-bot commented 2026-08-18 01:33:14 +00:00
Review
Copy Link
Copy Source

[HIGH] The user query is concatenated into the URL raw, so spaces or '&', '#', '=' break the request and let a malicious query inject extra query parameters (URL/query injection).

Fix: URL-encode the query with URLEncoder.encode(query, StandardCharsets.UTF_8), as Wiki.search and Search.web already do.

String url = baseUrl + "?action=query&list=search&format=json&srlimit=1&srsearch="
                + URLEncoder.encode(query, StandardCharsets.UTF_8);

📎 ref: https://owasp.org/www-community/Injection_Flaws

**[HIGH]** The user query is concatenated into the URL raw, so spaces or '&', '#', '=' break the request and let a malicious query inject extra query parameters (URL/query injection). Fix: URL-encode the query with URLEncoder.encode(query, StandardCharsets.UTF_8), as Wiki.search and Search.web already do. ```java String url = baseUrl + "?action=query&list=search&format=json&srlimit=1&srsearch=" + URLEncoder.encode(query, StandardCharsets.UTF_8); ``` 📎 ref: https://owasp.org/www-community/Injection_Flaws
pragent-bot commented 2026-08-18 01:33:14 +00:00
Review
Copy Link
Copy Source

[LOW] baseUrl is concatenated with no validation or normalization; a trailing slash or missing scheme produces a malformed URL, unlike Search which normalizes baseUrl with replaceAll("/+$", "").

Fix: Normalize baseUrl (strip trailing slashes) and guard against null/blank before building the URL.

String base = baseUrl == null ? "" : baseUrl.replaceAll("/+$", "");
        String url = base + "?action=query&list=search&format=json&srlimit=1&srsearch="
                + URLEncoder.encode(query, StandardCharsets.UTF_8);
**[LOW]** baseUrl is concatenated with no validation or normalization; a trailing slash or missing scheme produces a malformed URL, unlike Search which normalizes baseUrl with replaceAll("/+$", ""). Fix: Normalize baseUrl (strip trailing slashes) and guard against null/blank before building the URL. ```java String base = baseUrl == null ? "" : baseUrl.replaceAll("/+$", ""); String url = base + "?action=query&list=search&format=json&srlimit=1&srsearch=" + URLEncoder.encode(query, StandardCharsets.UTF_8); ```
try {
HttpClient c = HttpClient.newHttpClient();
pragent-bot commented 2026-08-18 01:33:14 +00:00
Review
Copy Link
Copy Source

[MEDIUM] A fresh HttpClient.newHttpClient() is built per call with no connect or request timeout, bypassing the class's configured http client and timeoutSeconds; a slow wiki can hang the calling thread indefinitely.

Fix: Reuse the shared this.http client and set .timeout(Duration.ofSeconds(timeoutSeconds)) on the request, as get()/postJson() do.

HttpRequest req = HttpRequest.newBuilder(URI.create(url))
                    .timeout(Duration.ofSeconds(timeoutSeconds))
                    .header("User-Agent", AGENT)
                    .header("Authorization", "Bearer " + bearer)
                    .GET()
                    .build();
            HttpResponse<String> resp = http.send(req, HttpResponse.BodyHandlers.ofString());
**[MEDIUM]** A fresh HttpClient.newHttpClient() is built per call with no connect or request timeout, bypassing the class's configured http client and timeoutSeconds; a slow wiki can hang the calling thread indefinitely. Fix: Reuse the shared this.http client and set .timeout(Duration.ofSeconds(timeoutSeconds)) on the request, as get()/postJson() do. ```java HttpRequest req = HttpRequest.newBuilder(URI.create(url)) .timeout(Duration.ofSeconds(timeoutSeconds)) .header("User-Agent", AGENT) .header("Authorization", "Bearer " + bearer) .GET() .build(); HttpResponse<String> resp = http.send(req, HttpResponse.BodyHandlers.ofString()); ```
HttpRequest req = HttpRequest.newBuilder(URI.create(url))
.header("Authorization", "Bearer " + WIKI_API_KEY)
pragent-bot commented 2026-08-18 01:33:14 +00:00
Review
Copy Link
Copy Source

[HIGH] No User-Agent is sent; the class's own comment documents that pt.minecraft.wiki returns 403 to a default UA, so this helper will be rejected by the very wiki it targets.

Fix: Add the existing AGENT header (.header("User-Agent", AGENT)) to the request, matching get() and postJson().

HttpRequest req = HttpRequest.newBuilder(URI.create(url))
                    .header("User-Agent", AGENT)
                    .header("Authorization", "Bearer " + bearer)
                    .GET()
                    .build();
**[HIGH]** No User-Agent is sent; the class's own comment documents that pt.minecraft.wiki returns 403 to a default UA, so this helper will be rejected by the very wiki it targets. Fix: Add the existing AGENT header (.header("User-Agent", AGENT)) to the request, matching get() and postJson(). ```java HttpRequest req = HttpRequest.newBuilder(URI.create(url)) .header("User-Agent", AGENT) .header("Authorization", "Bearer " + bearer) .GET() .build(); ```
.GET()
.build();
HttpResponse<String> resp = c.send(req, HttpResponse.BodyHandlers.ofString());
if (resp.statusCode() == 200) {
return resp.body();
}
return "";
} catch (Exception e) {
e.printStackTrace();
pragent-bot commented 2026-08-18 01:33:14 +00:00
Review
Copy Link
Copy Source

[MEDIUM] e.printStackTrace() writes stack traces to stderr bypassing the plugin's warn sink, so failures are invisible to operators and inconsistent with the rest of HttpFetcher/Wiki which route errors through warn/IOException.

Fix: Throw or propagate via the caller's Consumer warn (as Wiki.lookup does) instead of swallowing with printStackTrace.

} catch (Exception e) {
            throw new IOException("Wiki search falhou: " + e.getMessage(), e);
        }
**[MEDIUM]** e.printStackTrace() writes stack traces to stderr bypassing the plugin's warn sink, so failures are invisible to operators and inconsistent with the rest of HttpFetcher/Wiki which route errors through warn/IOException. Fix: Throw or propagate via the caller's Consumer<String> warn (as Wiki.lookup does) instead of swallowing with printStackTrace. ```java } catch (Exception e) { throw new IOException("Wiki search falhou: " + e.getMessage(), e); } ```
return "";
}
}
private String body(HttpResponse<String> response) throws IOException { private String body(HttpResponse<String> response) throws IOException {
if (response.statusCode() / 100 != 2) { if (response.statusCode() / 100 != 2) {
throw new IOException("HTTP " + response.statusCode() + ": " throw new IOException("HTTP " + response.statusCode() + ": "