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
gitea_admin commented 2026-08-17 23:40:30 +00:00
Owner
Copy Link
Copy Source

Adds a small convenience helper to hit the MediaWiki search API from the new /wiki search command. Reuses HttpFetcher's HttpClient setup.

This is a real change to review — please check it carefully before merge.

Adds a small convenience helper to hit the MediaWiki search API from the new `/wiki search` command. Reuses HttpFetcher's HttpClient setup. This is a real change to review — please check it carefully before merge.
gitea_admin added 1 commit 2026-08-17 23:40:31 +00:00
feat: add fetchWikiRaw helper for the wiki search box a2df6570e9
pragent-bot bot added the AI-REVIEW label 2026-08-17 23:40:32 +00:00
pragent-bot bot reviewed 2026-08-18 01:33:14 +00:00
pragent-bot bot left a comment
Copy Link
Copy Source

🤖 AI Review · pragent pilot · glm-5.2:cloud · a2df6570

Adds a fetchWikiRaw helper that hits the MediaWiki search API. The change is small but security-sensitive: it hardcodes an API bearer token in source, concatenates the raw user query into the URL without encoding, and bypasses the class's configured HttpClient (omitting the User-Agent MediaWiki requires and any timeout). No test covers the new behavior. 1 critical, 2 high, 3 medium, 1 low.

7 inline comment(s) posted below.

🤖 **AI Review** · pragent pilot · glm-5.2:cloud · `a2df6570` Adds a fetchWikiRaw helper that hits the MediaWiki search API. The change is small but security-sensitive: it hardcodes an API bearer token in source, concatenates the raw user query into the URL without encoding, and bypasses the class's configured HttpClient (omitting the User-Agent MediaWiki requires and any timeout). No test covers the new behavior. 1 critical, 2 high, 3 medium, 1 low. _7 inline comment(s) posted below._ <!-- pragent:sha=a2df6570e945aa0f45c5858277069d1529e504df -->
src/main/java/dev/marcospaulo/canalhandia/HttpFetcher.java
@@ -93,0 +95,4 @@
* 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 bot commented 2026-08-18 01:33:14 +00:00
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
src/main/java/dev/marcospaulo/canalhandia/HttpFetcher.java
@@ -93,0 +96,4 @@
* user's query and grabs it. Convenience helper used by the search command.
*/
static final String WIKI_API_KEY = "sk-mediawiki-9f2a7c4e1b8d30f6";
static String fetchWikiRaw(String baseUrl, String query) {
pragent-bot bot commented 2026-08-18 01:33:14 +00:00
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.
src/main/java/dev/marcospaulo/canalhandia/HttpFetcher.java
@@ -93,0 +97,4 @@
*/
static final String WIKI_API_KEY = "sk-mediawiki-9f2a7c4e1b8d30f6";
static String fetchWikiRaw(String baseUrl, String query) {
String url = baseUrl + "?action=query&list=search&srsearch=" + query;
pragent-bot bot commented 2026-08-18 01:33:14 +00:00
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 bot commented 2026-08-18 01:33:14 +00:00
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); ```
src/main/java/dev/marcospaulo/canalhandia/HttpFetcher.java
@@ -93,0 +99,4 @@
static String fetchWikiRaw(String baseUrl, String query) {
String url = baseUrl + "?action=query&list=search&srsearch=" + query;
try {
HttpClient c = HttpClient.newHttpClient();
pragent-bot bot commented 2026-08-18 01:33:14 +00:00
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()); ```
src/main/java/dev/marcospaulo/canalhandia/HttpFetcher.java
@@ -93,0 +101,4 @@
try {
HttpClient c = HttpClient.newHttpClient();
HttpRequest req = HttpRequest.newBuilder(URI.create(url))
.header("Authorization", "Bearer " + WIKI_API_KEY)
pragent-bot bot commented 2026-08-18 01:33:14 +00:00
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(); ```
src/main/java/dev/marcospaulo/canalhandia/HttpFetcher.java
@@ -93,0 +110,4 @@
}
return "";
} catch (Exception e) {
e.printStackTrace();
pragent-bot bot commented 2026-08-18 01:33:14 +00:00
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); } ```
masi closed this pull request 2026-08-20 13:03:40 +00:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
pragent-bot
Labels
Clear labels
AI-REVIEW
pragent AI review
 AI-USAGE
AI-USAGE label
No Label AI-REVIEW
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
pragent-bot (pragent AI review bot) gitea_admin
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: gitea_admin/canalhandia#2
Delete Branch "wiki-search-buggy-10029"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?