starred/GeoIntel
1
0
Fork 0
mirror of https://github.com/atiilla/GeoIntel.git synced 2026-04-26 16:15:57 +03:00
Code Issues 2 Projects Releases Packages Wiki Activity

[PR #13] [MERGED] Security & code quality hardening: API key exposure, CORS, path traversal, temp file leak, input limits #13

New issue
Closed
opened 2026-03-15 11:26:30 +03:00 by kerem · 0 comments
kerem commented 2026-03-15 11:26:30 +03:00
Owner
Copy link

📋 Pull Request Information

Original PR: https://github.com/atiilla/GeoIntel/pull/13
Author: @Copilot
Created: 3/9/2026
Status: ✅ Merged
Merged: 3/9/2026
Merged by: @atiilla

Base: main ← Head: copilot/fix-security-issues

📝 Commits (2)

  • 4fb472a Initial plan
  • 8268e5c security: fix API key exposure, CORS, path traversal, temp file cleanup, input limits, debug mode, and unused imports

📊 Changes

3 files changed (+57 additions, -37 deletions)

View changed files

📝 geointel/api_client.py (+4 -3)
📝 geointel/image_processor.py (+1 -1)
📝 geointel/web_server.py (+52 -33)

📄 Description

Several security and code quality issues identified in code review — API key leaking via URL, overly permissive CORS, unguarded static file serving, temp file resource leak, unbounded prompt inputs, and minor code hygiene issues.

geointel/api_client.py

  • API key moved from URL query param to x-goog-api-key header — prevents key exposure in server logs, proxy logs, and browser history
  • Exception chaining preserved with raise APIError(...) from e
# Before
def _build_endpoint_url(self) -> str:
    return f"{self.base_url}/{self.model}:generateContent?key={self.api_key}"

# After
def _build_endpoint_url(self) -> str:
    return f"{self.base_url}/{self.model}:generateContent"

def _get_request_headers(self) -> Dict[str, str]:
    return {"Content-Type": "application/json", "x-goog-api-key": self.api_key}

geointel/web_server.py

  • CORS restricted to localhost — CORS(app) → CORS(app, origins=["http://127.0.0.1:5000", "http://localhost:5000"])
  • Static file allowlist — catch-all /<path:filename> route now rejects requests for extensions outside html|css|js|ico|png|jpg|jpeg|svg|woff|woff2|ttf|map
  • Temp file cleanup guaranteed — file lifecycle wrapped in try/finally; previously leaked on any exception between write and cleanup
  • Input length capped — context_info and location_guess truncated to 500 chars (MAX_CONTEXT_LENGTH) before being forwarded to the AI prompt
  • Debug mode disabled — __main__ block changed to run_server(debug=False)
  • Removed unused imports — render_template, Optional

geointel/image_processor.py

  • Removed unused import — Union
Original prompt

Security & Code Quality Fixes

A code review identified the following issues that need to be addressed. Please fix all of them in a single PR.

🔴 Critical Security Issues

1. API Key in URL Query String — geointel/api_client.py line 39

The Gemini API key is appended as a URL query parameter, which means it appears in server logs, browser history, and proxy logs.

Current:

def _build_endpoint_url(self) -> str:
    return f"{self.base_url}/{self.model}:generateContent?key={self.api_key}"

Fix: Move the API key to the x-goog-api-key HTTP request header instead, and remove it from the URL:

def _build_endpoint_url(self) -> str:
    return f"{self.base_url}/{self.model}:generateContent"

def _get_request_headers(self) -> Dict[str, str]:
    return {
        "Content-Type": "application/json",
        "x-goog-api-key": self.api_key
    }

2. Overly Broad CORS Policy — geointel/web_server.py line 22

CORS(app) with no arguments enables CORS for all origins.

Fix: Restrict to localhost only since this is a local tool:

CORS(app, origins=["http://127.0.0.1:5000", "http://localhost:5000"])

3. Path Traversal Risk via Catch-All Static File Route — geointel/web_server.py lines 39–41

The catch-all route /<path:filename> could serve arbitrary files. Add an allowlist of safe extensions:

import re

ALLOWED_STATIC_EXTENSIONS = re.compile(r'\.(html|css|js|ico|png|jpg|jpeg|svg|woff|woff2|ttf|map)$', re.IGNORECASE)

@app.route('/<path:filename>')
def serve_static(filename):
    if not ALLOWED_STATIC_EXTENSIONS.search(filename):
        return jsonify({'error': 'Not found'}), 404
    return send_from_directory(app.template_folder, filename)

🟡 Medium Issues

4. Temporary Files Not Cleaned Up on Error — geointel/web_server.py

If an exception occurs after saving the temp file but before the cleanup block, the file is never deleted.

Fix: Wrap the temp file lifecycle in a try/finally block so cleanup always happens:

temp_path = None
try:
    with tempfile.NamedTemporaryFile(delete=False, suffix=suffix, dir=app.config['UPLOAD_FOLDER']) as f:
        f.write(image_bytes)
        temp_path = f.name

    result = geointel.locate(
        image_path=temp_path,
        context_info=context_info,
        location_guess=location_guess
    )
finally:
    if temp_path and os.path.exists(temp_path):
        try:
            os.unlink(temp_path)
        except OSError as e:
            logger.warning(f"Failed to clean up temporary file {temp_path}: {e}")

5. No Input Length Limits on Prompt Fields — geointel/web_server.py

context_info and location_guess are passed directly to the AI prompt with no length restriction.

Fix: Add reasonable length caps:

MAX_CONTEXT_LENGTH = 500
context_raw = data.get('context')
guess_raw = data.get('guess')
context_info = str(context_raw)[:MAX_CONTEXT_LENGTH] if context_raw else None
location_guess = str(guess_raw)[:MAX_CONTEXT_LENGTH] if guess_raw else None

6. debug=True in __main__ block — geointel/web_server.py line 240

Running web_server.py directly enables Flask's interactive debugger, which is dangerous.

Fix:

if __name__ == '__main__':
    run_server(debug=False)

7. Preserve Exception Chain — geointel/api_client.py line 200

Use raise ... from e to preserve the original exception chain for easier debugging:

raise APIError(f"Unexpected error: {e}") from e

🟢 Minor / Code Quality Issues

8. Remove Unused Imports

  • geointel/web_server.py line 6: Remove render_template and Optional (unused)
  • geointel/image_processor.py line 4: Remove Union (unused)

Summary of files to change:

  • geointel/api_client.py — Issues 1, 7
  • geointel/web_server.py — Issues 2, 3, 4, 5, 6, 8
  • geointel/image_processor.py — Issue 8

Please make all these changes in a single pull request with a clear commit message.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Code review

Assistant: Here is a thorough code review of the atiilla/GeoIntel repository. The project is well-structured overall, but there are several security, reliability, and code quality issues worth addressing.

🔍 Code Review: atiilla/GeoIntel

🏗️ Architecture & Structure

The project is cleanly modularized:

  • api_client.py — Gemini API communication
  • image_processor.py — Image loading, validation, encoding
  • geointel.py — Core orchestration
  • web_server.py — Flask web API
  • cli.py — CLI interface
  • config.py — Central configuration
  • geointel_ui_template/ — Frontend (HTML + JS)

This separation of concerns is a strength. ✅

🔴 Critical Issues

1. API Key Transmitted in URL Query String

File: geointel/api_client.py, line 39


</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

<!-- START COPILOT CODING AGENT TIPS -->
---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey).


---

<sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
## 📋 Pull Request Information **Original PR:** https://github.com/atiilla/GeoIntel/pull/13 **Author:** [@Copilot](https://github.com/apps/copilot-swe-agent) **Created:** 3/9/2026 **Status:** ✅ Merged **Merged:** 3/9/2026 **Merged by:** [@atiilla](https://github.com/atiilla) **Base:** `main` ← **Head:** `copilot/fix-security-issues` --- ### 📝 Commits (2) - [`4fb472a`](https://github.com/atiilla/GeoIntel/commit/4fb472ae3c288c80ba6ac5d1d1f16b9dd270a5b0) Initial plan - [`8268e5c`](https://github.com/atiilla/GeoIntel/commit/8268e5c010d0ce7e9e3cb220e5308ab7f0121e4d) security: fix API key exposure, CORS, path traversal, temp file cleanup, input limits, debug mode, and unused imports ### 📊 Changes **3 files changed** (+57 additions, -37 deletions) <details> <summary>View changed files</summary> 📝 `geointel/api_client.py` (+4 -3) 📝 `geointel/image_processor.py` (+1 -1) 📝 `geointel/web_server.py` (+52 -33) </details> ### 📄 Description Several security and code quality issues identified in code review — API key leaking via URL, overly permissive CORS, unguarded static file serving, temp file resource leak, unbounded prompt inputs, and minor code hygiene issues. ## `geointel/api_client.py` - **API key moved from URL query param to `x-goog-api-key` header** — prevents key exposure in server logs, proxy logs, and browser history - **Exception chaining preserved** with `raise APIError(...) from e` ```python # Before def _build_endpoint_url(self) -> str: return f"{self.base_url}/{self.model}:generateContent?key={self.api_key}" # After def _build_endpoint_url(self) -> str: return f"{self.base_url}/{self.model}:generateContent" def _get_request_headers(self) -> Dict[str, str]: return {"Content-Type": "application/json", "x-goog-api-key": self.api_key} ``` ## `geointel/web_server.py` - **CORS restricted to localhost** — `CORS(app)` → `CORS(app, origins=["http://127.0.0.1:5000", "http://localhost:5000"])` - **Static file allowlist** — catch-all `/<path:filename>` route now rejects requests for extensions outside `html|css|js|ico|png|jpg|jpeg|svg|woff|woff2|ttf|map` - **Temp file cleanup guaranteed** — file lifecycle wrapped in `try/finally`; previously leaked on any exception between write and cleanup - **Input length capped** — `context_info` and `location_guess` truncated to 500 chars (`MAX_CONTEXT_LENGTH`) before being forwarded to the AI prompt - **Debug mode disabled** — `__main__` block changed to `run_server(debug=False)` - **Removed unused imports** — `render_template`, `Optional` ## `geointel/image_processor.py` - **Removed unused import** — `Union` <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> ## Security & Code Quality Fixes A code review identified the following issues that need to be addressed. Please fix all of them in a single PR. --- ### 🔴 Critical Security Issues #### 1. API Key in URL Query String — `geointel/api_client.py` line 39 The Gemini API key is appended as a URL query parameter, which means it appears in server logs, browser history, and proxy logs. **Current:** ```python def _build_endpoint_url(self) -> str: return f"{self.base_url}/{self.model}:generateContent?key={self.api_key}" ``` **Fix:** Move the API key to the `x-goog-api-key` HTTP request header instead, and remove it from the URL: ```python def _build_endpoint_url(self) -> str: return f"{self.base_url}/{self.model}:generateContent" def _get_request_headers(self) -> Dict[str, str]: return { "Content-Type": "application/json", "x-goog-api-key": self.api_key } ``` --- #### 2. Overly Broad CORS Policy — `geointel/web_server.py` line 22 `CORS(app)` with no arguments enables CORS for all origins. **Fix:** Restrict to localhost only since this is a local tool: ```python CORS(app, origins=["http://127.0.0.1:5000", "http://localhost:5000"]) ``` --- #### 3. Path Traversal Risk via Catch-All Static File Route — `geointel/web_server.py` lines 39–41 The catch-all route `/<path:filename>` could serve arbitrary files. Add an allowlist of safe extensions: ```python import re ALLOWED_STATIC_EXTENSIONS = re.compile(r'\.(html|css|js|ico|png|jpg|jpeg|svg|woff|woff2|ttf|map)$', re.IGNORECASE) @app.route('/<path:filename>') def serve_static(filename): if not ALLOWED_STATIC_EXTENSIONS.search(filename): return jsonify({'error': 'Not found'}), 404 return send_from_directory(app.template_folder, filename) ``` --- ### 🟡 Medium Issues #### 4. Temporary Files Not Cleaned Up on Error — `geointel/web_server.py` If an exception occurs after saving the temp file but before the cleanup block, the file is never deleted. **Fix:** Wrap the temp file lifecycle in a `try/finally` block so cleanup always happens: ```python temp_path = None try: with tempfile.NamedTemporaryFile(delete=False, suffix=suffix, dir=app.config['UPLOAD_FOLDER']) as f: f.write(image_bytes) temp_path = f.name result = geointel.locate( image_path=temp_path, context_info=context_info, location_guess=location_guess ) finally: if temp_path and os.path.exists(temp_path): try: os.unlink(temp_path) except OSError as e: logger.warning(f"Failed to clean up temporary file {temp_path}: {e}") ``` --- #### 5. No Input Length Limits on Prompt Fields — `geointel/web_server.py` `context_info` and `location_guess` are passed directly to the AI prompt with no length restriction. **Fix:** Add reasonable length caps: ```python MAX_CONTEXT_LENGTH = 500 context_raw = data.get('context') guess_raw = data.get('guess') context_info = str(context_raw)[:MAX_CONTEXT_LENGTH] if context_raw else None location_guess = str(guess_raw)[:MAX_CONTEXT_LENGTH] if guess_raw else None ``` --- #### 6. `debug=True` in `__main__` block — `geointel/web_server.py` line 240 Running `web_server.py` directly enables Flask's interactive debugger, which is dangerous. **Fix:** ```python if __name__ == '__main__': run_server(debug=False) ``` --- #### 7. Preserve Exception Chain — `geointel/api_client.py` line 200 Use `raise ... from e` to preserve the original exception chain for easier debugging: ```python raise APIError(f"Unexpected error: {e}") from e ``` --- ### 🟢 Minor / Code Quality Issues #### 8. Remove Unused Imports - `geointel/web_server.py` line 6: Remove `render_template` and `Optional` (unused) - `geointel/image_processor.py` line 4: Remove `Union` (unused) --- ### Summary of files to change: - `geointel/api_client.py` — Issues 1, 7 - `geointel/web_server.py` — Issues 2, 3, 4, 5, 6, 8 - `geointel/image_processor.py` — Issue 8 Please make all these changes in a single pull request with a clear commit message. The following is the prior conversation context from the user's chat exploration (may be truncated): User: Code review Assistant: Here is a thorough **code review** of the `atiilla/GeoIntel` repository. The project is well-structured overall, but there are several security, reliability, and code quality issues worth addressing. --- ## 🔍 Code Review: `atiilla/GeoIntel` ### 🏗️ Architecture & Structure The project is cleanly modularized: - `api_client.py` — Gemini API communication - `image_processor.py` — Image loading, validation, encoding - `geointel.py` — Core orchestration - `web_server.py` — Flask web API - `cli.py` — CLI interface - `config.py` — Central configuration - `geointel_ui_template/` — Frontend (HTML + JS) This separation of concerns is a **strength**. ✅ --- ### 🔴 Critical Issues #### 1. API Key Transmitted in URL Query String **File:** `geointel/api_client.py`, line 39 ```python name=geointel/api_client.py url=... </details> <!-- START COPILOT CODING AGENT SUFFIX --> *This pull request was created from Copilot chat.* > <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
kerem 2026-03-15 11:26:30 +03:00
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
nextjs
No results found.
Labels
Clear labels   
pull-request

Mirrored from GitHub Pull Request

No labels
pull-request
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/GeoIntel#13
No description provided.