security: fix CodeQL stack-trace-exposure and URL-sanitization findings - #192
Merged
Conversation
web/app.py exposed raw exception text (`str(e)`) to API callers on every 5xx response across 12 handlers (py/stack-trace-exposure, 11 CodeQL alerts) — paths, SQL fragments, and config values could leak through error bodies. Add `_error_response()`/`_log_and_get_error_id()`: the real exception is still logged server-side via `logger.exception`, but callers now only see a generic message plus an opaque error_id for correlating support reports with server logs. WebConfig.host defaulted to "0.0.0.0" but is never actually read to bind a socket — the real bind address comes from web/app.py's --host CLI arg (already defaults to 127.0.0.1, gated behind an API key for wider binds) or an explicit --host in deployment tooling. Flip the dead default to match, so the config editor UI doesn't advertise an all-interfaces bind that was never wired up. tests/test_basic.py: CodeQL's py/incomplete-url-substring-sanitization flagged 4 assertions using `any(lit in x for x in list)` / `lit in list` on domain- and URL-shaped string literals — a false positive since these are exact list-membership checks in test assertions, not sanitization logic. Rewrote as direct membership and a set-equality comparison, which are both stronger assertions (catch missing/extra entries) and no longer match the query's heuristic. Verified via local CodeQL CLI 2.25.6 (same version/query suite the repo's default setup uses): 15 alerts -> 0. Full pytest suite passes (701 passed; excluded failures are network/environment issues unrelated to this change — embedding model download blocked, plus test_osint_collectors.py's live network tests). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqYpmV8TABMzq27HBTLtNP
rolandpg
marked this pull request as ready for review
July 7, 2026 20:21
| # tooling (e.g. the Docker image's 0.0.0.0). Kept here for the config | ||
| # editor UI; default matches the safe CLI default rather than | ||
| # advertising an all-interfaces bind. | ||
| host: str = "127.0.0.1" |
Contributor
There was a problem hiding this comment.
🔍 Documentation still advertises 0.0.0.0 as the default for web.host
The reference documentation at docs/reference/configuration.md:386-394 still shows host: str = "0.0.0.0" in the code snippet and 0.0.0.0 in the defaults table, but the actual dataclass default was changed to 127.0.0.1 in this PR. The config editor UI tooltip descriptions at web/ui/js/views/configuration.js:77 and web/templates/config_editor.html:200 also describe this as "Bind host for the web interface" without noting it's cosmetic-only. This drift could confuse operators consulting the docs.
Was this helpful? React with 👍 or 👎 to provide feedback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Clears all 15 open CodeQL alerts (verified locally with CodeQL CLI 2.25.6, same version/default suite the repo uses — this session's GitHub App token doesn't have
security_eventsread access to confirm against the live dashboard).Related issue
Follow-up to manual triage of https://github.com/ThreatRecall/zettelforge/security/code-scanning
Changes
web/app.py— 11py/stack-trace-exposurealerts: every 5xx handler returned rawstr(e)to API callers, potentially leaking paths, SQL fragments, or config values. Added_error_response()/_log_and_get_error_id(): the real exception is still logged server-side vialogger.exception, but the client now only sees a generic message plus an opaqueerror_idfor correlating a support report with server logs. Swept all 12 sites with this pattern (including the per-item error in the bulk-ingest endpoint, which CodeQL's query didn't flag but has the identical shape).src/zettelforge/config.py—WebConfig.hostdefaulted to"0.0.0.0"but is never actually read to bind a socket; the real bind address comes fromweb/app.py's--hostCLI arg (already defaults to127.0.0.1, gated behind an API key for wider binds) or an explicit--hostin deployment tooling (e.g. the Docker image passes0.0.0.0directly). Flipped the dead default so the config editor UI doesn't advertise an all-interfaces bind that isn't actually wired up.tests/test_basic.py— 4py/incomplete-url-substring-sanitizationfalse positives: CodeQL's heuristic flags domain/URL-shaped string literals used within, even for exact list-membership assertions in test code. Rewrote as direct membership checks and a set-equality comparison — both are stronger assertions (they also catch unexpected extras) and no longer match the query's pattern.Testing
pytest tests/ -v) — 701 passed (excluded: an unrelated network-blocked embedding-model download test, andtest_osint_collectors.py's live-network tests, both environment issues in this sandbox unrelated to this change)ruff check src/zettelforge/)Verified via local CodeQL rebuild + rescan: 15 alerts → 0 (all three configured languages — Python, GitHub Actions, JavaScript/TypeScript — checked; the latter two had zero alerts before and after).
Note
While rescanning, CodeQL also flagged
examples/athf_bridge.py:48as unparseable (SyntaxError: '{' was never closed), which silently excludes that file from every scan, including the real GitHub Actions run. Pre-existing, unrelated to this change — flagging here rather than fixing, since it's out of scope for this PR.Generated by Claude Code