cw2007/POSA_Copyrighter

changukyu d008e35c2e docs: add 2026-06-22 multi-agent project review report

66 verified findings (0 CRITICAL, 6 HIGH, 43 MEDIUM, 17 LOW) with a
prioritized remediation roadmap. Generated from an adversarially-verified
multi-agent review.

2026-06-22 14:00:45 +09:00

121 KiB

Raw Blame History

Copyrighter 프로젝트 코드 리뷰 — 2026-06-22

다중 에이전트 리뷰(12개 유닛, 도메인 전문 리뷰어) · 모든 CRITICAL/HIGH 발견은 적대적 검증(실제 코드 기반 반증 시도)을 거침. 검증 통과 발견 66건 (적대적 검증으로 반증 제거 3건). CRITICAL 0 · HIGH 6 · MEDIUM 43 · LOW 17.

⚠️ arch-concurrency(아키텍처/동시성) 유닛은 일시적 레이트리밋으로 실패했습니다. 해당 영역은 sec-sql/bug-store 유닛이 부분적으로 커버했으며, 재실행을 권장합니다.

1. 요약 (Executive Summary)

Overall Health: Solid foundation, no CRITICAL issues, a handful of real HIGH-severity correctness bugs

The copyrighter project is in good shape post-remediation. Across 66 verified findings (3 refuted and dropped) there are 0 CRITICAL and 6 HIGH issues — the remaining 43 MEDIUM / 17 LOW are mostly defence-in-depth, maintainability, and test-coverage gaps rather than live exploits. The prior review's hardening held up: wildcard CORS is gone, auth covers all data routes, the request body cap + SQLite WAL/busy_timeout are in place, SSRF is guarded with redirect re-validation, path traversal is blocked, and the Python god-file was split into mixins.

What is genuinely solid

Security perimeter (HTTP/SSRF/SQL/XSS units all confirm this): parameterized queries everywhere with a STORE_TABLES allowlist, escapeHtml() applied consistently to innerHTML data, safeUrl() allowlisting external URLs, SSRF guard with per-redirect-hop re-validation, untrusted media served with strict CSP+nosniff+no-store. No hardcoded secrets, no eval/new Function, no unsafe deserialization (all payloads are plain JSON).
Air-gap posture (bld-airgap): runtime deps pinned and transitively complete in a single pip download; GUI loads only local assets (bundled font, no CDN); no telemetry/auto-update anywhere.
Test breadth (tst-coverage): 390 test functions / 35 files; Playwright E2E for decision-flow + upload; record_decision atomicity already pinned with a rollback test.

Biggest risks (all confirmed HIGH)

Data-integrity / audit-trail bugs in the store. mark_evidence_status performs a non-atomic read-modify-write-audit across four separate connections with no _write_lock and no _transaction() — concurrent operator edits silently last-writer-win and the audit log diverges from DB state. promote_collection_candidates likewise lacks the write lock, uses a non-deterministic _timestamp_id(), and has no transaction/idempotency guard, so concurrent batch-promotes mint duplicate, non-collapsible knowledge entries. (Note: mark_evidence_status is the single most-reported issue — it surfaced 6 times across units; it is the top fix.)
Silent loss of operator actions in the GUI. deactivateCorrectionEntry and the provider-retry handler both mutate module-level arrays and append a synthetic audit event with no API call — the action evaporates on the next refreshFromApi(). Operators believe work happened when nothing was persisted.
Risk under-scoring (mis-classification). _web_detection_bucket collapses every named entity into one "entity" bucket, so a second distinct entity (e.g. two different celebrities/characters in one image) is silently dropped — suppressing ~40 points and potentially flipping a genuinely high-risk image to medium, reducing operator scrutiny.
Frontend concurrency: rerunHistoricalQuery fires async runManualSearch with no await and no in-flight guard, enabling duplicate/racing search POSTs and interleaved bootstrap refreshes.
Maintainability hotspots (acknowledged debt): _auto_naver_search (234 lines) / _auto_google_custom_search (165 lines) are near-identical copy-paste cascades that must be kept in sync; app.js is 3,926 lines (5x ceiling) and styles.css is 2,819.

The unifying risk is audit/state integrity — multiple independent findings show operator actions that either silently fail to persist or persist without a matching audit record. For a rights-review compliance tool, that is the theme most worth fixing first.

2. 통계

심각도	건수
CRITICAL	0
HIGH	6
MEDIUM	43
LOW	17

분류	건수
security	23
bug	19
maintainability	8
error-handling	8
testing	4
build	3
performance	1

3. 교차 테마

Audit-trail & state-persistence integrity

The single most important cross-cutting risk: multiple independent findings show operator actions that either persist without a matching audit record, or appear to succeed but never reach the server. mark_evidence_status writes evidence+audit+rescore non-atomically across four connections (no _write_lock, no _transaction), so concurrent edits last-writer-win and the audit log diverges. promote_collection_candidates has the same transaction gap plus a non-deterministic ID, producing duplicate knowledge entries. On the frontend, deactivateCorrectionEntry and the provider-retry button mutate local arrays and push synthetic audit events with no API call, so the action silently evaporates on the next refreshFromApi(). For a rights-review compliance tool this is the theme to fix first.

_관련 발견: mark_evidence_status: non-atomic read-modify-write without _write_lock or transaction, promote_collection_candidates missing write lock and non-idempotent ID, deactivateCorrectionEntry mutates shared module-level arrays without an API call, provider-retry button writes a local-only synthetic audit entry with no API call, rerun_enrichment and run_auto_search perform unguarded concurrent read-modify-write on evidence and submissions, mark_evidence_status atomicity is not tested

Unguarded concurrency / fire-and-forget async

ThreadingHTTPServer means store read-modify-write sequences need _write_lock + _transaction, and the GUI needs in-flight guards on async actions. Several store methods skip the lock; several frontend handlers fire async calls without await, void, or a disabled-button guard, enabling duplicate/racing POSTs and interleaved bootstrap refreshes.

_관련 발견: rerunHistoricalQuery calls async runManualSearch without await and without an in-flight guard, No in-flight guard on setDecision, rerun_enrichment and run_auto_search perform unguarded concurrent read-modify-write on evidence and submissions, promote_collection_candidates missing write lock and non-idempotent ID, mark_evidence_status: non-atomic read-modify-write without _write_lock or transaction

Risk-scoring correctness (under-escalation)

Bugs in RiskScorer cause genuinely high-risk images to score lower than they should or to show misleading justifications. The entity-bucket collapse drops all but the first named entity; the category/entity ordering asymmetry makes scoring depend on iteration order; and zero-point skip reasons pollute the operator-facing reasons list. All reduce the fidelity of the signal operators rely on to escalate.

관련 발견: Multiple distinct web-detection entity signals collapse into one scoring bucket, Entity and category-matched WEB_DETECTION items share the same dedup bucket causing priority-dependent under-scoring, SEARCH_SKIPPED and EXTERNAL_SKIPPED reasons pollute ScoreResult.reasons despite contributing zero points, ScoreResult.reasons is a mutable list inside a frozen dataclass, allowing inadvertent mutation

Silent error swallowing without logging

A recurring pattern of bare except blocks and hardcoded fallbacks that hide infrastructure failures (SSRF rejections, network/disk errors, malformed Ollama responses, corrupt DB rows) at the same level as expected skips, making real failures invisible in logs and, for one case, silently leaking EXIF/GPS to external APIs.

_관련 발견: _sync_face_crops swallows image load failure without any log or audit trail, _store_candidate_image silently discards network and fingerprint errors with no log, _response_text falls back to 'Evidence summary unavailable.' without logging or raising, json.loads() on DB rows with no error handling, _search_result_page_image_candidates and search_result_stylesheet_image_urls swallow all page-fetch exceptions silently, Silent EXIF/GPS metadata leakage when PIL re-encoding fails in build_external_derivative, Provider lastFailure attribution reads evidence[0] rather than the first failed item, Unhandled exception types in request handlers log stack traces to stderr

No live exploit on the air-gapped localhost deployment, but the security posture relies too heavily on single layers (SameSite cookie, DB CHECK constraints) with no browser-level backstop. The app shell ships no CSP/X-Frame-Options, the session cookie is the raw master token, several innerHTML/src/href sinks skip escapeHtml/safeUrl, and there is no auth rate-limiting or token-entropy floor. Cheap to fix; important if the deployment ever sits behind a proxy.

관련 발견: No security headers on main application responses (missing CSP, X-Frame-Options, X-Content-Type-Options), No Content-Security-Policy header on the app shell or API responses, Cookie stores raw auth token, missing Secure flag, No minimum-entropy check on COPYRIGHTER_AUTH_TOKEN, No rate limiting on authentication or any API endpoint, CSRF protection relies solely on SameSite=Strict with no Origin validation fallback, innerHTML templates inject server-derived label lookups without escapeHtml, riskClass() injects unvalidated server string into class attribute without escaping, submission.asset assigned directly to img.src without safeUrl, similar-strip and face-crop-strip image URLs not passed through safeUrl, faceCrops href and img src use escapeHtml but not safeUrl, fetch() calls use no explicit credentials mode

Outbound HTTP / SSRF consistency

The candidate-image/page fetch path is well-guarded (SSRF check re-validated per redirect, size-capped), but the JSON API transport diverges: no response-size cap, no redirect SSRF re-validation, and Google API keys embedded in URLs that would appear in tracebacks. Aligning UrllibJsonTransport with store_remote_fetch's hardened opener closes the gap.

관련 발견: UrllibJsonTransport reads API responses without a size limit, UrllibJsonTransport follows HTTP redirects without SSRF re-validation, Google and Custom Search API keys embedded in outbound request URLs expose them in Python tracebacks

Maintainability hotspots & copy-paste duplication

Large files and duplicated logic raise the cost and risk of every future change. The two auto-search mega-functions are near-identical cascades that must be kept in sync (a fix to one can silently miss the other); the adapter methods and keyword-candidate fallback repeat the same boilerplate; app.js (3926) and styles.css (2819) far exceed the 800-line ceiling. Most of this is documented, consciously deferred debt.

_관련 발견: _auto_naver_search() and auto_google_custom_search() each exceed 165 lines, NaverSearchAdapter has three search() / search_pages() / search_web_pages() methods with nearly identical structure, collect_keyword_candidates() is 85 lines with a repeated 3-phase naver fallback pattern, app.js is 3926 lines, styles.css at 2819 lines, formatReason() uses a cascade of string replacement fallbacks, window.prompt() used synchronously inside async event handlers

Test-coverage gaps for shipped fixes & security invariants

Several recent fixes and security guarantees lack pinning tests, so a future refactor could silently regress them: the Naver blog/web dedup fix, mark_evidence_status atomicity, media-route auth enforcement, untrusted-media security headers, safeUrl/escapeHtml behaviour, and the redirect-SSRF handler.

_관련 발견: No regression test for the Naver blog/web dedup fix (commit 71a6da1), mark_evidence_status atomicity is not tested; only record_decision rollback is pinned, Media route authentication is not verified under auth_token enforcement, Security headers on untrusted media (_file with untrusted=True) are not asserted in any test, safeUrl / escapeHtml XSS defence in app.js is structural-only; no behavioural assertions, _fetch_page_url_bytes and fetch_stylesheet_url_bytes SSRF guard not independently tested, import-folder with empty/missing path body is not validated by the test suite, E2E decision flow covers only 4 scenarios; no test for hold/reject after prior approve

Air-gapped deployment completeness

Core air-gap posture is correct (pinned deps, local assets, no telemetry), but three transfer/integrity gaps remain in documentation and supply chain: Ollama model weights and Playwright Chromium binaries are not documented for offline transfer, and the planned hash-verified lock file was never committed.

관련 발견: offline-install.md omits Ollama model bundling procedure, offline-install.md omits Playwright Chromium binary transfer procedure, No full transitive dependency lock file with hashes, LLM assistant is unconditionally constructed even when OLLAMA_BASE_URL is not set

4. 리메디에이션 로드맵

Prioritized Remediation Roadmap

Phase 1 — HIGH-severity correctness & integrity (fix before merge)

These are confirmed bugs with operator-visible or data-loss impact. Small, surgical fixes.

Make mark_evidence_status atomic (finding: "mark_evidence_status: non-atomic read-modify-write without _write_lock or _transaction") — wrap the read/mutate/write/rescore/audit body in with self._write_lock: with self._transaction() as conn: and thread conn= through _put, _rescore_submission, and add_audit_event, mirroring record_decision (sqlite_store.py:569). This is the highest priority — it was independently reported 6 times. Effort: ~1 hr. Add a rollback test mirroring test_record_decision_rolls_back_when_audit_fails (closes "mark_evidence_status atomicity is not tested").
Fix promote_collection_candidates concurrency + idempotency (finding: "promote_collection_candidates missing write lock and non-idempotent ID") — acquire _write_lock, replace _timestamp_id() with _stable_id('kb-candidate-batch', *sorted(candidate_ids)), wrap all writes in _transaction(), and add the same already-promoted guard the single-candidate path has. Effort: ~1.5 hr.
Persist deactivateCorrectionEntry and provider-retry (findings: "deactivateCorrectionEntry mutates shared module-level arrays without an API call"; "provider-retry button writes a local-only synthetic audit entry with no API call") — replace the local mutation + synthetic audit with a real await apiJson(...) followed by await refreshFromApi(), following the toggleProvider pattern. If no backend endpoint exists yet, disable the button / show "not yet available" rather than faking success. Effort: ~2 hr each (depends on whether endpoints exist).
Fix entity dedup under-scoring (finding: "Multiple distinct web-detection entity signals collapse into one scoring bucket") — make the bucket entity-name-specific: f"entity:{str(item.data.get('entity','')).casefold()}". While here, also fix the related LOW ordering asymmetry ("Entity and category-matched WEB_DETECTION items share the same dedup bucket") by mirroring _web_detection_points priority in _web_detection_bucket. Add a regression test for two distinct entities flipping a score to high. Effort: ~1.5 hr.
Add in-flight guard + await to rerunHistoricalQuery/runManualSearch (finding: "rerunHistoricalQuery calls async runManualSearch without await and without an in-flight guard") — disable the submit button before await and re-enable in finally; void or await the call. Effort: ~1 hr.

Phase 2 — MEDIUM security hardening (defence-in-depth; batch into one security PR)

None are exploitable in the current air-gapped localhost model, but they are cheap and the deployment may move behind a proxy.

Security headers on the app shell (findings: "No security headers on main application responses"; "No Content-Security-Policy header on the app shell or API responses") — in _file() when untrusted=False and content is HTML/JS, add Content-Security-Policy: default-src 'self'; ... frame-ancestors 'none', X-Frame-Options: DENY, X-Content-Type-Options: nosniff. The app uses no inline scripts/CDN so a strict CSP is compatible. Effort: ~1 hr.
Decouple session cookie from the raw token (finding: "Cookie stores raw auth token, missing Secure flag") — store an HMAC-derived session tag instead of the verbatim COPYRIGHTER_AUTH_TOKEN, and add the Secure attribute. Effort: ~1.5 hr.
Auth-token entropy check + auth rate limiting (findings: "No minimum-entropy check on COPYRIGHTER_AUTH_TOKEN"; "No rate limiting on authentication or any API endpoint") — reject tokens < 32 chars at startup; add a per-IP failed-attempt counter with short lockout + time.sleep on failure. Effort: ~2 hr together.
Cap outbound response size (finding: "UrllibJsonTransport reads API responses without a size limit") — apply response.read(MAX + 1) and raise HttpJsonError past the cap, mirroring store_remote_fetch. Prevents OOM from a runaway Ollama/API response. Effort: ~45 min.
Origin/Referer CSRF fallback (finding: "CSRF protection relies solely on SameSite=Strict") and keep API keys out of tracebacks (finding: "Google and Custom Search API keys embedded in outbound request URLs") — add an Origin check for cookie-authed non-GET requests; move Google keys to a header/POST body or strip them from re-raised exceptions. Effort: ~2 hr together.
Escape remaining innerHTML / class sinks (findings: "innerHTML templates inject server-derived label lookups without escapeHtml"; "riskClass() injects unvalidated server string"; "submission.asset assigned directly to img.src without safeUrl"; "similar-strip and face-crop-strip image URLs not passed through safeUrl"; "faceCrops href and img src use escapeHtml but not safeUrl")* — wrap all server-derived values in escapeHtml(), allowlist riskClass, and route every src/href through safeUrl(). These are one-liners; batch them. Effort: ~1.5 hr total.

Phase 3 — MEDIUM correctness, error-handling & robustness

Add _write_lock to operator rerun paths (finding: "rerun_enrichment and run_auto_search perform unguarded concurrent read-modify-write"). Effort: ~30 min.
Fix ensure_queue UPDATE WHERE-key bug (finding: "ensure_queue UPDATE uses computed queue_id instead of existing row id on folder_path match") — use the existing row's id. Effort: ~45 min.
Stop swallowing errors silently (findings: "_sync_face_crops swallows image load failure"; "_store_candidate_image silently discards network and fingerprint errors"; "_response_text falls back ... without logging"; "json.loads() on DB rows with no error handling"; "_search_result_page_image_candidates ... swallow all page-fetch exceptions") — add logging.warning/debug with URL/submission-id context; wrap json.loads in try/except raising a domain error mapped to a safe 500. Effort: ~2 hr.
Fix provider lastFailure attribution (finding: "Provider lastFailure attribution reads evidence[0] rather than the first failed item") — use next(... if item.source in FAILURE_SOURCES). Effort: ~30 min.
Drop zero-point skip reasons from ScoreResult.reasons (finding: "SEARCH_SKIPPED and EXTERNAL_SKIPPED reasons pollute ScoreResult.reasons"). Effort: ~30 min.
GUI null-guards + double-submit guards (findings: "rerunSelectedEnrichment accesses button.disabled without null guard"; "applyCoverageFilter accesses sourceSelect.value without null guard"; "No in-flight guard on setDecision"). Effort: ~1 hr.
Targeted evidence query in dedup hot path (finding: "Auto-search dedup loads entire evidence table") — swap _evidence_by_submission().get(id) for _evidence_for_submission(id). Effort: ~20 min (the only performance finding).
EXIF/GPS leak on PIL fallback (finding: "Silent EXIF/GPS metadata leakage when PIL re-encoding fails") — log a warning (or hard-fail) instead of silently sending original bytes; important since an air-gapped target without Pillow hits this every submission. Effort: ~45 min.
Make ScoreResult.reasons immutable (finding: "ScoreResult.reasons is a mutable list inside a frozen dataclass") — convert to tuple in __post_init__. Effort: ~30 min.

Phase 4 — MEDIUM maintainability (schedule deliberately; larger refactors)

Extract shared search-plan helper (finding: "_auto_naver_search() and _auto_google_custom_search() each exceed 165 lines") — highest-value refactor; collapses two (partly three) copy-paste cascades into one _execute_search_query_plan(...). Effort: ~1 day.
De-duplicate adapter boilerplate (finding: "NaverSearchAdapter has three search()/search_pages()/search_web_pages() methods") and collapse the naver fallback cascade (finding: "collect_keyword_candidates() is 85 lines with a repeated 3-phase naver fallback"). Effort: ~0.5 day.
Split app.js and styles.css (findings: "app.js is 3926 lines"; "styles.css at 2819 lines") — already documented as accepted/deferred debt (downgraded to MEDIUM). Split into the proposed ES-module / CSS-partial layout; concatenate at build for air-gap. Effort: ~1-2 days. Lower urgency — no runtime risk.
Replace window.prompt() with inline forms (finding: "window.prompt() used synchronously inside async event handlers") — also unblocks reliable Playwright testing. Effort: ~0.5 day.

Phase 5 — Test coverage & docs (close regression gaps)

Add the missing regression tests (findings: "No regression test for the Naver blog/web dedup fix"; "mark_evidence_status atomicity is not tested" — covered by item 1; "Media route authentication is not verified"; "Security headers on untrusted media ... not asserted"; "safeUrl/escapeHtml XSS defence ... no behavioural assertions"; "_fetch_page/stylesheet SSRF guard not independently tested"; "import-folder with empty/missing path body is not validated"; "E2E decision flow ... no hold/reject after prior approve"). Effort: ~1 day total; do alongside the fixes they protect.
Document offline extras (findings: "offline-install.md omits Ollama model bundling procedure"; "offline-install.md omits Playwright Chromium binary transfer"; "No full transitive dependency lock file with hashes") — add Ollama model + Playwright Chromium transfer sections and a requirements-lock.txt with --hash (the plan's Task 7). Effort: ~0.5 day.

Phase 6 — LOW defence-in-depth (opportunistic cleanup)

Batch when touching the relevant files: SSRF re-validation on UrllibJsonTransport redirects, explicit credentials: 'same-origin' on fetch(), _put() f-string fallback → explicit raise, _drop_stale_rebuild_temp_tables regex re-validation, sanitized handle_error override, and gating _llm_assistant construction on OLLAMA_BASE_URL (avoids per-submission connect-timeout stalls). Effort: ~0.5 day combined.

5. 발견 상세

🟠 HIGH

1. mark_evidence_status: non-atomic read-modify-write without _write_lock or _transaction

분류 bug · 파일 src/rights_filter/server/sqlite_store.py:587-617 · 신뢰도 high · 검증 confirmed · 유닛 sec-sql
설명: mark_evidence_status performs a read (_get at line 596), mutates the dict in-memory (lines 600-608), writes (_put at line 609), rescores (line 610), then writes an audit event (lines 611-616) — all across four separate database connections with no coordinating lock and no _transaction() wrapper. This is the exact non-atomic pattern that record_decision (line 557-585) already fixed by using _transaction(). Under ThreadingHTTPServer, two concurrent operator requests marking the same evidence item will open interleaved connections: the second _get reads stale pre-first-write state, the second _put overwrites the first silently (last-writer-wins), and the audit entry for the losing write is recorded against the wrong final state. The rescore then fires on a different connection, potentially racing against the _put that just committed. The prior-context digest (§3) explicitly lists this as still PENDING: 'mark_evidence_status atomic state+audit transaction — same _transaction pattern as record_decision, not yet applied.'
영향: Evidence status changes can be silently lost when two operators act concurrently on the same evidence item. The audit log records events that do not match the final DB state, undermining the audit trail integrity that is a product invariant ('filter never auto-changes review status').
권장: Wrap the read-mutate-write-audit sequence in a single _transaction() context and acquire self._write_lock, mirroring the pattern in record_decision (lines 569-584). The corrected body should be: with self._write_lock: with self._transaction() as conn: payload = self._get('evidence', evidence_id, conn=conn); ... self._put('evidence', evidence_id, payload, conn=conn); self._rescore_submission(submission_id); self.add_audit_event(..., conn=conn).

2. promote_collection_candidates missing write lock and non-idempotent ID

분류 bug · 파일 src/rights_filter/server/store_operations.py:694-759 · 신뢰도 high · 검증 confirmed · 유닛 bug-store
설명: promote_collection_candidates uses _timestamp_id() (non-deterministic) for entry_id and does not acquire _write_lock. Two concurrent batch-promote requests create two separate knowledge entries with different IDs and mark candidates with different promotedKnowledgeId values. The writes also use separate auto-commit calls with no _transaction() wrapper, so a server crash mid-loop leaves the knowledge entry written but some candidates not marked promoted. The single-candidate variant promote_collection_candidate (line 654) correctly acquires _write_lock, uses _stable_id for idempotency, and wraps its writes in _transaction().
영향: Concurrent double-clicks or parallel API requests create duplicate knowledge entries for the same candidate batch, polluting the reference DB with duplicates that cannot be idempotently collapsed.
권장: Wrap the method body with 'with self._write_lock:' and replace _timestamp_id() with _stable_id('kb-candidate-batch', *sorted(candidate_ids)). Wrap all _put and add_audit_event calls in 'with self._transaction() as conn:' passing conn to each call, mirroring the pattern in promote_collection_candidate.

3. Multiple distinct web-detection entity signals collapse into one scoring bucket

분류 bug · 파일 src/rights_filter/analysis/risk_scoring.py:29-37 · 신뢰도 high · 검증 confirmed · 유닛 bug-analysis
설명: In RiskScorer.score, after computing bucket = _web_detection_bucket(item) for a WEB_DETECTION item, the code skips any item whose bucket is already in counted_web_buckets. _web_detection_bucket returns "entity" for ANY item that has a non-empty data['entity'] field — regardless of which entity it identifies. As a result, if Google Vision returns two independent entity annotations for different known subjects (e.g. entity='BTS' then entity='BLACKPINK', both with category='celebrity'), the second item is unconditionally skipped once the 'entity' bucket is claimed by the first. Only the first entity ever contributes points. This can suppress 45+ points and shift a genuinely high-risk image to medium, preventing operator escalation.
영향: Under-scoring of images that trigger multiple distinct entity recognitions. A composition image with two identifiable characters/celebrities would lose up to 45 points from the second detection, potentially staying at 'medium' rather than 'high', reducing operator scrutiny.
권장: Make the dedup bucket specific enough to represent truly redundant signals. Options: (a) use entity:<entity_name> as the bucket key (e.g. f"entity:{str(item.data.get('entity','')).casefold()}") so two different named entities get distinct buckets; (b) remove the entity bucket entirely and instead cap the WEB_DETECTION contribution in a post-loop clamp rather than per-category early exit. The intent of the dedup (preventing 10 identical 'visual similar' items from stacking) is correct — it just needs to be scoped to exact-duplicate signals, not all entity detections.

4. rerunHistoricalQuery calls async runManualSearch without await and without an in-flight guard

분류 bug · 파일 web/operator-gui/app.js:2466-2479 · 신뢰도 high · 검증 confirmed · 유닛 bug-frontend
설명: rerunHistoricalQuery (line 2466) populates the query input and provider select, then calls runManualSearch(new Event('submit')) synchronously — but runManualSearch is async. Because the call is not awaited and there is no void, the returned Promise is discarded. More critically, runManualSearch has no in-flight guard (no disabled state, no flag): if the user clicks two different 'Query Re-run' buttons in quick succession, or if the suggestion runner and the re-run fire close together, two concurrent POST /api/search/manual requests can race, both calling refreshFromApi() concurrently, which causes two parallel /api/bootstrap fetches and two applyBootstrap + renderAll calls that interleave non-atomically. The runManualSearch path itself also clears queryInput.value mid-flight, meaning the second call may POST an empty query string.
영향: Concurrent POSTs can produce duplicate evidence entries or an empty-query search; interleaved bootstrap refreshes can leave the UI in an inconsistent state.
권장: Add an in-flight guard to runManualSearch (analogous to isSuggestedQueryRunActive in executeSuggestedQueries): disable the submit button before await apiJson(...) and re-enable in a finally. In rerunHistoricalQuery, either void the call explicitly or convert the function to async and await runManualSearch(...). The submit event propagation pattern (passing new Event('submit')) bypasses the form's own submit listener flow; it works here but is fragile — consider extracting the search logic into a shared helper instead.

5. deactivateCorrectionEntry mutates shared module-level arrays without an API call — state is never persisted and will be overwritten

분류 bug · 파일 web/operator-gui/app.js:3221-3248 · 신뢰도 high · 검증 confirmed · 유닛 bug-frontend
설명: deactivateCorrectionEntry (line 3221) directly mutates items inside the corrections array (entry.active = false) and pushes a synthetic entry onto auditEvents. There is no API call — no POST/PATCH to the server. The function then calls renderAll(). The apparent effect is shown in the UI, but the next call to refreshFromApi() (triggered by any other action, e.g. approving a case) will call applyBootstrap which calls replaceCollection(corrections, payload.corrections) and replaceCollection(auditEvents, payload.auditEvents), overwriting the local mutations entirely. The 'deactivate derived entries' action silently becomes a no-op on the server, and the synthetic audit log entry disappears. This violates the invariant that every operator action must be persisted.
영향: Operator believes derived knowledge entries are deactivated. On next data refresh the UI silently reverts. Audit trail entry is also lost. Data integrity is compromised.
권장: Replace the local mutation with a real API call (e.g. POST /api/corrections/{id}/deactivate or PATCH /api/knowledge/{derived-entry-id}). Until a server endpoint exists, at minimum add a visible warning in the UI that this action is not yet persisted, or disable the button. The void cast on deactivateKnowledgeEntry at line 3634 shows the correct pattern for similar operations.

6. _auto_naver_search() and _auto_google_custom_search() each exceed 165 lines — highest-risk large functions in the Python layer

분류 maintainability · 파일 src/rights_filter/server/store_enrichment.py:391-791 · 신뢰도 high · 검증 confirmed · 유닛 mnt-hotspots
설명: _auto_naver_search() spans lines 557–791 (234 lines) and _auto_google_custom_search() spans lines 391–556 (165 lines). Both functions contain an identical structural skeleton: (a) plan queries, (b) check existing dedup signatures, (c) for each planned query call the image/primary search, (d) annotate evidence with strategy metadata, (e) write evidence rows, (f) call similarity matching, (g) append a history entry, (h) call the page/web secondary search with its own dedup check, (i) repeat (d)–(g) for the secondary results, (j) flush queryHistory to the submission, (k) update provider lastSuccess/lastFailure/usage. The only meaningful differences are: naver has three search types (image, blog, web) vs Google's two (image, web); naver uses blog_query_count / web_query_count rate limiters; the provider keys differ. This structural duplication means any fix to the copy-annotate-write-history pattern must be applied in two places. Additionally, _manual_search_evidence() at lines 426–534 (109 lines) replicates a subset of the same cascade (naver image → blog → web fallback; google image → web fallback) as a third copy.
영향: Any fix to the dedup logic, the query_strategy annotation, or the history-flush pattern must be applied in both mega-functions. A bug introduced in one copy (e.g., missing existing_signatures.add for one search type) will not be caught without running both providers end-to-end.
권장: Extract a shared _execute_search_query_plan(submission_id, provider_id, planned_queries, image_store, *, call_one_query_fn) helper that owns steps (b)–(k): it accepts a callable call_one_query_fn(query) -> tuple[list[Evidence], str] (returns evidence + history provider label) and handles dedup, evidence write, similarity, history flush, and usage update. _auto_naver_search and _auto_google_custom_search become thin callers that build the per-provider callable and pass it in. This also makes _manual_search_evidence naturally align with the same helper.

🟡 MEDIUM

7. mark_evidence_status state update and audit write are non-atomic

분류 bug · 파일 src/rights_filter/server/sqlite_store.py:587-617 · 신뢰도 high · 검증 unverified · 유닛 sec-http
설명: This is the deferred item explicitly tracked in the prior-review plan. _put(evidence, ...) (line 609), _rescore_submission (line 610), and add_audit_event (line 611) execute as three separate auto-committed statements. A crash or concurrent write between the _put and add_audit_event calls leaves evidence with a new operatorStatus but no corresponding audit record, producing a silent audit divergence. record_decision (line 569) already uses self._transaction() for the same pattern — mark_evidence_status should too.
영향: Audit trail can silently diverge from actual evidence state, undermining the integrity of the rights-review record.
권장: Wrap the entire function body in with self._transaction() as conn: and thread conn=conn through _put, _rescore_submission, and add_audit_event, matching the pattern already used by record_decision.

8. ensure_queue UPDATE uses computed queue_id instead of existing row id on folder_path match

분류 bug · 파일 src/rights_filter/server/store_queue.py:99-130 · 신뢰도 high · 검증 unverified · 유닛 bug-store
설명: ensure_queue selects existing rows by id OR folder_path (lines 99-102). When a row is found via the folder_path match but that row has a different id than the newly computed queue_id, the UPDATE on lines 113-115 uses queue_id as the WHERE key and silently matches 0 rows. Line 117 then deactivates the actual existing row (id != queue_id), and the final SELECT finds nothing because no row was inserted or correctly updated. _queue_row_by_id(queue_id) also returns None, so the method returns None instead of a queue dict.
영향: If a folder path normalization ever changes (e.g., symlink resolution), the next ensure_queue call silently returns None, causing seed_from_image_store to crash on str(queue['id']) with a TypeError.
권장: After fetching existing, extract its id: existing_id = existing['id'] if existing else None. In the else branch use WHERE id = ? with existing_id. Adjust the deactivation on line 117 and the final SELECT on line 120 to use the effective active id.

9. rerun_enrichment and run_auto_search perform unguarded concurrent read-modify-write on evidence and submissions

분류 bug · 파일 src/rights_filter/server/store_operations.py:202-281, 283-312 · 신뢰도 medium · 검증 unverified · 유닛 bug-store
설명: Both rerun_enrichment and run_auto_search perform long chains of _get and _put calls on submissions and evidence tables without holding _write_lock. Two concurrent requests for the same submission can interleave: one reads pre-rerun evidence, the other finishes and writes its results, and the first then overwrites with stale data. seed_from_image_store correctly wraps its entire path with _write_lock (sqlite_store.py:229-234); the operator-triggered rerun paths do not. The _write_lock is a reentrant RLock so inner callers like _apply_provider_usage_delta that also acquire it will not deadlock.
영향: Under concurrent operator-triggered reruns for the same submission, evidence writes are silently overwritten and per-provider usage counters can lose increments.
권장: Acquire self._write_lock at the top of both rerun_enrichment and run_auto_search, mirroring the pattern used in seed_from_image_store and purge_expired_face_crops.

10. SEARCH_SKIPPED and EXTERNAL_SKIPPED reasons pollute ScoreResult.reasons despite contributing zero points

분류 bug · 파일 src/rights_filter/analysis/risk_scoring.py:57-63 · 신뢰도 high · 검증 unverified · 유닛 bug-analysis
설명: In the FAILURE / EXTERNAL_SKIPPED / SEARCH_SKIPPED / ENRICHMENT_FAILURE branch (lines 45-63), the code correctly adds 0 to score for SEARCH_SKIPPED and EXTERNAL_SKIPPED sources, but then unconditionally calls reasons.append(item.reason) on line 63 for all non-LLM paths including these zero-point sources. The ScoreResult.reasons list is shown to operators as justification for the risk score. Reasons like 'Search API skipped (quota)' or 'External provider disabled' are operational status messages, not risk signals, so including them in the reasons list misrepresents what drove the score — and they can appear even in a score=0 / band='low' result. The bug is that the reasons.append is outside the if score += 30 block, coupling reason emission to all non-LLM sources regardless of point contribution.
영향: Operators see misleading reason text in the review UI for skip/disabled states. A score=0 'low' result could display 'External provider disabled' as a reason, implying a risk signal when none exists. Separately, a legitimate high-risk item that also had search skipped will show the skip reason alongside the true risk reasons, diluting the signal clarity.
권장: Move reasons.append(item.reason) inside the block that awards non-zero points, or introduce a separate operational_notes list in ScoreResult for zero-contribution status messages. Minimal fix: if item.source in {EvidenceSource.FAILURE, EvidenceSource.ENRICHMENT_FAILURE}: score += 30; reasons.append(item.reason) (handle the skipped sources separately without appending their reasons).

11. rerunSelectedEnrichment accesses button.disabled without null guard

분류 bug · 파일 web/operator-gui/app.js:2540, 2551, 2580 · 신뢰도 high · 검증 unverified · 유닛 bug-frontend
설명: rerunSelectedEnrichment fetches document.getElementById('bulk-rerun') (line 2540). If ids.length is 0, the early return at line 2545 fires before any button access. But if ids.length > 0, the code reaches button.disabled = true at line 2551 and button.disabled = false in finally at line 2580. If the element is ever absent from the DOM (e.g. test environment, or if index.html is modified), both accesses throw a TypeError that propagates out of the async function as an unhandled rejection, bypassing the setImportStatus error path. The analogous pattern in rerunEnrichment (line 2498) correctly does if (button) button.disabled = true.
영향: Crashes the function in environments where the DOM element is missing (tests, future HTML refactors), producing an uncaught TypeError.
권장: Apply the same null guard as rerunEnrichment: replace button.disabled = true with if (button) button.disabled = true and in the finally block if (button) button.disabled = false.

12. provider-retry button writes a local-only synthetic audit entry with no API call

분류 bug · 파일 web/operator-gui/app.js:3665-3687 · 신뢰도 high · 검증 unverified · 유닛 bug-frontend
설명: The [data-provider-retry] click handler (line 3665) pushes a hardcoded synthetic event into auditEvents and calls renderAll(), but makes no API call to actually retry failed enrichments. The audit entry says 'retry failed enrichments' but nothing is retried. On the next refreshFromApi() call the synthetic entry is overwritten. This is similar to the deactivateCorrectionEntry issue — the operator believes they triggered a retry but no server-side action occurred.
영향: Operator action has no server-side effect; operator is misled by the synthetic audit entry which disappears on next refresh.
권장: Implement the API call (e.g. POST /api/providers/{id}/retry-failures) and await it before updating the audit log. Until the endpoint is available, either hide the button or show a 'not yet implemented' message instead of creating a misleading synthetic audit entry.

13. applyCoverageFilter accesses sourceSelect.value without null guard

분류 bug · 파일 web/operator-gui/app.js:498-510 · 신뢰도 medium · 검증 unverified · 유닛 bug-frontend
설명: applyCoverageFilter calls const sourceSelect = document.getElementById('source-filter') then immediately accesses sourceSelect.value and sourceSelect.options at lines 504-505 without checking that sourceSelect is non-null. The function is called from the delegated coverage-tabs click handler which can fire at any time. If the DOM element is absent (test environment, or the coverage-tabs listener fires before the DOM is fully ready on a slow machine), this throws TypeError: Cannot read properties of null.
영향: Uncaught TypeError if the DOM element is absent; potential crash of the coverage filter flow.
권장: Add an early return guard: if (!sourceSelect) return; immediately after the getElementById call, consistent with the null check pattern used in renderImportStatus (line 1262) and renderCollectionCandidates (line 1947).

14. No in-flight guard on setDecision — rapid decision button clicks can POST multiple decisions

분류 bug · 파일 web/operator-gui/app.js:2328-2380 · 신뢰도 high · 검증 unverified · 유닛 bug-frontend
설명: setDecision (line 2328) does not disable the approve/hold/reject buttons before await apiJson(...) at line 2362. An operator who double-clicks a decision button, or clicks two different decision buttons quickly (e.g. 'approve' then immediately 'reject'), will fire two concurrent POST /api/submissions/{id}/decision requests. The second one may overwrite the first or produce a conflict depending on server logic. The server may guard against this, but the client has no protection. Compare with rerunEnrichment (line 2498) which correctly disables its button before the await.
영향: Duplicate or conflicting decision POST requests can be sent; server state may diverge from operator intent.
권장: Disable all three decision buttons (approve/hold/reject) before the await and re-enable them in a finally block: query document.querySelectorAll('[data-decision]') and toggle .disabled around the API call.

15. Provider lastFailure attribution reads evidence[0] rather than the first failed item

분류 bug · 파일 src/rights_filter/server/store_enrichment.py:311, 545, 780 · 신뢰도 high · 검증 unverified · 유닛 err-silent
설명: In _rerun_google_image_search (line 311), _auto_google_custom_search (line 545), and _auto_naver_search (line 780), when the failure condition is triggered (any(item.source in {FAILURE, SEARCH_SKIPPED, ENRICHMENT_FAILURE} ...)), the code sets lastFailure to all_domain_evidence[0].reason — the first item in the list, not the first failing item. If any successful evidence items come before the failure item in the list, the lastFailure field is set to the reason of a successful result (e.g. 'Naver search result found'), making the provider dashboard actively misleading.
영향: The operator's provider health panel shows a spurious, plausible-sounding success message as the failure reason, obscuring the actual API error. This makes diagnosing quota exhaustion or API key rotation failures harder.
권장: Replace all_domain_evidence[0].reason with next((item.reason for item in all_domain_evidence if item.source in {EvidenceSource.FAILURE, EvidenceSource.SEARCH_SKIPPED, EvidenceSource.ENRICHMENT_FAILURE}), '<provider> failed') to ensure the reported reason is always from an actual failure item.

16. mark_evidence_status state+audit write is not wrapped in a transaction

분류 bug · 파일 src/rights_filter/server/sqlite_store.py:587-617 · 신뢰도 high · 검증 unverified · 유닛 err-silent
설명: mark_evidence_status (lines 587–617) performs three separate writes without an enclosing _transaction(): self._put("evidence", ...) at line 609, self._rescore_submission at line 610 (which itself does _get + _put), and self.add_audit_event at line 611. Each opens and commits its own connection. If the process is interrupted between the evidence _put and the add_audit_event, the evidence status changes are applied but unaudited. This is the same pattern that was fixed for record_decision (H6 in the prior review) but the fix was not extended to mark_evidence_status, which is noted in the prior digest's 'still PENDING' section.
영향: An interrupted operator evidence-status change produces no audit trail, creating a divergence between the evidence state and the audit log. This is a governance integrity issue for the rights-review workflow.
권장: Wrap the entire mark_evidence_status body (after validation) in with self._transaction() as conn:, passing conn to _put, _rescore_submission, and add_audit_event, following the same pattern as record_decision at line 569.

17. deactivateCorrectionEntry() mutates shared array in-place without a server round-trip — client state can desync

분류 bug · 파일 web/operator-gui/app.js:3221-3248 · 신뢰도 high · 검증 unverified · 유닛 mnt-hotspots
설명: Lines 3227–3229: correction.derivedEntries.forEach((entry) => { entry.active = false; }); mutates the in-memory corrections array directly without calling any API endpoint. Lines 3233–3245 then push a fake audit event into the local auditEvents array. No HTTP call is made, so the deactivation is never persisted. After the next refreshFromApi() (which happens after the next unrelated action), the correction entries will reappear as active and the synthetic audit event will vanish. This violates the 'no mutation' coding standard and the 'errors handled explicitly' standard. Compare with deactivateKnowledgeEntry() (line 3346) which correctly calls the API.
영향: Operators who click 'Deactivate derived entries' will see a brief success in the UI, but the state will silently revert to active on the next API refresh. This is a silent functional regression: the action appears to work but persists nothing.
권장: Add a backend endpoint (e.g. POST /api/corrections/{id}/deactivate-derived) and replace the local mutation with an async call to that endpoint followed by await refreshFromApi(). If the endpoint does not exist yet, at minimum add a console.warn and document the stub, but the current silent-success UX is misleading to operators.

18. Provider retry button appends a fake audit event client-side without calling any API

분류 bug · 파일 web/operator-gui/app.js:3665-3687 · 신뢰도 high · 검증 unverified · 유닛 mnt-hotspots
설명: The data-provider-retry click handler (lines 3665–3687) pushes a synthetic audit event into the local auditEvents array and calls renderAll() — but makes no HTTP request. The retry does not actually trigger any server-side re-enrichment. The audit event disappears after the next refreshFromApi(). This is a stub that was never wired to a backend, but there is no visual indication to the operator that the button is non-functional beyond a transient log entry.
영향: Operators who click 'Retry failed' believe a re-enrichment has been triggered. No work actually occurs, and the next refresh silently removes the fake log entry. This can cause operators to miss genuine failures.
권장: Either wire the button to POST /api/submissions/{id}/rerun-enrichment or POST /api/submissions/{id}/search-auto, or disable and mark the button as 'not yet available' so operators are not misled. The existing rerunEnrichment() function (line 2484) already implements the correct API call pattern.

19. offline-install.md omits Ollama model bundling procedure

분류 build · 파일 docs/operations/offline-install.md:1-60 · 신뢰도 high · 검증 unverified · 유닛 bld-airgap
설명: The offline-install.md covers only Python wheels (numpy/opencv/pillow). It does not mention Ollama or the qwen2.5:0.5b-instruct model weights, which must also cross the air gap. _llm_assistant in src/rights_filter/integrations/env_clients.py:239–245 always constructs an OllamaGenerateLlmClient pointing to http://127.0.0.1:11434 (defaulting to the local Ollama daemon). If Ollama is installed on the target but the model was never pulled on the build machine and transferred, every rerun-enrichment call will fail with an ENRICHMENT_FAILURE evidence record (2-second connect timeout, graceful degradation via InternalLlmAssistant.summarize catch-all at llm_assistance.py:71). The runtime failure is non-crashing, but the operator will see silent LLM failures on every submission in a fully air-gapped deploy without documentation telling them why.
영향: LLM-based evidence summaries silently fail on air-gapped targets where Ollama is installed but the model cache was not transferred. Operators may not understand why the feature is broken since the runbook does not mention this requirement.
권장: Add an 'Optional: Ollama LLM' section to docs/operations/offline-install.md covering: (1) On the build machine: ollama pull qwen2.5:0.5b-instruct then locate and copy ~/.ollama/models/ (or the path in OLLAMA_MODELS). (2) On the target: install Ollama binary offline, point OLLAMA_MODELS at the transferred model cache. (3) Note that LLM is optional — if Ollama is absent, analysis proceeds without LLM summaries, which is acceptable. copyrighter-operation-worklist.md:196 already documents ollama pull for online setups; cross-reference that and add the offline variant.

20. json.loads() on DB rows with no error handling — malformed payload propagates as uncontrolled 500

분류 error-handling · 파일 src/rights_filter/server/store_persistence.py:229, 257, 294, 303 · 신뢰도 medium · 검증 unverified · 유닛 sec-sql
설명: All four json.loads() calls that deserialize stored payload rows (_get at line 229, _all at line 257, _evidence_for_submission at line 294, _evidence_by_submission at line 303) are bare — no try/except. The json_valid(payload) CHECK constraint guards INSERT/UPDATE paths and will catch newly-written bad rows, but it does not protect reads: legacy rows inserted before the constraint migration, rows written directly to the DB file outside this application, or any future code path that bypasses _put() would produce a JSONDecodeError. That exception propagates through the request handler without being caught, causing an unhandled 500. Depending on how the HTTP layer formats that error, a Python traceback including the file path, DB path, and partial payload value could be sent to the client.
영향: A single corrupt row can render an entire API response (bootstrap(), review(), audit_events()) unavailable and may leak internal path or data structure information if the traceback reaches the operator GUI.
권장: Add a try/except json.JSONDecodeError around each json.loads() in the persistence layer. On failure, log the error with the table and row id (but not the raw payload content) and either skip the row with a sentinel value or raise a domain-level StorageCorruptionError that the HTTP handler maps to a 500 with a safe static message. Example: try: return json.loads(row['payload']) except json.JSONDecodeError as exc: raise StorageCorruptionError(f'{table}/{id_value} has corrupt JSON payload') from exc.

21. _response_text falls back to 'Evidence summary unavailable.' without logging or raising

분류 error-handling · 파일 src/rights_filter/analysis/llm_assistance.py:138-149 · 신뢰도 high · 검증 unverified · 유닛 err-silent
설명: _response_text (line 138) attempts three different response-key patterns from the Ollama API response. If none match — including if Ollama silently returns a non-empty but structurally unexpected response body — the function silently returns the hardcoded fallback "Evidence summary unavailable." (line 149) with no log, no metric, and no signal to the caller. The caller OllamaGenerateLlmClient.summarize_evidence (line 50–56) will propagate this as if it were a normal summary string. InternalLlmAssistant.summarize then persists it as EvidenceSource.LLM_SUMMARY with verified=True when source URLs exist.
영향: Silent API contract breakage: if Ollama's response format changes, every LLM evidence record becomes a fabricated 'unavailable' stub stored with full verified=True status, invisible to operators.
권장: When all key patterns miss on a non-empty response body, log at WARNING level with the actual response keys (sanitized to avoid PII) and return the fallback only after logging. Alternatively, raise HttpJsonError so InternalLlmAssistant.summarize stores an ENRICHMENT_FAILURE record that is auditable.

22. _sync_face_crops swallows image load failure without any log or audit trail

분류 error-handling · 파일 src/rights_filter/server/store_enrichment.py:150-153 · 신뢰도 high · 검증 unverified · 유닛 err-silent
설명: _sync_face_crops (line 143) wraps image_store.image_payload(submission_id) in a bare except Exception: return (lines 150–153). The method is called from both the main seed loop (sqlite_store.py:248) and _rerun_internal_analysis (store_enrichment.py:270). When the image file is missing or corrupted, the face-crop sync silently returns: no log entry, no audit event, and the submission's faceCrops field is never set (left as absent or stale from a prior run). Downstream calls such as _google_face_crop_web_detection then read submission.get("faceCrops") and may use stale bounding boxes from a previous iteration.
영향: Stale faceCrops from a previous analysis survive in the submission record, causing _google_face_crop_web_detection to attempt Google Vision calls on face boxes from an image that may no longer exist, wasting quota and potentially generating misleading evidence. The failure is entirely invisible in the audit log.
권장: Replace except Exception: return with except Exception as exc: logging.warning("face crop sync skipped for %s: %s", submission_id, exc); return. Optionally clear faceCrops on the submission before returning so stale boxes are not left in place.

23. _store_candidate_image silently discards network and fingerprint errors with no log

분류 error-handling · 파일 src/rights_filter/server/store_search_candidates.py:664-679 · 신뢰도 high · 검증 unverified · 유닛 err-silent
설명: _store_candidate_image has two bare except Exception blocks: one at line 672 that discards errors reading a cached image from disk (returning record = None), and another at line 678 that discards all fetch errors (returning None to the caller). Neither block logs the error type, URL, or submission ID. This method is called inside tight loops in _search_result_image_similarity_evidence, _face_crop_search_result_similarity_evidence, _collection_candidates_from_evidence, and _candidate_payload_from_evidence. A systematic SSRF guard rejection, a 4xx/5xx HTTP error, or a disk-full OSError all produce the same silent None return, making it impossible to distinguish expected skip conditions (non-image URL) from real infrastructure failures.
영향: SSRF guard rejections, rate-limit 429 responses, and disk-full errors are all silently swallowed. An operator running keyword collection or manual search sees 0 candidates returned with no explanation. There is no way to detect a systematic provider failure from logs alone.
권장: Add at minimum a logging.debug (for expected non-image URLs) and logging.warning (for network or OS errors) inside each except block, including the URL and a truncated exception message. The SSRF ValueError in particular should be logged at WARNING to make attacker-induced SSRF attempts visible.

24. mark_evidence_status() writes evidence and audit in separate un-transacted calls

분류 error-handling · 파일 src/rights_filter/server/sqlite_store.py:587-617 · 신뢰도 high · 검증 unverified · 유닛 mnt-hotspots
설명: mark_evidence_status() (lines 596–617) performs: (a) self._put('evidence', ...) at line 609, (b) self._rescore_submission() at line 610, (c) self.add_audit_event() at line 611 — three separate DB writes without a wrapping _transaction(). If the process dies between (a) and (c), the evidence status is updated but no audit trail is recorded. This is explicitly listed as a remaining deferred item in the Prior Review Digest ('mark_evidence_status atomic state+audit transaction — same _transaction() pattern as record_decision, not yet applied').
영향: Evidence status changes can be recorded without an audit trail if the server crashes between the evidence write and the audit write. For a rights-review tool this is a compliance risk.
권장: Wrap the body of mark_evidence_status() in with self._transaction() as conn: and pass conn=conn to both self._put and self.add_audit_event, matching the pattern used in record_decision and promote_collection_candidate.

25. ScoreResult.reasons is a mutable list inside a frozen dataclass, allowing inadvertent mutation

분류 maintainability · 파일 src/rights_filter/domain/records.py:71-75 · 신뢰도 high · 검증 unverified · 유닛 bug-analysis
설명: ScoreResult is declared @dataclass(frozen=True) to signal immutability, but reasons: list[str] = field(default_factory=list) makes the reasons attribute a mutable list. Python's frozen dataclass prevents attribute reassignment (result.reasons = [...] raises FrozenInstanceError) but does NOT prevent mutation of the list object itself (result.reasons.append(...) silently succeeds). Any code path that holds a reference to the ScoreResult can mutate the reasons list after the fact, violating the immutability guarantee the frozen annotation implies. This is particularly risky because run.score (an AnalysisRun field) holds the ScoreResult, and run is shared across multiple code paths in EvidenceEnricher.enrich_latest.
영향: Any caller that obtains a ScoreResult and mutates .reasons will silently corrupt shared state. In the current AnalysisRun, where the same score object is read by the operator-facing review presenter after enrichment, a concurrent mutation (in a multi-threaded ThreadingHTTPServer context) would cause non-deterministic reason lists to appear in the UI.
권장: Change reasons to tuple[str, ...] (immutable) and update all callers to pass tuple(_unique(reasons)) at construction time. Alternatively, use reasons: list[str] = field(default_factory=list) with __post_init__ converting to a tuple: object.__setattr__(self, 'reasons', tuple(self.reasons)). Update ScoreResult usages in tests and RiskScorer.score accordingly.

26. window.prompt() used synchronously inside async event handlers — blocks event loop

분류 maintainability · 파일 web/operator-gui/app.js:3347, 3363 · 신뢰도 high · 검증 unverified · 유닛 bug-frontend
설명: deactivateKnowledgeEntry (line 3347) and reactivateKnowledgeEntry (line 3363) call window.prompt() synchronously. This freezes the browser tab while the native dialog is open, blocking all scheduled microtasks and macro-tasks. It also fails silently in some automated and sandboxed environments (e.g. Playwright tests, iframes with allow-modals removed) where window.prompt() returns null immediately. For deactivateKnowledgeEntry the null return means the function exits early (correct); for reactivateKnowledgeEntry an empty string is indistinguishable from the user pressing Cancel vs. OK with no text.
영향: Blocks the event loop during dialog display; breaks in sandboxed iframes and some test harnesses.
권장: Replace window.prompt() with an inline form rendered into the DOM (same pattern as the knowledge-edit-form shown at lines 1883-1903) or a modal dialog. This is consistent with the rest of the UI which uses inline form patterns. If window.prompt must remain, document the Playwright test expectation for page.on('dialog', ...) so CI does not hang.

27. app.js is 3926 lines — far exceeds 800-line file limit

분류 maintainability · 파일 web/operator-gui/app.js:1-3926 · 신뢰도 high · 검증 confirmed · 유닛 mnt-hotspots
설명: app.js is nearly 4000 lines. The project standard is 800 lines maximum. The file contains at least six distinct responsibility clusters that are already partially separated for Python code: state management and bootstrap (lines 1–230), formatting/labeling utilities (lines 526–830), DOM rendering functions for each view (lines 1113–2183), event orchestration functions (lines 2328–3430), the event dispatcher (lines 3458–3824), and the init/boot sequence (lines 3828–3906). The file was deliberately left as a monolith for the frontend while the Python layer was split into mixins, but at 4k lines it violates the project standard and makes targeted review, testing, and parallel editing impractical.
영향: No runtime bug today, but the file size makes diff review, targeted hot-fixes, and parallel module extraction significantly harder. Any PR touching this file touches 4k lines.
권장: Split into at least six ES modules or IIFE namespaces loaded via <script defer> tags (already the pattern used for operator-labels.js, submission-import.js, evidence-guidance.js, operator-search.js). Suggested split: (1) app-state.js — state object, replaceCollection, applyBootstrap, clearRuntimeData (lines ~1–230). (2) app-format.js — escapeHtml, safeUrl, formatReason, formatMatchType, formatDomain, coveragePercent, coverageTone, and all other pure formatting helpers (lines ~526–830). (3) app-render-queue.js — renderQueue, renderCoverageTabs, renderImportStatus, renderProviderPulse. (4) app-render-workbench.js — renderCaseReview, renderEvidenceGroups, renderEvidenceRow, renderEvidenceSearch. (5) app-render-knowledge.js — renderKnowledgeBase, renderCollectionCandidates, renderCorrections, renderProviderControls, renderAuditLog. (6) app-actions.js — setDecision, runManualSearch, rerunEnrichment, uploadSubmissionImage, addKnowledgeEntry, and all async action functions. (7) app-events.js / app.js (thin) — bindEvents, init, the DOMContentLoaded boot. Keep __uiCopyCompatibility in the file that owns the constants it shadows (app-format.js) per project memory.

28. NaverSearchAdapter has three search() / search_pages() / search_web_pages() methods with nearly identical policy-check-then-call-then-error structure

분류 maintainability · 파일 src/rights_filter/integrations/naver_search.py:158-286 · 신뢰도 high · 검증 unverified · 유닛 mnt-hotspots
설명: The three adapter methods (search at line 158, search_pages at line 200, search_web_pages at line 244) all follow this identical boilerplate: (1) isinstance(query, str) guard, (2) _requested_calls with different attribute name, (3) policy.can_call → SEARCH_SKIPPED evidence, (4) try: client.search_X(query) / policy.record_call / _map_X_response, (5) except: ENRICHMENT_FAILURE evidence. The only variation is the client method called, the query_signature function, the search_type tag in the data dict, and the label in the failure reason string. GoogleCustomSearchAdapter.search_images / search_web_pages (lines 94–182) has the same two-method duplication. Together these six adapter methods replicate ~100 lines of boilerplate.
영향: A change to the SEARCH_SKIPPED evidence format or policy.can_call signature requires touching six methods across two files. Medium risk of copy-paste drift over time.
권장: Extract a private _run_search(submission_id, query, *, search_type, client_fn, signature_fn, requested_calls, source, failure_label) helper in each adapter (or a shared base class). Each public method becomes 4–6 lines that call the helper with its specific parameters. This mirrors what GoogleCustomSearchClient._search_pages() already does at the HTTP level.

29. collect_keyword_candidates() is 85 lines with a repeated 3-phase naver fallback pattern

분류 maintainability · 파일 src/rights_filter/server/store_operations.py:568-652 · 신뢰도 high · 검증 unverified · 유닛 mnt-hotspots
설명: Lines 615–638 repeat the same naver fallback pattern twice: 'if provider == "naver" and collected == 0: call search_pages / extend candidates' (lines 615–626), then 'if provider == "naver" and collected == 0: call search_web_pages / extend candidates' (lines 627–638). The two blocks are identical in structure and only differ by calling search_pages vs search_web_pages and incrementing provider_call_count. The guard condition (collected == 0) is checked anew each time, so if search_pages populates collected, the web block still runs its guard check independently — the intent appears to be a cascade (image → blog → web only if prior stage empty), but this is achieved via sequential if-not-if-else, which is less legible than a loop or chain.
영향: Current code is correct (the second naver if will only run if collected is still 0 after the first), but it takes a careful reading to confirm that. Adding a fourth fallback in the future will require copy-pasting a third block.
권장: Replace the three provider-specific if-blocks with a small list of fallback callables: fallbacks = [(naver_adapter.search, naver_policy), (naver_adapter.search_pages, ...), (naver_adapter.search_web_pages, ...)] and iterate with early-break when collected > 0. This makes the cascade intent explicit and collapses 50 lines into ~15.

30. styles.css at 2819 lines is nearly 4x the 800-line ceiling

분류 maintainability · 파일 web/operator-gui/styles.css:1-2819 · 신뢰도 high · 검증 unverified · 유닛 mnt-hotspots
설명: The single stylesheet contains custom properties / reset, layout utilities, all five view panels, responsive breakpoints for each, component styles (evidence cards, provider chips, coverage tabs, knowledge entries, candidates, corrections, audit log), and animation/accessibility overrides. At 2819 lines a single diff touching layout can conflict with a diff touching a component style.
영향: No runtime risk. Significantly degrades review velocity and the ability to audit CSS scope creep for any particular view.
권장: Split into colocated files: base.css (custom properties, reset, layout shell), view-queue.css, view-workbench.css, view-knowledge.css, view-providers.css, view-audit.css, components.css (chips, badges, buttons, evidence cards), responsive.css (all @media). Since the project is air-gapped and uses a plain HTTP server with no bundler, combine them at build time (a simple cat into styles.css is sufficient) or load them separately in index.html.

31. Auto-search dedup loads entire evidence table instead of a targeted per-submission query

분류 performance · 파일 src/rights_filter/server/store_enrichment.py:410-411, 576-578 · 신뢰도 high · 검증 unverified · 유닛 bug-store
설명: _auto_google_custom_search (line 411) and _auto_naver_search (line 577) each call self._evidence_by_submission() with no arguments, which reads and JSON-parses every row in the evidence table across all queues and submissions, then immediately discards all but the target submission's slice via .get(submission_id, []). The method _evidence_for_submission(submission_id) exists in StorePersistenceMixin and executes a targeted 'WHERE submission_id = ?' query. With 500 submissions at 20 evidence items each, each auto-search dedup check deserializes approximately 10,000 JSON payloads to consume 20 of them.
영향: O(total-evidence) full-table scan on every per-submission rerun or auto-search invocation. Degrades linearly as the queue grows and worsens under concurrent reruns from multiple operator sessions.
권장: Replace self._evidence_by_submission().get(submission_id, []) with self._evidence_for_submission(submission_id) at both call sites. _evidence_for_submission already strips the submission_id key and returns list[dict], matching what _existing_naver_query_signatures and _existing_google_custom_query_signatures expect.

분류 security · 파일 src/rights_filter/server/http_app.py:289-291 · 신뢰도 high · 검증 confirmed · 유닛 sec-http
설명: The session cookie is set as {_SESSION_COOKIE}={auth_token}; HttpOnly; SameSite=Strict; Path=/. Two problems: (1) The cookie value IS the raw COPYRIGHTER_AUTH_TOKEN secret. Knowing the cookie value is identical to knowing the master credential, so any cookie-reading side-channel (e.g., shared browser profile, cookie-jar file exposure) directly exposes the token. A proper design uses an opaque session ID derived via HMAC from the token so the two are decoupled. (2) The Secure attribute is absent. When the operator browses over HTTPS (e.g., through a reverse proxy), the browser will also send this cookie over plain HTTP sub-requests, violating cookie-security best practice and potentially enabling interception. The Secure attribute is safe to add even on localhost because modern browsers honour it for 127.0.0.1.
영향: Cookie theft or file-based exposure reveals the master auth token, giving an attacker full API access.
권장: Replace the raw token value in the cookie with an HMAC-derived session tag: session_tag = hmac.new(auth_token.encode(), b'session', 'sha256').hexdigest(). Store and validate the tag instead of the token. Add the Secure attribute to the Set-Cookie directive: f"{_SESSION_COOKIE}={session_tag}; HttpOnly; Secure; SameSite=Strict; Path=/".

33. No minimum-entropy check on COPYRIGHTER_AUTH_TOKEN

분류 security · 파일 src/rights_filter/server/__main__.py:44-46 · 신뢰도 high · 검증 unverified · 유닛 sec-http
설명: Any non-empty string is accepted as auth_token (line 44: .strip() or None). A one-character token like a is silently treated as fully valid. hmac.compare_digest provides timing-safe comparison but cannot compensate for a guessable or trivially short secret. There is no startup warning or rejection for weak tokens, only a warning when the token is entirely absent.
영향: An operator who sets a short, memorable token (e.g., admin, 1234) allows brute-forcing or guessing of the auth credential.
권장: At startup, after reading auth_token, enforce a minimum length and entropy: reject tokens shorter than 32 characters with a hard error (sys.exit(1)) and emit a warning for tokens that are not at least 20 bytes of apparent random material. Recommend generating tokens with python -c "import secrets; print(secrets.token_urlsafe(32))".

34. No security headers on main application responses (missing CSP, X-Frame-Options, X-Content-Type-Options)

분류 security · 파일 src/rights_filter/server/http_app.py:303-312 · 신뢰도 high · 검증 unverified · 유닛 sec-http
설명: The _static handler serves index.html, app.js, and all static assets via _file(target) (no untrusted flag). This path sets only Content-Type and Content-Length. Security headers — Content-Security-Policy, X-Frame-Options, X-Content-Type-Options, Referrer-Policy — are absent. In contrast, untrusted media files (lines 327-342) correctly receive X-Content-Type-Options: nosniff, CSP with sandbox, Cache-Control: no-store, and Referrer-Policy. The asymmetry means the main application page can be framed (clickjacking) and has no CSP to restrict script sources.
영향: Without X-Frame-Options or a frame-ancestors CSP directive, any page reachable from localhost can embed the operator console in an iframe for clickjacking attacks.
권장: In _file, when untrusted is False (the trusted static app path), add at minimum: X-Content-Type-Options: nosniff, X-Frame-Options: SAMEORIGIN, and a narrow CSP such as default-src 'self'; script-src 'self'; style-src 'self'; font-src 'self'; img-src 'self' data:; connect-src 'self'.

35. No rate limiting on authentication or any API endpoint

분류 security · 파일 src/rights_filter/server/http_app.py:249-274 · 신뢰도 high · 검증 unverified · 유닛 sec-http
설명: Every request to a protected route goes through _require_auth which calls hmac.compare_digest against the token, but there is no brute-force protection. An attacker who can reach the server (including via DNS rebinding or a local network peer) can submit unlimited authentication attempts at the rate the Python server can handle them. ThreadingHTTPServer has no connection or request-rate cap. The same lack of limiting applies to computationally expensive routes like /api/submissions/{id}/rerun-enrichment and /api/collections/candidates/promote-batch.
영향: Allows automated brute-forcing of the auth token, particularly dangerous when operators choose short or dictionary-word tokens.
권장: For a single-operator local server the most practical mitigation is a per-IP failed-attempt counter with a short lockout (e.g., 5 failures → 30-second lockout) stored in a thread-safe dict in the closure. Also add a time.sleep on auth failure to slow sequential attempts. For production deployments a reverse proxy (nginx) with limit_req is the correct layer.

36. CSRF protection relies solely on SameSite=Strict with no Origin validation fallback

분류 security · 파일 src/rights_filter/server/http_app.py:249-274 · 신뢰도 medium · 검증 unverified · 유닛 sec-http
설명: Cookie-authenticated POST and PATCH routes depend entirely on SameSite=Strict for CSRF protection. This is generally sufficient for modern browsers, but SameSite=Strict is not honoured by all older browsers and can be bypassed in some DNS-rebinding scenarios (attacker sets the cookie origin to 127.0.0.1). There is no secondary Origin or Referer header check in _require_auth. Bearer-token-authenticated requests (header-based) are inherently CSRF-safe, but once the browser has the session cookie any cross-origin form POST from a co-located malicious page (on localhost, same effective origin) could succeed.
영향: In a DNS-rebinding attack or with legacy browsers, a malicious local page could issue state-changing API calls (approve/reject submissions, disable providers) using the operator's session cookie.
권장: Add an Origin/Referer check in _require_auth for non-GET requests authenticated via cookie: verify that the Origin or Referer header matches the server's own Host. This is a simple, stateless defence that does not require a CSRF token. Example: origin = self.headers.get('Origin', ''); if origin and origin not in allowed_origins: return self._json({'error': 'forbidden'}, HTTPStatus.FORBIDDEN).

37. UrllibJsonTransport reads API responses without a size limit

분류 security · 파일 src/rights_filter/integrations/http_json.py:39 · 신뢰도 high · 검증 unverified · 유닛 sec-ssrf
설명: Line 39 calls response.read() with no argument, reading the entire HTTP response body into memory in one shot. This applies to every outbound API call: Google Vision, Google Custom Search, Naver, and the Ollama LLM. There is no max-bytes cap. By contrast, store_remote_fetch.py correctly uses response.read(limit + 1) and enforces CANDIDATE_IMAGE_FETCH_LIMIT = 8_000_000. A malformed, adversarial, or network-intercepted response from any of these endpoints (or a misconfigured Ollama instance returning a runaway generation) could exhaust server memory and crash the process. The same unbounded read is performed on the error body at line 41 (exc.read()).
영향: A single adversarially large response (or runaway LLM output) can exhaust process memory; in a long-running server process this causes an OOM crash, taking the entire review service offline.
권장: Add a size cap consistent with the expected maximum API response, for example 4 MB for Google/Naver JSON and 1 MB for the Ollama /api/generate response. Apply response.read(MAX_RESPONSE_BYTES + 1) and raise HttpJsonError if the result exceeds the limit, mirroring the pattern already used in store_remote_fetch._fetch_url_bytes_with_headers.

38. Google and Custom Search API keys embedded in outbound request URLs expose them in Python tracebacks

분류 security · 파일 src/rights_filter/integrations/cloud_vision_web_detection.py:57 · 신뢰도 medium · 검증 unverified · 유닛 sec-ssrf
설명: The Google Vision REST client builds the request URL as f"{self.endpoint}?key={quote_plus(self.api_key)}" (line 57). GoogleCustomSearchClient._search_pages places 'key': self.api_key in the params dict that is passed to urlencode and appended to the URL (google_custom_search.py line 66, 75). Python's urllib stores the full URL as the Request.full_url attribute. If an unhandled exception occurs anywhere in the call stack — including inside the socket.create_connection path in _probe_connect — the full URL including ?key=<secret> will appear in the Python traceback (local variable dump via -v or framework-level logging). The Naver secret is sent as an HTTP header (X-Naver-Client-Secret), which does NOT appear in URL-level tracebacks, so Naver is not affected by this specific path.
영향: If Python tracebacks are captured by any logging infrastructure (stdout/stderr redirect, crash reporters, or future logging added to __main__.py), the raw API key is exposed in plaintext log lines, violating the no-secrets-in-logs rule.
권장: Pass the API key via a POST body or a custom HTTP header instead of the URL query string (Google Vision already accepts Authorization: Bearer via service-account credentials as an alternative). If the key must stay in the URL, strip it from any exception message before re-raising and ensure the server's exception handler does not log tracebacks that include local variables in production.

39. innerHTML templates inject server-derived label lookups without escapeHtml

분류 security · 파일 web/operator-gui/app.js:1223, 1235, 1381, 1387, 1574 · 신뢰도 high · 검증 unverified · 유닛 sec-xss
설명: Four innerHTML template sites insert riskLabels[submission.riskBand], submission.riskScore, and decisionLabels[submission.decisionStatus] as raw interpolations without escapeHtml(). Examples: line 1223 ${riskLabels[submission.riskBand]} ${submission.riskScore}, line 1235 ${decisionLabels[submission.decisionStatus]}, lines 1381 and 1387 in recommendation-box, and line 1574 in renderEvidenceSummary. The current blast radius is low because (a) riskBand and decisionStatus are constrained by SQLite CHECK constraints to short enum strings, (b) riskScore is typed integer at the DB and Python layers, and (c) the Korean label values in operator-labels.js contain no HTML. However, this is defence-in-depth missing: if a future migration widens the CHECK constraint, adds a new status value containing < or >, or the DB-to-API serialisation layer is modified to pass a string through, these unescaped slots become DOM-based stored XSS sinks. The app shell is also served with no Content-Security-Policy header (see separate finding), so there is no fallback layer.
영향: If DB integrity or serialisation assumptions break, stored XSS payloads in riskBand/decisionStatus strings execute without any CSP backstop in the operator's browser.
권장: Wrap every server-derived value in escapeHtml() before interpolating into an innerHTML template, even when the upstream type is expected to be numeric or enumerated. Change line 1223 to ${escapeHtml(riskLabels[submission.riskBand] ?? '')} ${escapeHtml(String(submission.riskScore))}, line 1235 to ${escapeHtml(decisionLabels[submission.decisionStatus] ?? '')}, and apply the same pattern at lines 1381, 1387, and 1574. For riskScore, searchCount, submission.evidence.length, and query.count (line 1827), cast to Number() and verify Number.isFinite before inserting — this costs nothing and removes a future type-confusion path.

40. No Content-Security-Policy header on the app shell or API responses

분류 security · 파일 src/rights_filter/server/http_app.py:314-345 · 신뢰도 high · 검증 unverified · 유닛 sec-xss
설명: The _file() method only sets a CSP header when untrusted=True (i.e., for operator-uploaded and externally-fetched media files served under /media/, /knowledge-media/, etc.). The app shell (index.html, app.js, *.css) and all /api/ JSON responses are served with no Content-Security-Policy, no X-Frame-Options, and no X-XSS-Protection header. An attacker who achieves any DOM-based XSS (or a future reflected/stored vector) gets unrestricted script execution because there is no browser-level fallback. There is also no frame-ancestors directive, so the page can be framed by any same-site page — relevant because the tool handles sensitive copyright review decisions.
영향: Any XSS vulnerability (present or future) will have no CSP defence. The absence of X-Frame-Options / frame-ancestors allows UI-redress attacks if another page on the same host is compromised.
권장: Add security headers for the trusted app shell in _file() when untrusted is False and the content type is text/html or application/javascript. At minimum add: Content-Security-Policy: default-src 'self'; script-src 'self'; style-src 'self'; img-src 'self' data: blob:; font-src 'self'; connect-src 'self'; frame-ancestors 'none' and X-Frame-Options: DENY. Because the current app uses no inline scripts or CDN resources, this strict policy should be compatible without modification.

41. riskClass() injects unvalidated server string into class attribute without escaping

분류 security · 파일 web/operator-gui/app.js:841-843, 1199, 1223, 1381, 1574 · 신뢰도 medium · 검증 unverified · 유닛 sec-xss
설명: riskClass(submission.riskBand) produces risk-<riskBand> and this string is interpolated raw into class="..." attributes inside innerHTML templates (e.g. line 1199 class="queue-row ${...}", line 1223 class="risk-badge ${riskClass(...)}")). If riskBand contained a " followed by an event handler (e.g. " onmouseover=alert(1) x="), it would break out of the attribute. The same pattern exists with sourceClass() at lines 1725, 1819, 1973 — though sourceClass() maps through an allowlist so the return value is always a fixed string (source-naver, source-google, etc.), making those safe. riskClass() does not allowlist; it concatenates directly. The SQLite CHECK constraint currently prevents non-enum values, but there is no client-side guard.
영향: CSS class injection that could break attribute boundaries if the DB constraint is relaxed or bypassed. Lower practical risk than a direct text node injection, but the pattern is inconsistent with sourceClass() which does allowlist.
권장: Either (a) add an allowlist inside riskClass: const VALID_RISK_BANDS = new Set(['high','medium','low','failed','pending']); return 'risk-' + (VALID_RISK_BANDS.has(riskBand) ? riskBand : 'pending'); or (b) apply escapeHtml() to the return value before using it in an attribute. Option (a) is preferable — it also tightens the logic semantically.

42. Silent EXIF/GPS metadata leakage when PIL re-encoding fails in build_external_derivative

분류 security · 파일 src/rights_filter/analysis/preprocessing.py:47-49 · 신뢰도 high · 검증 unverified · 유닛 bug-analysis
설명: In build_external_derivative, after calling _reencode_without_metadata(original.content, width, height), the fallback if content is None: content = original.content silently substitutes the ORIGINAL bytes — including any embedded EXIF/GPS/ICC data — when PIL is unavailable or decoding fails. The comment on lines 44-47 correctly notes that clearing only the metadata dict is insufficient because camera serials and GPS coordinates may survive inside the bytes. The fallback therefore undoes that protection without any log message, warning, or raised exception. The bytes in original.content are then used in the returned ImagePayload which callers pass to external APIs (Google Vision, Naver image search). An air-gapped deployment where PIL/Pillow is not installed would hit this path on every submission.
영향: Camera GPS coordinates, serial numbers, and other EXIF data from operator-uploaded images may be transmitted to third-party external APIs (Google Vision, Naver image search) without the operator's knowledge. This is a privacy/data-governance violation in the context of the product's stated policy invariants around not sending original images to external APIs.
권장: At minimum, log a warning when the fallback fires: import logging; logging.getLogger(__name__).warning('PIL re-encode failed; sending original bytes to external API — EXIF/GPS may be present'). For stricter enforcement, raise PreprocessingError('cannot strip metadata: PIL unavailable') so callers can fail safely rather than silently leak. The choice between warn-and-continue vs. hard-fail should be configurable at the integration boundary, but silent success is never acceptable here given the documented privacy requirement.

43. submission.asset assigned directly to img.src without safeUrl — inconsistent URL sanitisation

분류 security · 파일 web/operator-gui/app.js:1203, 1314 · 신뢰도 high · 검증 confirmed · 유닛 bug-frontend
설명: Two places assign submission asset URLs directly to <img src> without passing through safeUrl: (1) queue thumbnail at line 1203: src="${escapeHtml(submission.asset)}", and (2) the workbench case image at line 1314: document.getElementById('case-image').src = submission.asset. Evidence thumbnails and all external URLs elsewhere in the codebase correctly pass through safeUrl (e.g. lines 1766, 1788, 1984). submission.asset is server-supplied; if the server were ever to return a javascript:, data:, or vbscript: scheme URL for an asset, the browser would execute or render it directly. The escapeHtml call on line 1203 only prevents HTML injection in the attribute string — it does not block non-http schemes.
영향: Potential XSS via a malicious asset URL returned by the server. Defense-in-depth gap relative to the established safeUrl pattern applied everywhere else.
권장: Wrap both uses with safeUrl: line 1203 → src="${escapeHtml(safeUrl(submission.asset))}", and line 1314 → document.getElementById('case-image').src = safeUrl(submission.asset). Also apply safeUrl to item.asset for similar-strip items at line 1344 and crop.url for face-crop strip at line 1360. Add an onerror handler on the case image for consistency with evidence thumbnails (line 1768).

44. similar-strip and face-crop-strip image URLs not passed through safeUrl

분류 security · 파일 web/operator-gui/app.js:1336-1365 · 신뢰도 high · 검증 unverified · 유닛 bug-frontend
설명: The similar-strip at line 1344 uses src="${escapeHtml(item.asset)}" and href="${escapeHtml(item.asset)}" and the face-crop strip at line 1360 uses href="${escapeHtml(crop.url)}" and src="${escapeHtml(crop.url)}" — none of these go through safeUrl. These values come from the bootstrap API response. The pattern is inconsistent with evidence thumbnails which always use safeUrl. See the parent finding above (submission.asset) for the security justification.
영향: Same XSS defence-in-depth gap as the submission.asset finding; lower exploitability because these fields come from Google Vision / internal image store, but inconsistency means the guarantee is not maintained.
권장: Wrap item.asset and crop.url in safeUrl before embedding in src and href attributes, matching the pattern used for all other external URLs in the codebase.

45. Media route authentication is not verified under auth_token enforcement

분류 security · 파일 tests/rights_filter/server/test_http_app.py:1318-1354 · 신뢰도 high · 검증 unverified · 유닛 tst-coverage
설명: test_protected_routes_require_token_when_configured (line 1318) tests /api/bootstrap with and without a Bearer token and verifies the session cookie is issued. It also checks that /health and / are open. However, it never fetches /media/, /knowledge-media/, /collected-media/, or /face-crop-media/ without the token to confirm those routes return 401 when auth_token is set. _PROTECTED_PREFIXES in http_app.py line 48 explicitly includes all four media prefixes, but the test only exercises the API prefix. A test gap here means a regression (e.g., accidentally removing a media prefix from _PROTECTED_PREFIXES) would not be caught.
영향: Silent regression risk: media routes holding biometric face crops and collected candidate images could become openly accessible without any test failing.
권장: In the existing auth test, add requests to /media/images/sample.svg with no credentials and assert 401, then verify the same URL succeeds with the Bearer token or session cookie. Cover at least one path under each of the four media prefixes.

46. Security headers on untrusted media (_file with untrusted=True) are not asserted in any test

분류 security · 파일 tests/rights_filter/server/test_http_app.py:1140-1168 · 신뢰도 high · 검증 unverified · 유닛 tst-coverage
설명: http_app.py lines 327-344 emit X-Content-Type-Options: nosniff, Content-Security-Policy: default-src 'none'; ... sandbox, Cache-Control: no-store, and Referrer-Policy: no-referrer for every untrusted media response. These headers are the primary XSS/SVG-script defence for operator-uploaded images. The only security-header test is test_http_server_serves_bundled_font_with_mime_and_immutable_cache (line 1140), which checks trusted static assets. No test fetches /media/, /knowledge-media/, /collected-media/, or /face-crop-media/ and verifies the Content-Security-Policy or no-store header is present.
영향: A future refactor that accidentally removes the untrusted=True flag from a file-serving call, or removes the sandbox from the CSP string, will not be caught by the test suite.
권장: Add a test that GETs a /media/ path and inspects the response headers, asserting Content-Security-Policy contains sandbox, Cache-Control equals no-store, X-Content-Type-Options equals nosniff, and Referrer-Policy equals no-referrer. Mirror the approach used in test_http_server_serves_bundled_font_with_mime_and_immutable_cache.

47. safeUrl / escapeHtml XSS defence in app.js is structural-only; no behavioural assertions

분류 security · 파일 tests/operator_gui/test_static_workbench.py:36-862 · 신뢰도 medium · 검증 unverified · 유닛 tst-coverage
설명: The 41 tests in test_static_workbench.py verify function and class names exist in the source text (e.g., assert 'safeUrl' not found... actually safeUrl is used 128 times in app.js but there is no assertion that function safeUrl exists or that it handles javascript:, //host, /\host, or data: inputs correctly). The test suite has no unit test for safeUrl (lines 544-563 of app.js) or escapeHtml. Since these are plain JavaScript functions in a non-module IIFE, they can be tested via Playwright's page.evaluate() or via a small Node.js script invoked in CI.
영향: Any future edit to safeUrl that inadvertently allows javascript: or protocol-relative URLs would not be caught before merge.
권장: Add a Playwright test or a test_static_workbench.py assertion that reads the app.js source and confirms function safeUrl is present, OR add a browser-level test using page.evaluate to call safeUrl('javascript:alert(1)') and assert it returns '#', and safeUrl('//evil.example.com') returns '#'.

48. No regression test for the Naver blog/web dedup fix (commit `71a6da1`)

분류 testing · 파일 tests/rights_filter/server/test_sqlite_store.py:6753-6913 · 신뢰도 high · 검증 confirmed · 유닛 tst-coverage
설명: Commit 71a6da1 fixed a bug where a duplicate blog-query signature used a shared continue that also skipped the web search for that iteration. The fix splits blog and web into independent guards. No test exercises the specific scenario: run auto-search twice on the same submission with the same query so the blog signature is a duplicate, and assert that the web search still executes. The existing tests (lines 6753 and 6834) only exercise the successful fallback path; they never set auto_naver_blog_query_limit to zero while keeping auto_naver_web_query_limit > 0, nor do they verify that a deduplicated blog signature does not suppress the web call.
영향: The original bug — web search being silently dropped whenever a blog query was a duplicate — could silently regress with no test catching it.
권장: Add a test that seeds a submission, runs seed_from_image_store a second time (so the blog signature already exists in existing_signatures), then calls run_auto_search or re-enriches and asserts that the web-search endpoint was still called (via the RoutingFakeJsonTransport.calls list). A name like test_duplicate_naver_blog_signature_does_not_suppress_web_search makes the intent explicit.

49. mark_evidence_status atomicity is not tested; only record_decision rollback is pinned

분류 testing · 파일 tests/rights_filter/server/test_sqlite_store.py:8015-8044 · 신뢰도 high · 검증 unverified · 유닛 tst-coverage
설명: record_decision has a rollback test at line 8015 that monkeypatches add_audit_event to raise and then asserts that the decision status did not change. mark_evidence_status (sqlite_store.py lines 587-617) performs a similar multi-step write — _put the evidence row, _rescore_submission, then add_audit_event — but it does NOT wrap these in _transaction(). If add_audit_event fails after _put succeeds, the status change is committed without an audit entry. The deferred open item in the remediation plan (mark_evidence_status atomic state+audit transaction) was explicitly scoped out but means the atomicity gap is real and untested.
영향: A mid-sequence error (e.g., SQLite busy during audit_events insert) leaves evidence status changed with no audit record, contradicting the audit-completeness invariant.
권장: Wrap mark_evidence_status in _transaction() the same way record_decision is, then add a rollback test mirroring test_record_decision_rolls_back_when_audit_fails: monkeypatch add_audit_event to raise, call mark_evidence_status, and assert the evidence payload is unchanged and the audit log has no matching entry.

⚪ LOW

50. mark_evidence_status does not wrap status update, rescore, and audit event in one transaction

분류 bug · 파일 src/rights_filter/server/sqlite_store.py:587-617 · 신뢰도 high · 검증 unverified · 유닛 bug-store
설명: mark_evidence_status performs three separate auto-committed writes: _put('evidence', ...) at line 609, _rescore_submission at line 610 (which itself does _get and _put on submissions), and add_audit_event at line 611. A server crash or exception between steps leaves the evidence status updated but the risk score unchanged and no audit trail written. This is the still-pending deferred item from the prior review.
영향: Low-probability crash scenario, but when it occurs the evidence status and risk score are inconsistent and the operator change has no audit trail.
권장: Wrap the three writes in _transaction(): acquire a conn, pass it to _put('evidence'), perform the rescore _get and _put on submissions with the same conn, and call add_audit_event with conn=conn. This mirrors the pattern already used in record_decision at line 569.

분류 bug · 파일 src/rights_filter/analysis/risk_scoring.py:76-102 · 신뢰도 medium · 검증 unverified · 유닛 bug-analysis
설명: There is a semantic asymmetry between _web_detection_points and _web_detection_bucket for items that carry both entity and a specific category. _web_detection_points (lines 87-88) awards 45 points via the category in {'character','celebrity','work'} path — before reaching the generic entity path (line 90, round(40 * confidence)). But _web_detection_bucket (lines 98-99) always returns 'entity' for any item with a non-empty entity, regardless of category. This means a high-category item (45 pts, fast-path) and a plain-entity item (40*conf pts, fallback) both claim the 'entity' bucket and are deduplicated against each other even though they may represent different scoring levels and different semantic signal strengths. If a high-category item is processed second (e.g. list ordering), it gets suppressed by a lower-value plain-entity item that arrived first.
영향: Score under-counting when a category-qualified entity and a plain entity are both present in the evidence list. The higher-value category detection may be discarded if a lower-confidence plain-entity detection arrived earlier in iteration order, causing inconsistent scoring based on evidence ordering rather than evidence content.
권장: In _web_detection_bucket, mirror the priority ordering of _web_detection_points: check if item.data.get('category') in {'character','celebrity','work'} first and return a category-specific bucket string (e.g. f"category:{item.data['category']}"), before falling through to the generic 'entity' bucket. This ensures the dedup key reflects which scoring path was taken, preventing cross-path deduplication.

52. No full transitive dependency lock file with hashes

분류 build · 파일 requirements.txt:9-11 · 신뢰도 high · 검증 unverified · 유닛 bld-airgap
설명: The remediation plan (Task 7, docs/superpowers/plans/2026-06-20-project-review-remediation.md:254) called for a requirements-lock.txt generated by pip freeze, which was never committed (commit 8958dd1 added only requirements.txt with the 3 direct deps and requirements-dev.txt). The current requirements.txt pins exact versions for the 3 direct runtime deps (numpy==2.3.5, opencv-python-headless==4.13.0.92, pillow==12.2.0). Since pip show confirms numpy has no transitive deps, pillow has no transitive deps, and opencv-python-headless's only dep is numpy, the pip download -r requirements.txt command in offline-install.md is deterministic and complete for the current dep set. However, there are no --hash flags, so supply-chain integrity is not verified: a compromised PyPI mirror or a wheel substitution attack would not be detected. Additionally requirements-dev.txt adds pytest, pytest-cov, pytest-timeout, and playwright==1.49.1, none of whose transitive deps are pinned in a lock file.
영향: Without hash verification, a compromised wheel in the offline wheelhouse would not be detected at install time. This is a supply-chain gap rather than a runtime failure.
권장: Generate a full lock with hashes on the build machine: pip download -r requirements.txt -d wheelhouse/ && pip hash wheelhouse/*.whl >> requirements.lock. Or use pip-compile --generate-hashes (pip-tools). At install time, use pip install --no-index --find-links wheelhouse/ --require-hashes -r requirements.lock. For the dev lock, do the same with requirements-dev.txt. This matches the original plan's intent for requirements-lock.txt.

53. offline-install.md omits Playwright Chromium binary transfer procedure

분류 build · 파일 docs/operations/offline-install.md:1-60 · 신뢰도 high · 검증 unverified · 유닛 bld-airgap
설명: requirements-dev.txt:6–9 contains a comment explaining that the Chromium binary used by Playwright E2E tests is not pip-installed and must be fetched via playwright install chromium on the build host, then the PLAYWRIGHT_BROWSERS_PATH cache must be bundled for the air-gapped target. However, offline-install.md has no corresponding step. A developer running the test suite on an air-gapped target will see Playwright-skipped tests without knowing how to provide the browser. The tests do skip cleanly when Chromium is absent, so this is a documentation gap, not a runtime crash.
영향: E2E tests cannot run on an air-gapped target without an undocumented manual step. This only affects development/CI environments, not the production server.
권장: Add a 'Development: Playwright Chromium' section to offline-install.md: (1) On the build machine: PLAYWRIGHT_BROWSERS_PATH=./pw-browsers playwright install chromium. (2) Transfer pw-browsers/ alongside the repo. (3) On the target: export PLAYWRIGHT_BROWSERS_PATH=/path/to/pw-browsers before running pytest. Reference the inline comment already in requirements-dev.txt.

54. Unhandled exception types in request handlers log stack traces to stderr

분류 error-handling · 파일 src/rights_filter/server/http_app.py:99-106 · 신뢰도 high · 검증 unverified · 유닛 sec-http
설명: The do_GET, do_POST, and do_PATCH handlers catch _PayloadTooLarge, sqlite3.OperationalError, KeyError, and ValueError. Any other exception (e.g., OSError reading a file, AttributeError, RuntimeError from the store layer) propagates to ThreadingHTTPServer.handle_error, which prints a full traceback including file paths and variable values to stderr (verified from stdlib source). log_message is suppressed but handle_error is not overridden. Additionally, ValueError messages from deep store layers are forwarded verbatim to the HTTP response body (lines 106, 212, 244), which can expose internal state (e.g., "naver provider not connected", "could not resolve fetch host").
영향: Internal file paths, module names, and system configuration details can leak to server logs (read by anyone with shell access) or to the client via ValueError messages.
권장: Override handle_error in CopyrighterHandler to suppress or sanitize the stderr output in production. For ValueError responses, consider logging the full message server-side and returning a generic client message, or at minimum audit each raise ValueError(...) in store modules to ensure no internal path or system detail is included.

55. _search_result_page_image_candidates and _search_result_stylesheet_image_urls swallow all page-fetch exceptions silently

분류 error-handling · 파일 src/rights_filter/server/store_search_candidates.py:424-427, 452-454 · 신뢰도 high · 검증 unverified · 유닛 err-silent
설명: Both _search_result_page_image_candidates (line 424–427) and _search_result_stylesheet_image_urls (line 452–454) use bare except Exception: return [] / except Exception: continue to suppress page and stylesheet fetch errors. These cover the entire range from ValueError (SSRF guard) through urllib.error.HTTPError (4xx/5xx), TimeoutError, and unexpected parser bugs. There is no logging at any level.
영향: SSRF guard rejections from attacker-influenced search result URLs are suppressed at the same level as transient network timeouts, making security-relevant events invisible in logs. A misconfigured page_fetcher (e.g. wrong timeout) would silently reduce evidence coverage with no operator visibility.
권장: Separate SSRF ValueError (log at WARNING with URL) from network/timeout errors (log at DEBUG). Add the URL to all log messages so operators can correlate with provider results.

56. LLM assistant is unconditionally constructed even when OLLAMA_BASE_URL is not set

분류 error-handling · 파일 src/rights_filter/integrations/env_clients.py:239-245 · 신뢰도 high · 검증 unverified · 유닛 err-silent
설명: _llm_assistant (line 239) always constructs and returns an InternalLlmAssistant wrapping an OllamaGenerateLlmClient, regardless of whether OLLAMA_BASE_URL or OLLAMA_MODEL are set in the environment. This contrasts with _naver_adapter, _google_adapter, and _google_custom_search_adapter, which all return None when their required credentials are absent. As a result, the llm provider is always enabled=True in provider_payloads (line 175: llm_assistant is not None) and _ensure_llm_summary will attempt to call Ollama on every enrichment cycle. The OllamaGenerateLlmClient's connect-probe will then fire on each call when Ollama is not running, incurring a connect_timeout-length delay per submission during the seed loop.
영향: When the operator has not deployed Ollama, every seed/reload/rerun operation adds a per-submission connect_timeout (default 2s) delay. With many submissions this can make the reload endpoint appear to hang. The provider readiness dashboard also misleadingly shows the LLM as configured and enabled.
권장: Return None from _llm_assistant when neither OLLAMA_BASE_URL nor OLLAMA_MODEL is set, matching the pattern of the other adapters. Only construct the assistant when at least OLLAMA_BASE_URL is present.

57. formatReason() uses a cascade of string replacement fallbacks that will mistranslate unexpected field values

분류 maintainability · 파일 web/operator-gui/app.js:570-614 · 신뢰도 high · 검증 unverified · 유닛 mnt-hotspots
설명: The final fallback block (lines 598–613) applies chained .replaceAll() calls on the raw string. Strings like 'internal confidence page' will be progressively transformed to '내부 신뢰도 페이지', which may or may not be the intended meaning. Any new server-side evidence reason string that partially matches one of the 12 token patterns will be silently garbled. The function has already grown to 44 lines (exceeding the 50-line limit is close) and each new reason string requires a new branch.
영향: Low current risk since the known reason strings are well-tested, but any new evidence reason added on the backend that is not also added here will be silently garbled rather than passed through unchanged.
권장: Move the authoritative label mapping to the Python layer (already done for most display labels). Have the server serialise a displayLabel field alongside reason in the evidence payload. The JS should use displayLabel if present and fall back to reason raw only for unknown strings, eliminating the translation cascade entirely.

58. UrllibJsonTransport follows HTTP redirects without SSRF re-validation

분류 security · 파일 src/rights_filter/integrations/http_json.py:34-39 · 신뢰도 medium · 검증 unverified · 유닛 sec-ssrf
설명: urlopen(request, timeout=timeout) uses Python's default opener, which includes HTTPRedirectHandler. This handler follows 301/302/307 redirects automatically but performs no SSRF guard on the redirect target. By contrast, store_remote_fetch.py uses a custom _ValidatingRedirectHandler that calls _assert_public_http_url(newurl) on every hop. For the current fixed API endpoints (googleapis.com, openapi.naver.com) this risk is theoretical, but the OLLAMA_BASE_URL is operator-configurable. If an operator points OLLAMA_BASE_URL at a server that issues a redirect to an internal metadata endpoint (e.g., 169.254.169.254), the transport would follow it without any guard. A similar risk applies if a future integration introduces an operator-configurable endpoint.
영향: Low in the current deployment (OLLAMA_BASE_URL is set by the operator at startup from .env, not from HTTP requests), but becomes medium if a future feature exposes the endpoint as a user-configurable API parameter.
권장: Build UrllibJsonTransport with the same _ValidatingRedirectHandler already used in store_remote_fetch.py, or at minimum apply it when connect_timeout is set (i.e., when the caller has opted into connectivity checks). This makes the defense consistent across all outbound HTTP in the codebase.

59. faceCrops href and img src use escapeHtml but not safeUrl — missing defense-in-depth against javascript: URLs

분류 security · 파일 web/operator-gui/app.js:1359-1360 · 신뢰도 high · 검증 unverified · 유닛 sec-ssrf
설명: The face-crop strip renders crop.url into both href and img src attributes using only escapeHtml, not safeUrl. Every other external URL in the same file goes through escapeHtml(safeUrl(url)) (see lines 1766-1768 for the evidence preview and line 1788 for the evidence link). In practice crop.url is always server-generated as /face-crop-media/{submission_id}/crop-{index}.jpg (store_enrichment.py line 178) and contains no attacker-controlled components, so the immediate risk is zero. However, the inconsistency means that if the construction of crop.url ever changes (e.g., a future feature that includes an external thumbnail URL), the missing safeUrl call would allow a javascript: or data: URL to reach the DOM without being blocked.
영향: No current exploitable path; the URL is server-generated and not attacker-influenced. The impact is a future-regression risk if the URL source changes.
권장: Change both occurrences at lines 1359 and 1360 to use escapeHtml(safeUrl(crop.url)) to match the established pattern used everywhere else in the file. This is a one-line change per occurrence.

60. Latent f-string SQL injection surface in _put() fallback branch

분류 security · 파일 src/rights_filter/server/store_persistence.py:212-215 · 신뢰도 medium · 검증 unverified · 유닛 sec-sql
설명: The _put() method has an explicit if-elif chain for each known table ('submissions', 'evidence', 'providers', 'knowledge_entries', 'collection_candidates', 'corrections'). Any table that passes _validate_table() but falls through the chain hits the generic fallback at lines 212-215: conn.execute(f'insert or replace into {table} (id, payload) values (?, ?)', ...). The same pattern appears in _get() (line 226) and _all() (line 256). Currently, the only tables in STORE_TABLES not covered by the if-chain are 'audit_events' and 'submission_queues', and no caller passes these to _put(). However, _validate_table() is the only gate between a table name and an f-string SQL execution. If a future developer adds a new table to STORE_TABLES without adding a corresponding if branch in _put(), the fallback silently executes. More concretely, _get() and _all() always use the f-string path (no if-chain fallback) with only _validate_table() as the guard — those two functions cover every table. The mitigating factor is that table names never originate from user-controlled HTTP input; they are always call-site literals in the codebase.
영향: No current exploitation path exists because table names are call-site literals, not HTTP parameters. The risk is architectural: the pattern relies on STORE_TABLES being a complete allowlist that is kept in sync. A future refactor that adds a table from an untrusted source could inadvertently enable SQL injection. The _put() fallback also silently skips the typed-column extraction for new tables, which is a correctness bug waiting to happen.
권장: Remove the f-string fallback branch from _put() and replace it with an explicit raise ValueError(f'_put() has no handler for table {table!r}') to make the gap detectable at development time. For _get() and _all(), the _validate_table() call is sufficient since those are read-only queries with parameterized WHERE clauses, but add a comment confirming the whitelist is the only guard. Consider adding a unit test that calls _validate_table() for every entry in STORE_TABLES and asserts a corresponding branch exists in _put().

61. _drop_stale_rebuild_temp_tables drops table names read from sqlite_master via f-string without secondary validation

분류 security · 파일 src/rights_filter/server/store_schema.py:181-186 · 신뢰도 low · 검증 unverified · 유닛 sec-sql
설명: The function queries sqlite_master for table names matching the LIKE pattern '_copyrighter%_new' and then executes 'drop table if exists {row["name"]}' via an f-string (line 186). There is no secondary validation of the returned name against an expected format beyond what the SQL LIKE filter already enforces. In practice this is an internal SQLite file manipulated only by the application, and an attacker would need filesystem write access to plant a malicious table name — at which point they already have full DB access. However the LIKE pattern uses underscores which in SQL LIKE match any single character, not just literal underscores; the ESCAPE clause at the end of the query string properly escapes the underscores ('_ _ copyrighter _% _new' escape ''), so the pattern is correct as written.
영향: No realistic exploitation path in the current deployment model (local SQLite, operator-only access). If the DB file were exposed to a less-trusted party who could create tables with crafted names, they could influence which tables are dropped at startup. The actual risk is near-zero given the access model.
권장: Add an assertion or re-validate that the table name from sqlite_master matches the Python-level regex r'^copyrighter[a-z]+_new$' before executing the DROP, converting the implicit LIKE-filter trust into an explicit Python guard. This is a defense-in-depth measure: import re; if not re.fullmatch(r'copyrighter[a-z]+_new', row['name']): continue.

분류 security · 파일 web/operator-gui/app.js:116-124 · 신뢰도 medium · 검증 unverified · 유닛 sec-xss
설명: All API calls go through apiJson(), which calls fetch(path, options) without specifying credentials. The Fetch API default is credentials: 'same-origin', meaning cookies are sent for same-origin requests and not for cross-origin ones. This works correctly today because the GUI and API are co-hosted on the same port. However, the default is implicit and undocumented: a future refactor that moves the API to a separate port or domain would silently stop sending the copyrighter_session cookie, causing 401 errors that are hard to diagnose. Because the auth cookie is SameSite=Strict, CSRF via cookie is not a threat, but the absence of an explicit mode is a maintenance hazard.
영향: Silent auth breakage if the API and GUI origins ever diverge; not a current security hole.
권장: Add credentials: 'same-origin' explicitly to the fetch options in apiJson(): const response = await fetch(path, { credentials: 'same-origin', headers: { ... }, ...options });. This documents intent and prevents future confusion.

63. window.prompt() used for deactivate/reactivate memo collection — blocks event loop and has no XSS guard on the result

분류 security · 파일 web/operator-gui/app.js:3347, 3363 · 신뢰도 high · 검증 unverified · 유닛 sec-xss
설명: deactivateKnowledgeEntry() (line 3347) and reactivateKnowledgeEntry() (line 3363) collect operator memos via window.prompt(). The prompt return value is passed directly to JSON.stringify({ reason: reason.trim() }) and sent to the API — this is safe for the JSON body itself. However: (1) window.prompt() is a synchronous modal that blocks the browser event loop and cannot be styled or localised, making it inconsistent with the rest of the operator-review workflow; (2) the result is not length-capped or sanitised client-side before being included in the API body; (3) the function is not guarded against concurrent invocations (a rapid double-click opens two prompts).
영향: UX inconsistency and minor quality concern; the API call itself has server-side validation. Not a direct XSS vector because the memo value is JSON-encoded before transmission and the audit/knowledge views already escape values with escapeHtml() when rendering.
권장: Replace window.prompt() with the existing textarea / memo-field pattern (as used for the decision-memo field in the decision pane). Add a debounce/disabled-flag on the deactivate/reactivate buttons to prevent concurrent submissions, consistent with other action buttons in the codebase.

64. _fetch_page_url_bytes and _fetch_stylesheet_url_bytes SSRF guard not independently tested

분류 security · 파일 tests/rights_filter/server/test_sqlite_store.py:8047-8062 · 신뢰도 medium · 검증 unverified · 유닛 tst-coverage
설명: test_fetch_rejects_internal_or_non_http_hosts (line 8059) calls sqlite_store_module._fetch_url_bytes(url), which tests only the image-fetch path. _fetch_page_url_bytes and _fetch_stylesheet_url_bytes in store_remote_fetch.py (lines 58-63) also call _fetch_url_bytes_with_headers, which calls _assert_public_http_url. However, since they go through the same _fetch_url_bytes_with_headers helper, the guard fires for all three. The _ValidatingRedirectHandler (lines 92-97) — which re-validates the SSRF guard on every redirect hop — has no test at all; the redirect-to-internal scenario that H8 targeted is not exercised.
영향: The redirect-SSRF regression path (a public URL that 302s to an internal address) is not covered. A future refactor that replaces _SSRF_SAFE_OPENER with a plain urlopen would not be caught.
권장: Add a test that monkeypatches _open_url to call _assert_public_http_url(newurl) for a redirect URL pointing to 169.254.169.254 and asserts ValueError is raised. This is the canonical regression test for the H8 fix.

65. import-folder with empty/missing path body is not validated by the test suite

분류 testing · 파일 tests/rights_filter/server/test_http_app.py:1400-1424 · 신뢰도 high · 검증 unverified · 유닛 tst-coverage
설명: http_app.py line 149 raises ValueError('submission folder path is required') when body.get('folder', '') or body.get('path', '') is falsy. test_upload_and_import_validation_returns_400 (line 1400) covers an unsupported file type, bad base64, empty image data, and a non-existent folder path — but it never POSTs an empty body {} or {'path': ''} to /api/submissions/import-folder. The path raise ValueError('submission folder path is required') is therefore uncovered.
영향: Low: the guard will still execute at runtime, but a refactor that accidentally breaks the empty-path check (e.g., changes the key name) would not be detected.
권장: Add post_expect('/api/submissions/import-folder', {}, 400) and post_expect('/api/submissions/import-folder', {'path': ''}, 400) to the existing test_upload_and_import_validation_returns_400 test.

66. E2E decision flow covers only 4 scenarios; no test for hold/reject after prior approve

분류 testing · 파일 tests/operator_gui/test_decision_flow.py:28-72 · 신뢰도 medium · 검증 unverified · 유닛 tst-coverage
설명: The Playwright decision-flow spec (4 tests) covers: reject-without-memo blocked, approve persists, hold persists, and reject-with-memo persists. It does not cover: changing a decision after a prior one has been saved (e.g., approve then hold — verifying the recommendation-box label updates on re-render from the API), nor the corrected decision status. The test_approve_persists_without_memo and test_hold_persists_without_memo also do not assert that the console has no errors AND that refreshFromApi was called (only console_errors is checked, not the API round-trip correctness).
영향: Nice-to-have coverage; the decision-persistence path is exercised in the server-side tests. The gap is primarily about regression-catching at the browser level.
권장: Add one E2E test that approves a submission, then changes it to held, and verifies the recommendation-box shows 보류 after the second decision — exercising the update path rather than just the initial-set path.

121 KiB Raw Blame History Unescape Escape