[PR #14] [MERGED] Fix 5 code scanning security vulnerabilities (XSS, API key exposure, error leakage, localStorage, CORS) #11

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

📋 Pull Request Information

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

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

📝 Commits (2)

6c858b7 Initial plan
6f5b018 Fix 5 security vulnerabilities: XSS, API key exposure, error info leak, localStorage, CORS

📊 Changes

3 files changed (+40 additions, -18 deletions)

View changed files

📝 geointel/cli.py (+1 -1)
📝 geointel/web_server.py (+23 -10)
📝 geointel_ui_template/index.html (+16 -7)

📄 Description

Add Web Crypto API utilities (_getCryptoKey, _encryptValue, _decryptValue) using AES-GCM with PBKDF2-derived key
Make getStoredApiKey async — decrypts stored value before returning
Make storeApiKey async — encrypts key before calling sessionStorage.setItem
Update updateApiKeyIndicator to be async and await getStoredApiKey()
Update showApiKeyModal to be async and await getStoredApiKey()
Update processImage to await getStoredApiKey()
Update saveApiKeyBtn click handler to be async and await storeApiKey()
Update window.load handler to be async and await async calls

Original prompt

Security Fixes — Code Scanning Alerts

Fix all 5 security vulnerabilities identified by code scanning in the atiilla/GeoIntel repository.

Fix 1 — XSS via `innerHTML` with unsanitized AI output (CWE-79)

File: geointel_ui_template/index.html — Line 995

Problem: The interpretation field from the Gemini API response is injected into the DOM using .innerHTML. The formatAnalysis() function already escapes &, <, > characters but the result is then set via .innerHTML. This is fine since it's escaped, BUT the current code path does NOT escape before setting innerHTML in all places. Make the fix robust.

Fix: The formatAnalysis() function already does basic escaping. Ensure the innerHTML assignment on line 995 only ever receives output from formatAnalysis() (which escapes &, <, >), and add a comment noting this is intentional safe HTML (pre-escaped). Also add a sanitizeText() helper that uses textContent assignment trick for proper escaping, and update formatAnalysis() to use it instead of manual regex replacement, making it more robust against future bypasses.

Replace the manual regex escaping in formatAnalysis:

// BEFORE (fragile - misses quotes and other vectors):
const safe = text.replace(/&/g,'&amp;').replace(/</g,'&lt;').replace(/>/g,'&gt;');

// AFTER (use the DOM's own escaping engine):
function escapeHtml(str) {
    const div = document.createElement('div');
    div.textContent = str;
    return div.innerHTML;
}

Then use escapeHtml() inside formatAnalysis() instead of the regex chain.

Fix 2 — API key transmitted in request body (CWE-319)

File: geointel/web_server.py and geointel_ui_template/index.html

Problem: The Gemini API key is passed from the browser in every /api/analyze POST request body (api_key field). This exposes the key in HTTP logs, browser history, and network traffic.

Fix: Add support for reading the API key from the X-Api-Key HTTP request header instead of (or in addition to) the request body. Update web_server.py to prefer the header over the body field:

# In web_server.py analyze_image():
api_key = request.headers.get('X-Api-Key') or data.get('api_key')

Update index.html to send the API key as a header instead of in the body:

// In processImage() fetch call:
headers: { 
    'Content-Type': 'application/json',
    'X-Api-Key': apiKey   // Send as header, not in body
},
body: JSON.stringify({
    image: e.target.result,
    model: getSelectedModel(),
    context: contextValue || null,
    guess: guessValue || null
    // api_key removed from body
})

Also update the modal note from "Stored locally in your browser. Never leaves your device." to "Stored in session only. Sent securely via request header." since we're also changing to sessionStorage (Fix 4).

Fix 3 — Internal error details leaked in 500 responses (CWE-209)

File: geointel/web_server.py

Problem: All exception handlers return str(e) in the details field, leaking internal error messages, file paths, and stack info to clients.

Fix: Remove str(e) from all 500-level JSON error responses. Keep logging str(e) server-side for debugging but return a generic message to the client. Apply to the generic Exception handler in analyze_image() and in reverse_image_search():

# BEFORE:
except Exception as e:
    logger.error(f"Unexpected error in analyze endpoint: {e}", exc_info=True)
    return jsonify({
        'error': 'Internal server error',
        'details': str(e)
    }), 500

# AFTER:
except Exception as e:
    logger.error(f"Unexpected error in analyze endpoint: {e}", exc_info=True)
    return jsonify({
        'error': 'Internal server error',
        'details': 'An unexpected error occurred. Please try again.'
    }), 500

Apply the same change to the reverse_image_search endpoint's generic exception handler.

Also update the internal_error error handler (line ~230):

@app.errorhandler(500)
def internal_error(e):
    logger.error(f"Internal server error: {e}")
    return jsonify({
        'error': 'Internal server error',
        'details': 'An unexpected error occurred.'
    }), 500

Fix 4 — API key persisted in `localStorage` (CWE-312)

File: geointel_ui_template/index.html

Problem: The Gemini API key is stored in localStorage which persists indefinitely and is accessible to any JavaScript running on the page.

Fix: Change localStorage to sessionStorage for the API key so it only exists for the current browser session and is cleared when the tab/browser is closed:

// BEFORE:
function getStoredApiKey() { return localStorage.getItem(STORAGE_KEYS.API_KEY); }
function storeApiKey(key) { localStorage.setItem(STORAGE_KEYS.API_KEY, key); updateApiKeyIndicator(); }
function getSelectedModel() { return localStorage.getItem(STORAGE_KEYS.MODEL) || 'gemini-3-flash-previ...

</details>



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

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

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

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. [Learn more about Advanced Security.](https://gh.io/cca-advanced-security)

---

<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/14 **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-vulnerabilities` --- ### 📝 Commits (2) - [`6c858b7`](https://github.com/atiilla/GeoIntel/commit/6c858b742ff51bd5e4f92be96287e396bf2c0d8f) Initial plan - [`6f5b018`](https://github.com/atiilla/GeoIntel/commit/6f5b018508947ae3c77d693e34de30c709574c48) Fix 5 security vulnerabilities: XSS, API key exposure, error info leak, localStorage, CORS ### 📊 Changes **3 files changed** (+40 additions, -18 deletions) <details> <summary>View changed files</summary> 📝 `geointel/cli.py` (+1 -1) 📝 `geointel/web_server.py` (+23 -10) 📝 `geointel_ui_template/index.html` (+16 -7) </details> ### 📄 Description - [ ] Add Web Crypto API utilities (`_getCryptoKey`, `_encryptValue`, `_decryptValue`) using AES-GCM with PBKDF2-derived key - [ ] Make `getStoredApiKey` async — decrypts stored value before returning - [ ] Make `storeApiKey` async — encrypts key before calling `sessionStorage.setItem` - [ ] Update `updateApiKeyIndicator` to be async and `await getStoredApiKey()` - [ ] Update `showApiKeyModal` to be async and `await getStoredApiKey()` - [ ] Update `processImage` to `await getStoredApiKey()` - [ ] Update `saveApiKeyBtn` click handler to be async and `await storeApiKey()` - [ ] Update `window.load` handler to be async and await async calls  <details> <summary>Original prompt</summary> ## Security Fixes — Code Scanning Alerts Fix all 5 security vulnerabilities identified by code scanning in the `atiilla/GeoIntel` repository. --- ### Fix 1 — XSS via `innerHTML` with unsanitized AI output (CWE-79) **File:** `geointel_ui_template/index.html` — Line 995 **Problem:** The `interpretation` field from the Gemini API response is injected into the DOM using `.innerHTML`. The `formatAnalysis()` function already escapes `&`, `<`, `>` characters but the result is then set via `.innerHTML`. This is fine since it's escaped, BUT the current code path does NOT escape before setting innerHTML in all places. Make the fix robust. **Fix:** The `formatAnalysis()` function already does basic escaping. Ensure the `innerHTML` assignment on line 995 only ever receives output from `formatAnalysis()` (which escapes `&`, `<`, `>`), and add a comment noting this is intentional safe HTML (pre-escaped). Also add a `sanitizeText()` helper that uses `textContent` assignment trick for proper escaping, and update `formatAnalysis()` to use it instead of manual regex replacement, making it more robust against future bypasses. Replace the manual regex escaping in `formatAnalysis`: ```javascript // BEFORE (fragile - misses quotes and other vectors): const safe = text.replace(/&/g,'&').replace(/</g,'<').replace(/>/g,'>'); // AFTER (use the DOM's own escaping engine): function escapeHtml(str) { const div = document.createElement('div'); div.textContent = str; return div.innerHTML; } ``` Then use `escapeHtml()` inside `formatAnalysis()` instead of the regex chain. --- ### Fix 2 — API key transmitted in request body (CWE-319) **File:** `geointel/web_server.py` and `geointel_ui_template/index.html` **Problem:** The Gemini API key is passed from the browser in every `/api/analyze` POST request body (`api_key` field). This exposes the key in HTTP logs, browser history, and network traffic. **Fix:** Add support for reading the API key from the `X-Api-Key` HTTP request header instead of (or in addition to) the request body. Update `web_server.py` to prefer the header over the body field: ```python # In web_server.py analyze_image(): api_key = request.headers.get('X-Api-Key') or data.get('api_key') ``` Update `index.html` to send the API key as a header instead of in the body: ```javascript // In processImage() fetch call: headers: { 'Content-Type': 'application/json', 'X-Api-Key': apiKey // Send as header, not in body }, body: JSON.stringify({ image: e.target.result, model: getSelectedModel(), context: contextValue || null, guess: guessValue || null // api_key removed from body }) ``` Also update the modal note from "Stored locally in your browser. Never leaves your device." to "Stored in session only. Sent securely via request header." since we're also changing to sessionStorage (Fix 4). --- ### Fix 3 — Internal error details leaked in 500 responses (CWE-209) **File:** `geointel/web_server.py` **Problem:** All exception handlers return `str(e)` in the `details` field, leaking internal error messages, file paths, and stack info to clients. **Fix:** Remove `str(e)` from all 500-level JSON error responses. Keep logging `str(e)` server-side for debugging but return a generic message to the client. Apply to the generic `Exception` handler in `analyze_image()` and in `reverse_image_search()`: ```python # BEFORE: except Exception as e: logger.error(f"Unexpected error in analyze endpoint: {e}", exc_info=True) return jsonify({ 'error': 'Internal server error', 'details': str(e) }), 500 # AFTER: except Exception as e: logger.error(f"Unexpected error in analyze endpoint: {e}", exc_info=True) return jsonify({ 'error': 'Internal server error', 'details': 'An unexpected error occurred. Please try again.' }), 500 ``` Apply the same change to the `reverse_image_search` endpoint's generic exception handler. Also update the `internal_error` error handler (line ~230): ```python @app.errorhandler(500) def internal_error(e): logger.error(f"Internal server error: {e}") return jsonify({ 'error': 'Internal server error', 'details': 'An unexpected error occurred.' }), 500 ``` --- ### Fix 4 — API key persisted in `localStorage` (CWE-312) **File:** `geointel_ui_template/index.html` **Problem:** The Gemini API key is stored in `localStorage` which persists indefinitely and is accessible to any JavaScript running on the page. **Fix:** Change `localStorage` to `sessionStorage` for the API key so it only exists for the current browser session and is cleared when the tab/browser is closed: ```javascript // BEFORE: function getStoredApiKey() { return localStorage.getItem(STORAGE_KEYS.API_KEY); } function storeApiKey(key) { localStorage.setItem(STORAGE_KEYS.API_KEY, key); updateApiKeyIndicator(); } function getSelectedModel() { return localStorage.getItem(STORAGE_KEYS.MODEL) || 'gemini-3-flash-previ... </details>  *This pull request was created from Copilot chat.* >  --- 🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. [Learn more about Advanced Security.](https://gh.io/cca-advanced-security) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>