From 36eb13e958f82b3100ec4945df4d8f0fc375031a Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 12 Aug 2026 17:20:22 +0800 Subject: [PATCH 1/3] chore(deps): update github-actions (main) (minor) (#489) chore(deps): update github-actions Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- .github/workflows/codeql-lint.yml | 6 +++--- .github/workflows/dep-audit.yml | 2 +- .github/workflows/release-please.yml | 2 +- .github/workflows/repo-events.yml | 2 +- .github/workflows/stale.yml | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/codeql-lint.yml b/.github/workflows/codeql-lint.yml index 2ee7ee46..2e0744d8 100644 --- a/.github/workflows/codeql-lint.yml +++ b/.github/workflows/codeql-lint.yml @@ -170,7 +170,7 @@ jobs: # ---- Frontend Lint (reusable) ---- frontend-lint: name: Frontend Lint - uses: chodeus/chodeus-ops/.github/workflows/node-ci.yml@aae195c19ec7069d2e91aca88107220ce807ef7e # main + uses: chodeus/chodeus-ops/.github/workflows/node-ci.yml@579feb04e3219248963bd27843298a8c2ea6b2fd # main with: node_version: '24' package_json_dir: frontend @@ -197,7 +197,7 @@ jobs: persist-credentials: false - name: Setup Node.js - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + uses: actions/setup-node@249970729cb0ef3589644e2896645e5dc5ba9c38 # v6.5.0 with: node-version: '24' cache: 'npm' @@ -433,7 +433,7 @@ jobs: notify-failure: needs: [codeql-python, codeql-javascript, backend-lint, backend-smoke, frontend-lint, frontend-tests, branch-isolation-guard, develop-invariant-guard, docker-push] if: failure() && (github.event_name == 'push' || github.event_name == 'workflow_dispatch') - uses: chodeus/chodeus-ops/.github/workflows/notify-discord.yml@aae195c19ec7069d2e91aca88107220ce807ef7e # main + uses: chodeus/chodeus-ops/.github/workflows/notify-discord.yml@579feb04e3219248963bd27843298a8c2ea6b2fd # main with: event_type: build status: failure diff --git a/.github/workflows/dep-audit.yml b/.github/workflows/dep-audit.yml index 26a6fa11..a4d2e4d3 100644 --- a/.github/workflows/dep-audit.yml +++ b/.github/workflows/dep-audit.yml @@ -21,7 +21,7 @@ permissions: {} jobs: osv-scan: - uses: google/osv-scanner-action/.github/workflows/osv-scanner-reusable.yml@9a498708959aeaef5ef730655706c5a1df1edbc2 # v2.3.8 + uses: google/osv-scanner-action/.github/workflows/osv-scanner-reusable.yml@8deb546fdb875b9996d27d4950be7312dac076a1 # v2.5.0 permissions: # Least-privilege for the reusable workflow: SARIF upload + checkout. actions: read diff --git a/.github/workflows/release-please.yml b/.github/workflows/release-please.yml index 6f7fddb0..5a58fe49 100644 --- a/.github/workflows/release-please.yml +++ b/.github/workflows/release-please.yml @@ -12,7 +12,7 @@ permissions: jobs: release: - uses: chodeus/chodeus-ops/.github/workflows/release-please.yml@aae195c19ec7069d2e91aca88107220ce807ef7e # main + uses: chodeus/chodeus-ops/.github/workflows/release-please.yml@579feb04e3219248963bd27843298a8c2ea6b2fd # main docker-version: needs: release diff --git a/.github/workflows/repo-events.yml b/.github/workflows/repo-events.yml index 162f9aa4..07d4448c 100644 --- a/.github/workflows/repo-events.yml +++ b/.github/workflows/repo-events.yml @@ -19,5 +19,5 @@ permissions: jobs: notify: - uses: chodeus/chodeus-ops/.github/workflows/repo-events.yml@aae195c19ec7069d2e91aca88107220ce807ef7e # main + uses: chodeus/chodeus-ops/.github/workflows/repo-events.yml@579feb04e3219248963bd27843298a8c2ea6b2fd # main secrets: inherit diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml index 5c029380..8756b436 100644 --- a/.github/workflows/stale.yml +++ b/.github/workflows/stale.yml @@ -11,4 +11,4 @@ permissions: jobs: stale: - uses: chodeus/chodeus-ops/.github/workflows/stale-issues.yml@aae195c19ec7069d2e91aca88107220ce807ef7e # main + uses: chodeus/chodeus-ops/.github/workflows/stale-issues.yml@579feb04e3219248963bd27843298a8c2ea6b2fd # main From 38d5fe8fb9ab31a0f3366d4053d7c0745bbfc54d Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 12 Aug 2026 17:33:53 +0800 Subject: [PATCH 2/3] chore(deps): update actions/setup-node action to v7 (main) (#493) chore(deps): update actions/setup-node action to v7 Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- .github/workflows/codeql-lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/codeql-lint.yml b/.github/workflows/codeql-lint.yml index 2e0744d8..992893ae 100644 --- a/.github/workflows/codeql-lint.yml +++ b/.github/workflows/codeql-lint.yml @@ -197,7 +197,7 @@ jobs: persist-credentials: false - name: Setup Node.js - uses: actions/setup-node@249970729cb0ef3589644e2896645e5dc5ba9c38 # v6.5.0 + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: node-version: '24' cache: 'npm' From 0244bfab12680fe19376095e167eaa4feff9a82e Mon Sep 17 00:00:00 2001 From: chodeus Date: Wed, 12 Aug 2026 22:45:57 +0800 Subject: [PATCH 3/3] fix(api): keep exception text out of response bodies; break the logger import cycle (#511) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(api): keep exception text out of response bodies; break the logger cycle - sweep all 171 sites where str(e)/f-string exception text reached a JSON response (17 api modules, the FastAPI catch-all included): responses carry stable public messages, the detail stays in logger.error (14 log lines added, 2 upgraded to exc_info) - ConfigError carries a curated .message (position-only YAML errors via problem_mark; no parser or OS text) and the handler reads the attribute, never the exception object - error/status codes byte-identical across the sweep (312 values verified) - move create_bar into logger.py (its dominant consumer) with a helper re-export: kills the logger<->helper import cycle and the lazy-import workaround that papered over it Closes code-scanning alerts 294 (stack-trace exposure) and 273 (cyclic import). * test: raise the leak-marker error from a real accessor, not __getattr__ Special methods should honor their contract (CodeQL 296); an explicit get_poster_stats stub also can't be swallowed by attribute-probing paths. * test: route the leak marker through the real stats accessor The previous stub method was never called (the route reads db.stats), making the marker assertion vacuous; bite re-proven against a deliberately leaky handler. * fix(api): review round — close the ConfigError class, confine deletes, sanitize passthroughs - ConfigError now re-raises to the shared CONFIG_INVALID contract from every api handler (50 sites across two rounds; a transitive checker proves zero swallowing trys remain, with two documented exceptions). A malformed config can no longer 200 as empty roots, a false "no secret configured" wiring panel, or silently-missing styles - worker failure results pass through worker_error(): stable public message, detail logged (the worker's own strings can embed exception text) - upstream Plex bodies stay out of responses (_PlexFetchError grows a server-only detail); validation 422s drop echoed input values and mask sensitive fields via the existing SENSITIVE_FIELD_NAMES - GDrive folder deletion re-asserts the resolved target immediately before rmtree; poster file deletion realpaths and requires configured-root membership before os.remove (poisoned rows can no longer point deletes outside the roots), and file_deleted reports the real outcome * test: record-and-raise AttributeError in the webhook tripwire Special methods keep their contract (CodeQL 298) and the touch list survives even if a handler swallows the exception; bite re-proven via exit codes with the guard genuinely removed. * fix(api): close the sentinel-assignment ConfigError shape delete_poster loads config before the destructive row delete (a malformed config can no longer 200 after an irreversible deletion); the directory guards' inner config=None fallbacks re-raise instead of impersonating PATH_NOT_ALLOWED; webhook trusted-proxy resolution fails closed either way. * fix(api): webhook secret dependency uses the shared ConfigError contract The deliberate 503 predates the campaign's uniform CONFIG_INVALID handling and its retryability rationale doesn't hold (failed webhook events aren't re-queued on any 5xx). ConfigError now propagates from the dependency; tests assert the shared 500 CONFIG_INVALID with zero enqueues. --- backend/api/auth.py | 2 +- backend/api/border_replacerr.py | 3 +- backend/api/config.py | 12 +- backend/api/instances.py | 58 +++++-- backend/api/jobs.py | 31 ++-- backend/api/labelarr.py | 27 +-- backend/api/logs.py | 6 +- backend/api/main.py | 23 ++- backend/api/media_api.py | 59 ++++--- backend/api/modules.py | 48 ++++-- backend/api/nestarr.py | 14 +- backend/api/notifications.py | 27 ++- backend/api/posters.py | 202 +++++++++++++++-------- backend/api/schedule.py | 30 +++- backend/api/setup.py | 3 +- backend/api/system.py | 82 +++++----- backend/api/utils.py | 11 ++ backend/api/webhooks.py | 71 ++++---- backend/util/config.py | 75 ++++++--- backend/util/helper.py | 25 +-- backend/util/logger.py | 19 ++- tests/test_api_config_errors.py | 31 ++++ tests/test_api_smoke.py | 236 ++++++++++++++++++++++++++- tests/test_regression_review_2026.py | 7 +- 24 files changed, 774 insertions(+), 328 deletions(-) diff --git a/backend/api/auth.py b/backend/api/auth.py index a52b85e3..367987a8 100644 --- a/backend/api/auth.py +++ b/backend/api/auth.py @@ -174,7 +174,7 @@ async def setup_auth( except ConfigError as e: logger.error(f"Auth setup failed: {e}") return error( - f"Failed to save auth configuration: {e}", + "Failed to save auth configuration", code="AUTH_SETUP_ERROR", status_code=500, ) diff --git a/backend/api/border_replacerr.py b/backend/api/border_replacerr.py index 99dc3548..59e40127 100644 --- a/backend/api/border_replacerr.py +++ b/backend/api/border_replacerr.py @@ -356,8 +356,9 @@ def generate_preview( except (ValueError, KeyError, AttributeError) as e: # A malformed holiday/border config must yield a clean 4xx, not an # unhandled 500 on the preview endpoint. + logger.error(f"Invalid border/holiday configuration: {e}") return error( - f"Invalid border/holiday configuration: {e}", + "Invalid border/holiday configuration", code="BORDER_CONFIG_INVALID", status_code=400, ) diff --git a/backend/api/config.py b/backend/api/config.py index 71304724..9621445f 100755 --- a/backend/api/config.py +++ b/backend/api/config.py @@ -121,14 +121,14 @@ async def get_config( except ConfigError as e: logger.error(f"Configuration error: {e}") return error( - f"Configuration error: {str(e)}", + "Configuration error", "CONFIG_ERROR", status_code=500, ) except Exception as e: logger.error(f"Error retrieving configuration: {e}") return error( - f"Error retrieving configuration: {str(e)}", + "Error retrieving configuration", "CONFIG_RETRIEVAL_ERROR", status_code=500, ) @@ -232,21 +232,21 @@ async def update_config( except ConfigError as e: logger.error(f"Configuration error: {e}") return error( - f"Configuration error: {str(e)}", + "Configuration error", code="CONFIG_ERROR", status_code=500, ) except ValueError as e: logger.error(f"Configuration validation failed: {e}") return error( - f"Configuration validation failed: {str(e)}", + "Configuration validation failed", "CONFIG_VALIDATION_ERROR", status_code=400, ) except Exception as e: logger.error(f"Configuration update failed: {e}") return error( - f"Configuration update failed: {str(e)}", + "Configuration update failed", code="CONFIG_UPDATE_ERROR", status_code=500, ) @@ -294,7 +294,7 @@ async def reveal_secret( except ConfigError as e: logger.error(f"Configuration error: {e}") return error( - f"Configuration error: {str(e)}", + "Configuration error", "CONFIG_ERROR", status_code=500, ) diff --git a/backend/api/instances.py b/backend/api/instances.py index 69908dda..3f353c03 100755 --- a/backend/api/instances.py +++ b/backend/api/instances.py @@ -19,6 +19,7 @@ from backend.util.config import ( REDACTED_PLACEHOLDER, ChubConfig, + ConfigError, InstanceDetail, load_config, redact_secrets, @@ -507,10 +508,10 @@ def _probe(probe): "response_time_ms": 0, } except Exception as exc: + logger.error(f"Health probe failed for '{name}': {exc}") return name, { "service": service, "status": "error", - "error": str(exc), } if probes: @@ -593,7 +594,7 @@ async def get_instances( except Exception as e: logger.error(f"Error retrieving instances: {e}") return error( - f"Error retrieving instances: {str(e)}", + "Error retrieving instances", code="INSTANCES_RETRIEVAL_ERROR", status_code=500, ) @@ -602,9 +603,13 @@ async def get_instances( class _PlexFetchError(Exception): """Carries the API error code/status the libraries endpoints surface.""" - def __init__(self, message: str, code: str, status_code: int) -> None: + def __init__( + self, message: str, code: str, status_code: int, detail: str = "" + ) -> None: + """Split the public message from the server-only `detail`.""" super().__init__(message) - self.message = message + self.message = message # public: returned to the client + self.detail = detail # server-only: never put in a response self.code = code self.status_code = status_code @@ -637,13 +642,16 @@ def _fetch_plex_libraries(plex_data: Any) -> list: res = requests.get(url, headers=headers, timeout=5, allow_redirects=False) except requests.exceptions.RequestException as req_exc: raise _PlexFetchError( - f"Failed to connect to Plex server: {req_exc}", + "Failed to connect to Plex server", "PLEX_CONNECTION_FAILED", 502, ) from req_exc if not res.ok: raise _PlexFetchError( - f"Plex server error: {res.text}", "PLEX_SERVER_ERROR", res.status_code + "Plex server error", + "PLEX_SERVER_ERROR", + res.status_code, + detail=res.text[:500], ) import defusedxml.ElementTree as ET @@ -786,7 +794,13 @@ def get_plex_libraries( try: libraries = _fetch_plex_libraries(plex_data) except _PlexFetchError as fe: - logger.error("Plex libraries fetch failed: %s", fe.message) + # exc_info carries the __cause__ the public message no longer does + logger.error( + "Plex libraries fetch failed: %s %s", + fe.message, + fe.detail, + exc_info=True, + ) return error(fe.message, code=fe.code, status_code=fe.status_code) libraries = _annotate_enabled(libraries, plex_data.enabled_libraries) @@ -798,7 +812,7 @@ def get_plex_libraries( except Exception as e: logger.error(f"Unexpected error retrieving Plex libraries: {e}") return error( - f"Error retrieving Plex libraries: {str(e)}", + "Error retrieving Plex libraries", code="PLEX_LIBRARIES_ERROR", status_code=500, ) @@ -1041,10 +1055,12 @@ def test_instance( code="CONNECTION_FAILED", status_code=502, ) + except ConfigError: + raise except Exception as e: logger.error(f"Connection test failed for {data.name} ({data.url}): {e}") return error( - f"Connection test error: {str(e)}", + "Connection test error", code="CONNECTION_TEST_ERROR", status_code=500, ) @@ -1148,10 +1164,12 @@ async def create_instance( {"service": service, "name": name}, ) + except ConfigError: + raise except Exception as e: logger.error(f"Failed to create instance {data.name}: {e}") return error( - f"Failed to create instance: {str(e)}", + "Failed to create instance", code="INSTANCE_CREATE_ERROR", status_code=500, ) @@ -1311,10 +1329,12 @@ async def update_instance( {"service": service, "name": new_name}, ) + except ConfigError: + raise except Exception as e: logger.error(f"Failed to update instance {instance_id}: {e}") return error( - f"Failed to update instance: {str(e)}", + "Failed to update instance", code="INSTANCE_UPDATE_ERROR", status_code=500, ) @@ -1398,10 +1418,12 @@ async def delete_instance( {"name": instance_id}, ) + except ConfigError: + raise except Exception as e: logger.error(f"Failed to delete instance {instance_id}: {e}") return error( - f"Failed to delete instance: {str(e)}", + "Failed to delete instance", code="INSTANCE_DELETE_ERROR", status_code=500, ) @@ -1755,6 +1777,8 @@ async def refresh_instance( job_id = result.get("data", {}).get("job_id") return ok(f"Refresh initiated for '{instance_id}'", {"job_id": job_id}) return error("Error enqueuing refresh", code="REFRESH_ERROR", status_code=500) + except ConfigError: + raise except Exception as e: logger.error(f"Error refreshing instance {instance_id}: {e}") return error( @@ -1814,6 +1838,8 @@ async def sync_instance( job_id = result.get("data", {}).get("job_id") return ok(f"Sync initiated for '{instance_id}'", {"job_id": job_id}) return error("Error enqueuing sync", code="SYNC_ERROR", status_code=500) + except ConfigError: + raise except Exception as e: logger.error(f"Error syncing instance {instance_id}: {e}") return error( @@ -1909,10 +1935,12 @@ async def toggle_instance( }, ) + except ConfigError: + raise except Exception as e: logger.error(f"Failed to toggle instance {instance_id}: {e}") return error( - f"Failed to toggle instance: {str(e)}", + "Failed to toggle instance", code="INSTANCE_TOGGLE_ERROR", status_code=500, ) @@ -2008,7 +2036,7 @@ def get_instance_logs( except Exception as e: logger.error(f"Error retrieving logs for instance {instance_id}: {e}") return error( - f"Error retrieving instance logs: {str(e)}", + "Error retrieving instance logs", code="INSTANCE_LOGS_ERROR", status_code=500, ) @@ -2117,11 +2145,11 @@ def check_instance_health( "response_time_ms": 0, } except Exception as exc: + logger.error(f"Health check failed for '{instance_id}': {exc}") health_data = { "name": instance_id, "service": service, "status": "error", - "error": str(exc), } return ok(f"Health check for '{instance_id}'", health_data) diff --git a/backend/api/jobs.py b/backend/api/jobs.py index 5a3dc3b6..853f0ac2 100755 --- a/backend/api/jobs.py +++ b/backend/api/jobs.py @@ -3,7 +3,7 @@ from fastapi import APIRouter, Depends -from backend.api.utils import error, get_database, get_logger, ok +from backend.api.utils import error, get_database, get_logger, ok, worker_error from backend.util.database import ChubDB @@ -64,14 +64,17 @@ async def get_job_stats( result = db.worker.job_stats("jobs", error_limit=10) if isinstance(result, dict) and "success" in result: - return result + failed = worker_error( + result, logger, "Error retrieving job statistics", "JOB_STATS_ERROR" + ) + return failed or result else: return ok("Job statistics retrieved", result if result else {"stats": {}}) except Exception as e: logger.error(f"Error fetching job stats: {e}") return error( - f"Error retrieving job statistics: {str(e)}", + "Error retrieving job statistics", "JOB_STATS_ERROR", status_code=500, ) @@ -109,6 +112,11 @@ async def list_jobs( ) if isinstance(result, dict) and "success" in result: + failed = worker_error( + result, logger, "Error listing jobs", "JOBS_LIST_ERROR" + ) + if failed: + return failed jobs = result.get("data", {}).get("jobs", []) _enrich_jobs(jobs) return result @@ -120,7 +128,7 @@ async def list_jobs( except Exception as e: logger.error(f"Error listing jobs: {e}") return error( - f"Error listing jobs: {str(e)}", code="JOBS_LIST_ERROR", status_code=500 + "Error listing jobs", code="JOBS_LIST_ERROR", status_code=500 ) @@ -193,7 +201,7 @@ async def list_webhook_origins( except Exception as e: logger.error(f"Error summarizing webhook origins: {e}") return error( - f"Error summarizing webhook origins: {str(e)}", + "Error summarizing webhook origins", code="WEBHOOK_ORIGIN_ERROR", status_code=500, ) @@ -210,12 +218,15 @@ async def delete_old_jobs( try: result = db.worker.cleanup_jobs("jobs", days=days) if isinstance(result, dict) and "success" in result: - return result + failed = worker_error( + result, logger, "Error cleaning up jobs", "JOBS_CLEANUP_ERROR" + ) + return failed or result return ok(f"Cleaned up jobs older than {days}d", result or {}) except Exception as e: logger.error(f"Error cleaning up jobs: {e}") return error( - f"Error cleaning up jobs: {str(e)}", + "Error cleaning up jobs", code="JOBS_CLEANUP_ERROR", status_code=500, ) @@ -238,7 +249,7 @@ async def get_job_detail( except Exception as e: logger.error(f"Error fetching job {job_id}: {e}") return error( - f"Error retrieving job details: {str(e)}", + "Error retrieving job details", "JOB_RETRIEVAL_ERROR", status_code=500, ) @@ -329,7 +340,7 @@ async def get_job_log_tail( ) except Exception as e: logger.error(f"Error tailing log for job {job_id}: {e}") - return error(f"Error tailing log: {str(e)}", "LOG_TAIL_ERROR", status_code=500) + return error("Error tailing log", "LOG_TAIL_ERROR", status_code=500) @router.post("/jobs/{job_id}/retry") @@ -358,5 +369,5 @@ async def retry_job( except Exception as e: logger.error(f"Error retrying job {job_id}: {e}") return error( - f"Error retrying job: {str(e)}", "JOB_RETRY_ERROR", status_code=500 + "Error retrying job", "JOB_RETRY_ERROR", status_code=500 ) diff --git a/backend/api/labelarr.py b/backend/api/labelarr.py index d3a11e09..0c236aa6 100755 --- a/backend/api/labelarr.py +++ b/backend/api/labelarr.py @@ -11,7 +11,7 @@ from fastapi.responses import JSONResponse from pydantic import BaseModel -from backend.api.utils import error, get_database, get_logger, ok +from backend.api.utils import error, get_database, get_logger, ok, worker_error from backend.util.config import ChubConfig, load_config router = APIRouter( @@ -189,16 +189,17 @@ async def sync_tags_to_plex( }, ) else: - return error( - f"Failed to create labelarr sync job: {result['message']}", - code="JOB_CREATION_FAILED", - status_code=500, + return worker_error( + result, + logger, + "Failed to create labelarr sync job", + "JOB_CREATION_FAILED", ) except Exception as e: logger.error(f"Error creating labelarr sync job: {e}", exc_info=True) return error( - f"Failed to create sync job: {str(e)}", + "Failed to create sync job", code="SYNC_JOB_ERROR", status_code=500, ) @@ -228,6 +229,7 @@ async def bulk_sync_tags( config: ChubConfig = Depends(get_config), logger: Any = Depends(get_logger), ) -> JSONResponse: + """Enqueue one labelarr sync job covering many media_cache ids (max 1000).""" try: if not request_data.media_cache_ids: return error( @@ -289,12 +291,11 @@ async def bulk_sync_tags( table_name="jobs", payload=payload, job_type="labelarr_bulk_sync" ) - if not result.get("success"): - return error( - result.get("message", "Failed to enqueue bulk labelarr job"), - code="JOB_ENQUEUE_FAILED", - status_code=500, - ) + failed = worker_error( + result, logger, "Failed to enqueue bulk labelarr job", "JOB_ENQUEUE_FAILED" + ) + if failed: + return failed job_id = result["data"]["job_id"] logger.info( @@ -313,7 +314,7 @@ async def bulk_sync_tags( except Exception as e: logger.error(f"Error in bulk labelarr sync: {e}", exc_info=True) return error( - f"Bulk sync failed: {str(e)}", + "Bulk sync failed", code="BULK_SYNC_ERROR", status_code=500, ) diff --git a/backend/api/logs.py b/backend/api/logs.py index 6d241c22..6dc18541 100755 --- a/backend/api/logs.py +++ b/backend/api/logs.py @@ -69,7 +69,7 @@ async def list_logs(logger: Any = Depends(get_logger)) -> Dict[str, Any]: except Exception as e: logger.error(f"Error listing log modules: {e}") return error( - f"Error listing log modules: {str(e)}", + "Error listing log modules", code="LOG_MODULES_LIST_ERROR", status_code=500, ) @@ -120,7 +120,7 @@ async def list_logs_for_module( except Exception as e: logger.error(f"Error listing logs for module {module_name}: {e}") return error( - f"Error listing module logs: {str(e)}", + "Error listing module logs", code="MODULE_LOGS_LIST_ERROR", status_code=500, ) @@ -210,7 +210,7 @@ def read_log( except Exception as e: logger.error(f"Error reading log file {module}/{filename}: {e}") return error( - f"Error reading log file: {str(e)}", + "Error reading log file", code="LOG_READ_ERROR", status_code=500, ) diff --git a/backend/api/main.py b/backend/api/main.py index 312f6dab..0ae3c98e 100755 --- a/backend/api/main.py +++ b/backend/api/main.py @@ -40,6 +40,7 @@ ConfigError, ConfigValidationError, format_validation_errors, + format_validation_errors_public, load_config, ) from backend.util.database import ChubDB @@ -444,16 +445,19 @@ async def handle_config_error(request: Request, exc: ConfigError) -> JSONRespons from the logs rather than appearing as an opaque internal error. """ logger = get_logger(request, "ERROR") - logger.error(f"Configuration error: {exc}") - detail_lines = [] + # exc_info carries the __cause__ the curated message deliberately omits + logger.error(f"Configuration error: {exc}", exc_info=True) + public_lines = [] if isinstance(exc, ConfigValidationError) and exc.validation_error: - detail_lines = format_validation_errors(exc.validation_error) - for line in detail_lines: + # The logged lines carry the offending values; the returned ones must not. + for line in format_validation_errors(exc.validation_error): logger.error(f" • {line}") + public_lines = format_validation_errors_public(exc.validation_error) return error( - f"Configuration invalid: {exc}", + # Read the curated attribute, never the exception object itself + "Configuration invalid: " + exc.message, code="CONFIG_INVALID", - data={"errors": detail_lines} if detail_lines else None, + data={"errors": public_lines} if public_lines else None, status_code=500, ) @@ -464,7 +468,7 @@ async def handle_exception(request: Request, exc: Exception) -> JSONResponse: logger = get_logger(request, "ERROR") logger.error(f"Unhandled Exception: {exc}", exc_info=True) return error( - f"Internal server error: {str(exc)}", code="INTERNAL_ERROR", status_code=500 + "Internal server error", code="INTERNAL_ERROR", status_code=500 ) @@ -528,14 +532,15 @@ async def handle_validation_exception( @app.get("/", response_class=HTMLResponse) -async def root() -> HTMLResponse: +async def root(request: Request) -> HTMLResponse: """Serves the main index.html page.""" html_path = STATIC_DIR / "index.html" try: return HTMLResponse(content=html_path.read_text(), status_code=200) except Exception as e: + get_logger(request, "ERROR").error(f"Error serving index page: {e}") return error( - f"Error serving index page: {str(e)}", + "Error serving index page", code="INDEX_PAGE_ERROR", status_code=500, ) diff --git a/backend/api/media_api.py b/backend/api/media_api.py index 9b11df8d..fe85661c 100644 --- a/backend/api/media_api.py +++ b/backend/api/media_api.py @@ -150,7 +150,7 @@ async def search_media( except Exception as e: logger.error(f"Error searching media: {e}") return error( - f"Error searching media: {str(e)}", + "Error searching media", code="MEDIA_SEARCH_ERROR", status_code=500, ) @@ -233,7 +233,7 @@ async def get_media_stats( except Exception as e: logger.error(f"Error retrieving media stats: {e}") return error( - f"Error retrieving media stats: {str(e)}", + "Error retrieving media stats", code="MEDIA_STATS_ERROR", status_code=500, ) @@ -285,7 +285,7 @@ async def get_media_stats_detailed( except Exception as e: logger.error(f"Error retrieving detailed media stats: {e}") return error( - f"Error retrieving detailed media stats: {str(e)}", + "Error retrieving detailed media stats", code="MEDIA_STATS_DETAILED_ERROR", status_code=500, ) @@ -338,7 +338,7 @@ async def get_genres( except Exception as e: logger.error(f"Error retrieving genres: {e}") return error( - f"Error retrieving genres: {str(e)}", + "Error retrieving genres", code="GENRES_ERROR", status_code=500, ) @@ -396,7 +396,7 @@ async def get_collections( except Exception as e: logger.error(f"Error retrieving collections: {e}") return error( - f"Error retrieving collections: {str(e)}", + "Error retrieving collections", code="COLLECTIONS_ERROR", status_code=500, ) @@ -514,7 +514,7 @@ def _not_excluded(dup): except Exception as e: logger.error(f"Error finding duplicates: {e}") return error( - f"Error finding duplicates: {str(e)}", + "Error finding duplicates", code="DUPLICATES_ERROR", status_code=500, ) @@ -582,7 +582,7 @@ async def refresh_media( except Exception as e: logger.error(f"Error serving POST /api/media/refresh: {e}") return error( - f"Error refreshing media: {str(e)}", + "Error refreshing media", code="MEDIA_REFRESH_ERROR", status_code=500, ) @@ -674,7 +674,7 @@ def _build_export() -> JSONResponse: except Exception as e: logger.error(f"Error exporting media: {e}") return error( - f"Error exporting media: {str(e)}", + "Error exporting media", code="MEDIA_EXPORT_ERROR", status_code=500, ) @@ -734,7 +734,7 @@ async def delete_collection( except Exception as e: logger.error(f"Error deleting collection: {e}") return error( - f"Error deleting collection: {str(e)}", + "Error deleting collection", code="COLLECTION_DELETE_ERROR", status_code=500, ) @@ -820,7 +820,7 @@ async def create_collection( except Exception as e: logger.error(f"Error creating collection: {e}") return error( - f"Error creating collection: {str(e)}", + "Error creating collection", code="COLLECTION_CREATE_ERROR", status_code=500, ) @@ -912,7 +912,7 @@ async def update_collection( except Exception as e: logger.error(f"Error updating collection: {e}") return error( - f"Error updating collection: {str(e)}", + "Error updating collection", code="COLLECTION_UPDATE_ERROR", status_code=500, ) @@ -1058,6 +1058,8 @@ def _remove_media_item(db, logger, rid, delete_files, add_exclusion): logger.warning( f"Instance {instance_name} not found in config, skipping ARR delete" ) + except ConfigError: + raise except Exception as e: logger.error(f"ARR delete failed for id={rid}: {e}") # Continue with cache deletion even if ARR delete fails @@ -1180,7 +1182,7 @@ async def get_low_rated( except Exception as e: logger.error(f"Error querying low-rated media: {e}") return error( - f"Error querying low-rated media: {str(e)}", + "Error querying low-rated media", code="LOW_RATED_ERROR", status_code=500, ) @@ -1318,7 +1320,7 @@ async def get_incomplete_metadata( except Exception as e: logger.error(f"Error querying incomplete metadata: {e}") return error( - f"Error querying incomplete metadata: {str(e)}", + "Error querying incomplete metadata", code="INCOMPLETE_META_ERROR", status_code=500, ) @@ -1361,6 +1363,7 @@ def get_orphaned_cache( logger: Any = Depends(get_logger), db: ChubDB = Depends(get_database), ) -> JSONResponse: + """List cache rows whose ARR source no longer has the item.""" try: config = load_config() live = _live_arr_ids_by_instance(config, logger) @@ -1386,10 +1389,12 @@ def get_orphaned_cache( "instances_checked": sorted(live.keys()), }, ) + except ConfigError: + raise except Exception as e: logger.error(f"Error finding orphaned cache: {e}", exc_info=True) return error( - f"Error finding orphaned cache: {str(e)}", + "Error finding orphaned cache", code="ORPHANED_ERROR", status_code=500, ) @@ -1580,10 +1585,12 @@ def _resolve(mid): with ThreadPoolExecutor(max_workers=min(8, len(body.ids))) as pool: members = list(pool.map(_resolve, body.ids)) return ok("Duplicate members resolved", {"members": members}) + except ConfigError: + raise except Exception as e: logger.error(f"Error fetching duplicate members: {e}", exc_info=True) return error( - f"Error fetching duplicate members: {str(e)}", + "Error fetching duplicate members", code="DUPLICATE_MEMBERS_ERROR", status_code=500, ) @@ -1615,7 +1622,7 @@ async def purge_orphaned_cache( except Exception as e: logger.error(f"Error purging orphaned cache: {e}", exc_info=True) return error( - f"Error purging orphaned cache: {str(e)}", + "Error purging orphaned cache", code="PURGE_ERROR", status_code=500, ) @@ -1683,7 +1690,7 @@ async def get_media_item( except Exception as e: logger.error(f"Error retrieving media item {media_id}: {e}") return error( - f"Error retrieving media item: {str(e)}", + "Error retrieving media item", code="MEDIA_GET_ERROR", status_code=500, ) @@ -1808,7 +1815,7 @@ async def update_media_metadata( except Exception as e: logger.error(f"Error updating media item {media_id}: {e}") return error( - f"Error updating media item: {str(e)}", + "Error updating media item", code="MEDIA_UPDATE_ERROR", status_code=500, ) @@ -1961,6 +1968,7 @@ async def delete_media_item( # If deleteFiles requested, remove from ARR first. The connect probe + # delete request are blocking, so run them off the event loop. def _delete_from_arr() -> None: + """Best-effort removal of the item from its source ARR instance.""" if not (delete_files and item.get("arr_id") and item.get("instance_name")): return try: @@ -1996,6 +2004,8 @@ def _delete_from_arr() -> None: logger.warning( f"Instance {instance_name} not found in config, skipping ARR delete" ) + except ConfigError: + raise except Exception as arr_err: logger.error(f"ARR delete failed for {media_id}: {arr_err}") # Continue with cache deletion even if ARR delete fails @@ -2009,10 +2019,12 @@ def _delete_from_arr() -> None: {"deleted_id": media_id, "files_deleted": delete_files}, ) + except ConfigError: + raise except Exception as e: logger.error(f"Error deleting media item {media_id}: {e}") return error( - f"Error deleting media item: {str(e)}", + "Error deleting media item", code="MEDIA_DELETE_ERROR", status_code=500, ) @@ -2117,7 +2129,7 @@ async def generate_collection_from_tag( except Exception as e: logger.error(f"Error generating collection from tag: {e}") return error( - f"Error generating collection: {str(e)}", + "Error generating collection", code="COLLECTION_FROM_TAG_ERROR", status_code=500, ) @@ -2151,7 +2163,7 @@ async def get_media_history( except Exception as e: logger.error(f"Error reading media history: {e}") return error( - f"Error reading media history: {str(e)}", + "Error reading media history", code="MEDIA_HISTORY_ERROR", status_code=500, ) @@ -2168,6 +2180,7 @@ def get_import_exclusion( logger: Any = Depends(get_logger), db: ChubDB = Depends(get_database), ) -> JSONResponse: + """Report whether a media item is excluded from ARR import lists.""" try: item = db.media.get_by_id(media_id) if not item: @@ -2228,10 +2241,12 @@ def get_import_exclusion( "exclusion_count": len(entries or []), }, ) + except ConfigError: + raise except Exception as e: logger.error(f"Error resolving import exclusion: {e}") return error( - f"Error resolving import exclusion: {str(e)}", + "Error resolving import exclusion", code="EXCLUSION_ERROR", status_code=500, ) diff --git a/backend/api/modules.py b/backend/api/modules.py index 168d3525..be3732ad 100755 --- a/backend/api/modules.py +++ b/backend/api/modules.py @@ -16,6 +16,7 @@ from pydantic import BaseModel from backend.api.utils import error, get_database, get_logger, ok +from backend.util.config import ConfigError from backend.util.database import ChubDB @@ -148,10 +149,12 @@ async def list_modules( ) return ok(f"Retrieved {len(modules_list)} modules", {"modules": modules_list}) + except ConfigError: + raise except Exception as e: logger.error(f"Error listing modules: {e}") return error( - f"Error listing modules: {str(e)}", + "Error listing modules", code="MODULES_LIST_ERROR", status_code=500, ) @@ -245,7 +248,7 @@ async def get_modules_history( except Exception as e: logger.error(f"Error retrieving module history: {e}") return error( - f"Error retrieving history: {str(e)}", + "Error retrieving history", code="MODULE_HISTORY_ERROR", status_code=500, ) @@ -319,7 +322,7 @@ async def get_modules_stats( except Exception as e: logger.error(f"Error retrieving module stats: {e}") return error( - f"Error retrieving stats: {str(e)}", + "Error retrieving stats", code="MODULE_STATS_ERROR", status_code=500, ) @@ -400,7 +403,7 @@ async def get_all_run_states( except Exception as e: logger.error(f"Error getting run states: {e}") return error( - f"Error getting run states: {str(e)}", + "Error getting run states", code="RUN_STATE_ERROR", status_code=500, ) @@ -564,7 +567,7 @@ async def module_status( except Exception as e: logger.error(f"Error getting status for module {module}: {e}") return error( - f"Error getting module status: {str(e)}", + "Error getting module status", code="MODULE_STATUS_ERROR", status_code=500, ) @@ -651,10 +654,12 @@ async def get_module( } return ok(f"Module '{name}' details retrieved", module_info) + except ConfigError: + raise except Exception as e: logger.error(f"Error retrieving module {name}: {e}") return error( - f"Error retrieving module: {str(e)}", + "Error retrieving module", code="MODULE_GET_ERROR", status_code=500, ) @@ -735,7 +740,7 @@ async def execute_module_by_name( except Exception as e: logger.error(f"Error executing module {name}: {e}") return error( - f"Error executing module: {str(e)}", + "Error executing module", code="MODULE_EXECUTE_ERROR", status_code=500, ) @@ -793,7 +798,7 @@ async def get_module_job_status( except Exception as e: logger.error(f"Error retrieving job {job_id}: {e}") return error( - f"Error retrieving job status: {str(e)}", + "Error retrieving job status", code="JOB_STATUS_ERROR", status_code=500, ) @@ -858,7 +863,7 @@ async def get_module_schema( except Exception as e: logger.error(f"Error retrieving schema for {name}: {e}") return error( - f"Error retrieving schema: {str(e)}", code="SCHEMA_ERROR", status_code=500 + "Error retrieving schema", code="SCHEMA_ERROR", status_code=500 ) @@ -945,15 +950,18 @@ async def update_module_config( return ok(f"Configuration for '{name}' updated", {"module": name}) except ValueError as e: + logger.error(f"Config validation failed for {name}: {e}") return error( - f"Config validation failed: {str(e)}", + "Config validation failed", code="CONFIG_VALIDATION_ERROR", status_code=400, ) + except ConfigError: + raise except Exception as e: logger.error(f"Error updating config for {name}: {e}") return error( - f"Error updating config: {str(e)}", + "Error updating config", code="CONFIG_UPDATE_ERROR", status_code=500, ) @@ -1030,10 +1038,12 @@ async def toggle_module( f"Module '{name}' {'enabled' if actual_enabled else 'disabled'}", {"module": name, "enabled": actual_enabled}, ) + except ConfigError: + raise except Exception as e: logger.error(f"Error toggling module {name}: {e}") return error( - f"Error toggling module: {str(e)}", + "Error toggling module", code="MODULE_TOGGLE_ERROR", status_code=500, ) @@ -1115,7 +1125,7 @@ async def get_module_history( except Exception as e: logger.error(f"Error retrieving history for {name}: {e}") return error( - f"Error retrieving history: {str(e)}", + "Error retrieving history", code="MODULE_HISTORY_ERROR", status_code=500, ) @@ -1178,7 +1188,7 @@ async def get_module_specific_stats( except Exception as e: logger.error(f"Error retrieving stats for {name}: {e}") return error( - f"Error retrieving stats: {str(e)}", + "Error retrieving stats", code="MODULE_STATS_ERROR", status_code=500, ) @@ -1270,7 +1280,7 @@ async def cancel_module_execution( except Exception as e: logger.error(f"Error cancelling job {job_id}: {e}", exc_info=True) return error( - f"Error cancelling execution: {str(e)}", + "Error cancelling execution", code="CANCEL_ERROR", status_code=500, ) @@ -1431,12 +1441,12 @@ def test_module( {"instance": inst_name, "service": service, "status": "unreachable"} ) except Exception as exc: + logger.error(f"Instance test failed for '{inst_name}': {exc}") results.append( { "instance": inst_name, "service": service, "status": "error", - "error": str(exc), } ) @@ -1446,10 +1456,12 @@ def test_module( {"results": results}, ) + except ConfigError: + raise except Exception as e: logger.error(f"Error testing module {name}: {e}") return error( - f"Error testing module: {str(e)}", + "Error testing module", code="MODULE_TEST_ERROR", status_code=500, ) @@ -1532,7 +1544,7 @@ async def run_module( except Exception as e: logger.error(f"Error running module {module}: {e}", exc_info=True) return error( - f"Error running module: {str(e)}", + "Error running module", code="MODULE_START_ERROR", status_code=500, ) diff --git a/backend/api/nestarr.py b/backend/api/nestarr.py index 3897f73b..cecf2c83 100644 --- a/backend/api/nestarr.py +++ b/backend/api/nestarr.py @@ -17,7 +17,7 @@ save_scan_results, ) from backend.util.arr import create_arr_client -from backend.util.config import load_config +from backend.util.config import ConfigError, load_config from backend.util.database import ChubDB router = APIRouter( @@ -103,10 +103,12 @@ def _scan_nested_media_sync(logger, db: ChubDB): "unmatched_enabled": bool(config.nestarr.library_mappings), }, ) + except ConfigError: + raise except Exception as e: logger.error(f"Scan failed: {e}", exc_info=True) return error( - message=f"Scan failed: {str(e)}", + message="Scan failed", code="SCAN_FAILED", status_code=500, ) @@ -268,10 +270,12 @@ def preview_fix(request: Request, body: FixRequest): }, ) + except ConfigError: + raise except Exception as e: logger.error(f"Preview failed: {e}", exc_info=True) return error( - message=f"Preview failed: {str(e)}", + message="Preview failed", code="PREVIEW_FAILED", status_code=500, ) @@ -363,10 +367,12 @@ def fix_nested_media( }, ) + except ConfigError: + raise except Exception as e: logger.error(f"Fix failed: {e}", exc_info=True) return error( - message=f"Fix failed: {str(e)}", + message="Fix failed", code="FIX_FAILED", status_code=500, ) diff --git a/backend/api/notifications.py b/backend/api/notifications.py index bd5ae7f2..b77367d7 100755 --- a/backend/api/notifications.py +++ b/backend/api/notifications.py @@ -30,6 +30,7 @@ from backend.util.config import ( ALL_MODULES_SENTINEL, ChubConfig, + ConfigError, ConfigNotifications, load_config, redact_secrets, @@ -169,7 +170,7 @@ async def get_all_notifications( except Exception as e: logger.error(f"Error retrieving notifications: {e}") return error( - f"Error retrieving notifications: {str(e)}", + "Error retrieving notifications", code="NOTIFICATIONS_RETRIEVAL_ERROR", status_code=500, ) @@ -183,6 +184,7 @@ async def get_all_notifications( async def create_destination( payload: DestinationPayload, logger: Any = Depends(get_logger) ) -> JSONResponse: + """Add one notification destination to the saved config.""" try: method_err = _validate_method(payload.method) if method_err: @@ -214,10 +216,12 @@ async def create_destination( "Notification destination created successfully", {"destination": redact_secrets(new_dest)}, ) + except ConfigError: + raise except Exception as e: logger.error(f"Failed to create notification destination: {e}") return error( - f"Failed to create destination: {str(e)}", + "Failed to create destination", code="NOTIFICATION_CREATE_ERROR", status_code=500, ) @@ -234,6 +238,7 @@ async def update_destination( payload: DestinationPayload, logger: Any = Depends(get_logger), ) -> JSONResponse: + """Replace one saved notification destination by id.""" try: method_err = _validate_method(payload.method) if method_err: @@ -280,10 +285,12 @@ async def update_destination( "Notification destination updated successfully", {"destination": redact_secrets(updated)}, ) + except ConfigError: + raise except Exception as e: logger.error(f"Failed to update notification destination {destination_id}: {e}") return error( - f"Failed to update destination: {str(e)}", + "Failed to update destination", code="NOTIFICATION_UPDATE_ERROR", status_code=500, ) @@ -297,6 +304,7 @@ async def update_destination( async def delete_destination( destination_id: str, logger: Any = Depends(get_logger) ) -> JSONResponse: + """Remove one saved notification destination by id.""" try: config = load_config() destinations = _destinations_as_dicts(config) @@ -318,10 +326,12 @@ async def delete_destination( "Notification destination deleted successfully", {"id": destination_id}, ) + except ConfigError: + raise except Exception as e: logger.error(f"Failed to delete notification destination {destination_id}: {e}") return error( - f"Failed to delete destination: {str(e)}", + "Failed to delete destination", code="NOTIFICATION_DELETE_ERROR", status_code=500, ) @@ -340,6 +350,7 @@ async def delete_destination( def test_notification( payload: TestPayload, logger: Any = Depends(get_logger) ) -> JSONResponse: + """Send a one-off test message through the supplied destination.""" try: logger.debug("Serving POST /api/notifications/test (%s)", payload.method) @@ -377,21 +388,23 @@ def test_notification( except ValueError as e: logger.error(f"Invalid notification configuration: {e}") return error( - f"Invalid notification configuration: {str(e)}", + "Invalid notification configuration", code="NOTIFICATION_CONFIG_INVALID", status_code=400, ) except ConnectionError as e: logger.error(f"Notification service connection failed: {e}") return error( - f"Failed to connect to notification service: {str(e)}", + "Failed to connect to notification service", code="NOTIFICATION_CONNECTION_FAILED", status_code=502, ) + except ConfigError: + raise except Exception as e: logger.error(f"Test notification failed: {e}") return error( - f"Test notification failed: {str(e)}", + "Test notification failed", code="NOTIFICATION_TEST_ERROR", status_code=500, ) diff --git a/backend/api/posters.py b/backend/api/posters.py index f87f8462..b7741593 100644 --- a/backend/api/posters.py +++ b/backend/api/posters.py @@ -15,9 +15,17 @@ from starlette.background import BackgroundTask from starlette.concurrency import run_in_threadpool -from backend.api.utils import error, get_database, get_logger, get_module_logger, ok +from backend.api.utils import ( + error, + get_database, + get_logger, + get_module_logger, + ok, + worker_error, +) from backend.modules.sync_gdrive import SyncGDrive from backend.modules.unmatched_assets import UnmatchedAssets +from backend.util.config import ConfigError from backend.util.database import ChubDB, escape_like from backend.util.helper import get_static_dir @@ -106,7 +114,7 @@ async def search_posters( except Exception as e: logger.error(f"Error searching posters: {e}") return error( - f"Error searching posters: {str(e)}", + "Error searching posters", code="POSTER_SEARCH_ERROR", status_code=500, ) @@ -192,7 +200,7 @@ async def get_poster_stats( except Exception as e: logger.error(f"Error retrieving poster stats: {e}") return error( - f"Error retrieving poster statistics: {str(e)}", + "Error retrieving poster statistics", code="POSTER_STATS_ERROR", status_code=500, ) @@ -274,7 +282,7 @@ async def get_poster_collections( except Exception as e: logger.error(f"Error retrieving poster collections: {e}") return error( - f"Error retrieving collections: {str(e)}", + "Error retrieving collections", code="POSTER_COLLECTIONS_ERROR", status_code=500, ) @@ -374,10 +382,12 @@ async def search_gdrive_sources( item["last_updated"] = loc_stats.get("last_updated") return ok(f"Found {len(gdrive_list)} GDrive sources", {"sources": gdrive_list}) + except ConfigError: + raise except Exception as e: logger.error(f"Error searching GDrive sources: {e}") return error( - f"Error searching GDrive sources: {str(e)}", + "Error searching GDrive sources", code="GDRIVE_SEARCH_ERROR", status_code=500, ) @@ -430,7 +440,7 @@ async def auto_match_posters( except Exception as e: logger.error(f"Error initiating auto-match: {e}") return error( - f"Error initiating auto-match: {str(e)}", + "Error initiating auto-match", code="AUTO_MATCH_ERROR", status_code=500, ) @@ -536,16 +546,20 @@ async def browse_posters( head = name.split(None, 1)[0] if head: configured_styles.add(head) + except ConfigError: + raise except Exception as e: logger.debug(f"Could not derive configured styles from config: {e}") result["styles"] = sorted(db_styles | configured_styles) return ok( f"Retrieved {len(result['items'])} of {result['total']} posters", result ) + except ConfigError: + raise except Exception as e: logger.error(f"Error browsing posters: {e}") return error( - f"Error browsing posters: {str(e)}", + "Error browsing posters", code="POSTER_BROWSE_ERROR", status_code=500, ) @@ -661,10 +675,12 @@ async def upload_poster( {"filename": safe_name, "path": dest_path, "size_bytes": len(contents)}, ) + except ConfigError: + raise except Exception as e: logger.error(f"Error uploading poster: {e}") return error( - f"Error uploading poster: {str(e)}", + "Error uploading poster", code="POSTER_UPLOAD_ERROR", status_code=500, ) @@ -733,7 +749,7 @@ async def create_poster_collection( except Exception as e: logger.error(f"Error creating poster collection: {e}") return error( - f"Error creating poster collection: {str(e)}", + "Error creating poster collection", code="POSTER_COLLECTION_CREATE_ERROR", status_code=500, ) @@ -816,7 +832,7 @@ async def add_to_collection( except Exception as e: logger.error(f"Error adding poster to collection: {e}") return error( - f"Error adding poster to collection: {str(e)}", + "Error adding poster to collection", code="POSTER_COLLECTION_ADD_ERROR", status_code=500, ) @@ -885,7 +901,7 @@ async def remove_from_collection( except Exception as e: logger.error(f"Error removing poster from collection: {e}") return error( - f"Error removing poster from collection: {str(e)}", + "Error removing poster from collection", code="POSTER_COLLECTION_REMOVE_ERROR", status_code=500, ) @@ -933,7 +949,7 @@ async def delete_poster_collection( except Exception as e: logger.error(f"Error deleting poster collection: {e}") return error( - f"Error deleting poster collection: {str(e)}", + "Error deleting poster collection", code="POSTER_COLLECTION_DELETE_ERROR", status_code=500, ) @@ -1167,7 +1183,7 @@ def _optimize_posters_sync( except Exception as e: logger.error(f"Error optimizing posters: {e}", exc_info=True) return error( - f"Error optimizing posters: {str(e)}", + "Error optimizing posters", code="OPTIMIZE_ERROR", status_code=500, ) @@ -1230,7 +1246,7 @@ async def list_poster_files(logger: Any = Depends(get_logger)) -> JSONResponse: except Exception as e: logger.error(f"Error listing poster files: {e}") return error( - f"Error listing poster files: {str(e)}", + "Error listing poster files", code="POSTER_LIST_ERROR", status_code=500, ) @@ -1289,7 +1305,7 @@ async def get_matched_poster_stats( except Exception as e: logger.error(f"Error retrieving matched posters stats: {e}") return error( - f"Error retrieving matched posters statistics: {str(e)}", + "Error retrieving matched posters statistics", code="MATCHED_POSTERS_STATS_ERROR", status_code=500, ) @@ -1346,7 +1362,7 @@ async def get_unmatched_assets_stats(logger: Any = Depends(get_logger)) -> JSONR except Exception as e: logger.error(f"Error retrieving unmatched stats: {e}") return error( - f"Error retrieving unmatched assets statistics: {str(e)}", + "Error retrieving unmatched assets statistics", code="UNMATCHED_STATS_ERROR", status_code=500, ) @@ -1382,7 +1398,7 @@ async def get_unmatched_assets_details( except Exception as e: logger.error(f"Error retrieving unmatched details: {e}") return error( - f"Error retrieving unmatched assets details: {str(e)}", + "Error retrieving unmatched assets details", code="UNMATCHED_DETAILS_ERROR", status_code=500, ) @@ -1414,7 +1430,7 @@ async def get_unmatched_artwork( except Exception as e: logger.error(f"Error retrieving unmatched artwork: {e}") return error( - f"Error retrieving unmatched artwork: {str(e)}", + "Error retrieving unmatched artwork", code="UNMATCHED_ARTWORK_ERROR", status_code=500, ) @@ -1456,7 +1472,7 @@ async def ignore_artwork( except Exception as e: logger.error(f"Error updating artwork ignore for {media_id}/{image_type}: {e}") return error( - f"Error updating artwork ignore flag: {str(e)}", + "Error updating artwork ignore flag", code="ARTWORK_IGNORE_ERROR", status_code=500, ) @@ -1581,7 +1597,7 @@ async def get_artwork_candidates( f"Error fetching artwork candidates for {media_id}/{image_type}: {e}" ) return error( - f"Error fetching artwork candidates: {str(e)}", + "Error fetching artwork candidates", code="ARTWORK_CANDIDATES_ERROR", status_code=500, ) @@ -1656,7 +1672,7 @@ def apply_artwork( except Exception as e: logger.error(f"Error applying artwork for {media_id}/{image_type}: {e}") return error( - f"Error applying artwork: {str(e)}", + "Error applying artwork", code="ARTWORK_APPLY_ERROR", status_code=500, ) @@ -1698,7 +1714,7 @@ async def unlock_artwork( except Exception as e: logger.error(f"Error unlocking artwork for {media_id}/{image_type}: {e}") return error( - f"Error unlocking artwork: {str(e)}", + "Error unlocking artwork", code="ARTWORK_UNLOCK_ERROR", status_code=500, ) @@ -1734,7 +1750,7 @@ async def ignore_match( except Exception as e: logger.error(f"Error updating ignore flag for {media_id}: {e}") return error( - f"Error updating ignore flag: {str(e)}", + "Error updating ignore flag", code="MATCH_IGNORE_ERROR", status_code=500, ) @@ -1770,7 +1786,7 @@ async def approve_match( except Exception as e: logger.error(f"Error approving match for {media_id}: {e}") return error( - f"Error approving match: {str(e)}", + "Error approving match", code="MATCH_APPROVE_ERROR", status_code=500, ) @@ -1807,7 +1823,7 @@ async def unlock_match( except Exception as e: logger.error(f"Error unlocking match for {media_id}: {e}") return error( - f"Error unlocking match: {str(e)}", + "Error unlocking match", code="MATCH_UNLOCK_ERROR", status_code=500, ) @@ -1924,7 +1940,7 @@ async def get_match_candidates( except Exception as e: logger.error(f"Error fetching match candidates for {media_id}: {e}") return error( - f"Error fetching candidates: {str(e)}", + "Error fetching candidates", code="MATCH_CANDIDATES_ERROR", status_code=500, ) @@ -2075,7 +2091,7 @@ def apply_match( except Exception as e: logger.error(f"Error applying poster {poster_id} to {media_id}: {e}") return error( - f"Error applying poster: {str(e)}", + "Error applying poster", code="MATCH_APPLY_ERROR", status_code=500, ) @@ -2136,7 +2152,7 @@ async def get_gdrive_stats( except Exception as e: logger.error(f"Error retrieving GDrive stats: {e}") return error( - f"Error retrieving GDrive statistics: {str(e)}", + "Error retrieving GDrive statistics", code="GDRIVE_STATS_ERROR", status_code=500, ) @@ -2262,7 +2278,7 @@ async def sync_gdrive_folders( except Exception as e: logger.error(f"Error starting GDrive sync: {e}") return error( - f"Error starting GDrive synchronization: {str(e)}", + "Error starting GDrive synchronization", code="GDRIVE_SYNC_START_ERROR", status_code=500, ) @@ -2334,6 +2350,8 @@ async def delete_gdrive_local( # Fail closed: without config we can't verify the path is a real drive. try: config = load_config() + except ConfigError: + raise except Exception: # noqa: S110 — treated as unavailable below config = None if config is None: @@ -2382,6 +2400,16 @@ async def delete_gdrive_local( # match above — never the raw request path, so a validated request string # can't reach rmtree. target == req_real by that match. target = os.path.realpath(matched.location) + # Re-confine the RE-RESOLVED path right before deleting: a symlink + # component can change between the membership check above and here. + if target != req_real or target == os.path.realpath(os.sep): + logger.error(f"Refusing to delete '{target}': resolved path changed") + return error( + "Location is not a configured Google Drive folder", + code="GDRIVE_LOCATION_NOT_CONFIGURED", + status_code=400, + ) + folder_removed = False if os.path.isdir(target): shutil.rmtree(target) @@ -2404,10 +2432,12 @@ async def delete_gdrive_local( }, ) + except ConfigError: + raise except Exception as e: logger.error(f"Error deleting local GDrive folder: {e}") return error( - f"Error deleting local GDrive folder: {str(e)}", + "Error deleting local GDrive folder", code="GDRIVE_DELETE_LOCAL_ERROR", status_code=500, ) @@ -2462,6 +2492,8 @@ async def analyze_poster_directory( try: config = load_config() + except ConfigError: + raise except Exception: # noqa: S110 — config may not be loaded at boot config = None @@ -2529,10 +2561,12 @@ async def analyze_poster_directory( }, ) + except ConfigError: + raise except Exception as e: logger.error(f"Error analyzing poster location: {e}") return error( - f"Error analyzing poster location: {str(e)}", + "Error analyzing poster location", code="POSTER_ANALYSIS_ERROR", status_code=500, ) @@ -2597,6 +2631,8 @@ async def preview_poster_file( try: config = load_config() + except ConfigError: + raise except Exception: # noqa: S110 — fail closed below config = None @@ -2660,10 +2696,12 @@ async def preview_poster_file( return FileResponse(str(file_path)) + except ConfigError: + raise except Exception as e: logger.error(f"Error serving poster preview: {e}") return error( - f"Error serving poster preview: {str(e)}", + "Error serving poster preview", code="POSTER_PREVIEW_ERROR", status_code=500, ) @@ -2721,16 +2759,17 @@ def upload_media_posters( result.get("data", {}), ) else: - return error( - f"Upload failed for media cache item {media_id}: {result.get('message', 'Unknown error')}", - code="MEDIA_UPLOAD_FAILED", - status_code=500, + return worker_error( + result, + logger, + f"Upload failed for media cache item {media_id}", + "MEDIA_UPLOAD_FAILED", ) except Exception as e: logger.error(f"Error uploading media cache item {media_id}: {e}") return error( - f"Error triggering upload: {str(e)}", + "Error triggering upload", code="MEDIA_UPLOAD_ERROR", status_code=500, ) @@ -2790,16 +2829,17 @@ def upload_collection_posters( result.get("data", {}), ) else: - return error( - f"Upload failed for collection cache item {collection_id}: {result.get('message', 'Unknown error')}", - code="COLLECTION_UPLOAD_FAILED", - status_code=500, + return worker_error( + result, + logger, + f"Upload failed for collection cache item {collection_id}", + "COLLECTION_UPLOAD_FAILED", ) except Exception as e: logger.error(f"Error uploading collection cache item {collection_id}: {e}") return error( - f"Error triggering upload: {str(e)}", + "Error triggering upload", code="COLLECTION_UPLOAD_ERROR", status_code=500, ) @@ -2860,7 +2900,7 @@ def backfill_poster_dimensions( except Exception as e: logger.error(f"Error backfilling poster dimensions: {e}") return error( - f"Error backfilling poster dimensions: {str(e)}", + "Error backfilling poster dimensions", code="BACKFILL_DIMENSIONS_ERROR", status_code=500, ) @@ -2889,7 +2929,7 @@ async def list_low_resolution_posters( except Exception as e: logger.error(f"Error listing low-resolution posters: {e}") return error( - f"Error listing low-resolution posters: {str(e)}", + "Error listing low-resolution posters", code="LOW_RES_ERROR", status_code=500, ) @@ -2916,7 +2956,7 @@ async def list_posters_added_since( except Exception as e: logger.error(f"Error listing posters added_since: {e}") return error( - f"Error listing posters added_since: {str(e)}", + "Error listing posters added_since", code="ADDED_SINCE_ERROR", status_code=500, ) @@ -2940,7 +2980,7 @@ async def list_recently_matched( except Exception as e: logger.error(f"Error listing recently matched posters: {e}") return error( - f"Error listing recently matched posters: {str(e)}", + "Error listing recently matched posters", code="RECENTLY_MATCHED_ERROR", status_code=500, ) @@ -2974,7 +3014,7 @@ async def list_applied_media_by_style( except Exception as e: logger.error(f"Error listing applied media by style: {e}") return error( - f"Error listing applied media: {str(e)}", + "Error listing applied media", code="APPLIED_MEDIA_ERROR", status_code=500, ) @@ -2994,6 +3034,8 @@ def _get_plex_path(request: Request) -> Optional[str]: from backend.util.config import load_config cfg = load_config() + except ConfigError: + raise except Exception: return None section = getattr(cfg, "poster_cleanarr", None) @@ -3008,14 +3050,17 @@ def _get_cleanarr_excluded_libraries(request: Request) -> List[str]: """Plex library names the user opted out of in poster_cleanarr config. Display-side mirror of the module's deletion-side deny-list — hides excluded - libraries from the by-media view and its libraries[] catalog. Best-effort: a - config failure returns [] (show everything). This is a UI filter, not a - safety guard — the in-use set is global regardless of any opt-out. + libraries from the by-media view and its libraries[] catalog. A malformed + config propagates (CONFIG_INVALID); other failures return [] (show + everything). This is a UI filter, not a safety guard — the in-use set is + global regardless of any opt-out. """ try: from backend.util.config import load_config cfg = load_config() + except ConfigError: + raise except Exception: return [] section = getattr(cfg, "poster_cleanarr", None) @@ -3154,7 +3199,7 @@ async def list_plex_metadata_by_media( except Exception as e: logger.error(f"Error listing plex metadata bundles: {e}") return error( - f"Error listing plex metadata: {str(e)}", + "Error listing plex metadata", code="PLEX_METADATA_LIST_ERROR", status_code=500, ) @@ -3207,7 +3252,7 @@ async def list_plex_metadata_bloat( except Exception as e: logger.error(f"Error listing plex metadata bloat: {e}") return error( - f"Error listing plex metadata bloat: {str(e)}", + "Error listing plex metadata bloat", code="PLEX_METADATA_BLOAT_ERROR", status_code=500, ) @@ -3286,7 +3331,8 @@ async def run_plex_metadata_cleanup( try: overrides = _build_cleanup_overrides(body) except ValueError as ve: - return error(str(ve), code="INVALID_MODE", status_code=400) + logger.error(f"Invalid cleanup request: {ve}") + return error("Invalid cleanup mode", code="INVALID_MODE", status_code=400) mode = overrides["mode"] payload = { @@ -3307,7 +3353,7 @@ async def run_plex_metadata_cleanup( except Exception as e: logger.error(f"Error enqueuing cleanup: {e}") return error( - f"Error enqueuing cleanup: {str(e)}", + "Error enqueuing cleanup", code="CLEANUP_ENQUEUE_ERROR", status_code=500, ) @@ -3346,7 +3392,7 @@ async def delete_plex_metadata_variant( except Exception as e: logger.error(f"Error deleting variant: {e}") return error( - f"Error deleting variant: {str(e)}", + "Error deleting variant", code="VARIANT_DELETE_ERROR", status_code=500, ) @@ -3436,10 +3482,12 @@ def _apply_active(): return ok( "Active poster updated", {"rating_key": rating_key, "path": safe_path} ) + except ConfigError: + raise except Exception as e: logger.error(f"Error setting active poster: {e}") return error( - f"Error setting active poster: {str(e)}", + "Error setting active poster", code="SET_ACTIVE_ERROR", status_code=500, ) @@ -3493,7 +3541,7 @@ async def scan_kometa_assets( except Exception as e: logger.error(f"Kometa asset scan read failed: {e}") return error( - f"Kometa asset scan read failed: {str(e)}", + "Kometa asset scan read failed", code="KOMETA_SCAN_ERROR", status_code=500, ) @@ -3526,7 +3574,7 @@ async def enqueue_plex_metadata_scan( except Exception as e: logger.error(f"Error enqueuing plex metadata scan: {e}") return error( - f"Error enqueuing scan: {str(e)}", + "Error enqueuing scan", code="SCAN_ENQUEUE_ERROR", status_code=500, ) @@ -3552,7 +3600,7 @@ async def enqueue_kometa_assets_scan( except Exception as e: logger.error(f"Error enqueuing kometa assets scan: {e}") return error( - f"Error enqueuing Kometa scan: {str(e)}", + "Error enqueuing Kometa scan", code="KOMETA_ENQUEUE_ERROR", status_code=500, ) @@ -3612,7 +3660,7 @@ async def get_poster( except Exception as e: logger.error(f"Error retrieving poster {poster_id}: {e}") return error( - f"Error retrieving poster: {str(e)}", + "Error retrieving poster", code="POSTER_GET_ERROR", status_code=500, ) @@ -3701,7 +3749,7 @@ def get_poster_thumbnail( except Exception as e: logger.error(f"Error generating thumbnail for poster {poster_id}: {e}") return error( - f"Error generating thumbnail: {str(e)}", + "Error generating thumbnail", code="THUMBNAIL_ERROR", status_code=500, ) @@ -3814,7 +3862,7 @@ def download_poster( except Exception as e: logger.error(f"Error downloading poster {poster_id}: {e}") return error( - f"Error downloading poster: {str(e)}", + "Error downloading poster", code="POSTER_DOWNLOAD_ERROR", status_code=500, ) @@ -3879,6 +3927,14 @@ async def delete_poster( except Exception: pass + # Authorize BEFORE the irreversible row delete: a malformed config must + # abort the whole request, not leave the row gone and the file orphaned. + config = None + if delete_file: + from backend.util.config import load_config + + config = load_config() + record = db.poster.delete_by_integer_id(poster_id) if not record: return error("Poster not found", code="POSTER_NOT_FOUND", status_code=404) @@ -3889,9 +3945,19 @@ async def delete_poster( full_path = ( os.path.join(folder, file_path) if folder and file_path else file_path ) - if delete_file and full_path and os.path.exists(full_path): - os.remove(full_path) - logger.info(f"Deleted poster file: {full_path}") + # A poisoned row must never reach os.remove: resolve first, then require + # membership in a configured root. + file_deleted = False + if delete_file and full_path: + from backend.util.path_safety import is_path_allowed + + real = os.path.realpath(full_path) + if real == os.path.realpath(os.sep) or not is_path_allowed(real, config): + logger.error(f"Refusing to delete poster file outside roots: {real}") + elif os.path.exists(real): + os.remove(real) + file_deleted = True + logger.info(f"Deleted poster file: {real}") # Mark associated media items as unmatched unmatched_count = 0 @@ -3930,14 +3996,16 @@ async def delete_poster( f"Poster {poster_id} deleted", { "deleted_id": poster_id, - "file_deleted": delete_file, + "file_deleted": file_deleted, "media_unmatched": unmatched_count, }, ) + except ConfigError: + raise except Exception as e: logger.error(f"Error deleting poster {poster_id}: {e}") return error( - f"Error deleting poster: {str(e)}", + "Error deleting poster", code="POSTER_DELETE_ERROR", status_code=500, ) diff --git a/backend/api/schedule.py b/backend/api/schedule.py index 05f8cf4c..0c3c822f 100755 --- a/backend/api/schedule.py +++ b/backend/api/schedule.py @@ -13,7 +13,13 @@ from backend.api.utils import error, get_logger, ok from backend.modules import MODULES -from backend.util.config import ChubConfig, ScheduleBlock, load_config, save_config +from backend.util.config import ( + ChubConfig, + ConfigError, + ScheduleBlock, + load_config, + save_config, +) from backend.util.scheduler import ( _profile_value, _upgradinatorr_profile_label, @@ -178,7 +184,7 @@ async def get_all_schedules( except Exception as e: logger.error(f"Error retrieving schedules: {e}") return error( - f"Error retrieving schedules: {str(e)}", + "Error retrieving schedules", code="SCHEDULES_RETRIEVAL_ERROR", status_code=500, ) @@ -238,7 +244,7 @@ async def get_module_schedule( except Exception as e: logger.error(f"Error retrieving schedule for module {module_id}: {e}") return error( - f"Error retrieving schedule: {str(e)}", + "Error retrieving schedule", code="SCHEDULE_RETRIEVAL_ERROR", status_code=500, ) @@ -304,8 +310,9 @@ async def update_module_schedule( try: validate_schedule(schedule_string) except ValueError as exc: + logger.error(f"Invalid schedule '{schedule_string}': {exc}") return error( - f"Invalid schedule '{schedule_string}': {exc}", + f"Invalid schedule '{schedule_string}'", code="INVALID_SCHEDULE", status_code=400, ) @@ -325,10 +332,12 @@ async def update_module_schedule( {"module": module_name, "schedule": schedule_string}, ) + except ConfigError: + raise except Exception as e: logger.error(f"Failed to update schedule for module {data.module}: {e}") return error( - f"Failed to update schedule: {str(e)}", + "Failed to update schedule", code="SCHEDULE_UPDATE_ERROR", status_code=500, ) @@ -362,8 +371,9 @@ async def update_module_schedule_blocks( try: validate_schedule(b.schedule) except ValueError as exc: + logger.error(f"Invalid schedule in block '{b.label}': {exc}") return error( - f"Invalid schedule in block '{b.label}': {exc}", + f"Invalid schedule in block '{b.label}'", code="INVALID_SCHEDULE", status_code=400, ) @@ -396,10 +406,12 @@ async def update_module_schedule_blocks( }, ) + except ConfigError: + raise except Exception as e: logger.error(f"Failed to update schedule blocks for module {data.module}: {e}") return error( - f"Failed to update schedule blocks: {str(e)}", + "Failed to update schedule blocks", code="SCHEDULE_BLOCKS_UPDATE_ERROR", status_code=500, ) @@ -467,10 +479,12 @@ async def delete_module_schedule( {"module": module_id}, ) + except ConfigError: + raise except Exception as e: logger.error(f"Failed to delete schedule for module {module_id}: {e}") return error( - f"Failed to delete schedule: {str(e)}", + "Failed to delete schedule", code="SCHEDULE_DELETE_ERROR", status_code=500, ) diff --git a/backend/api/setup.py b/backend/api/setup.py index 63e93ddd..4513eccc 100644 --- a/backend/api/setup.py +++ b/backend/api/setup.py @@ -51,5 +51,6 @@ async def complete_setup(logger: Any = Depends(get_logger)) -> JSONResponse: config.general.setup_completed = True save_config(config) except ConfigError as exc: - return error(f"Failed to save setup status: {exc}", status_code=500) + logger.error(f"Failed to save setup status: {exc}") + return error("Failed to save setup status", status_code=500) return ok("Setup marked complete", {"completed": True}) diff --git a/backend/api/system.py b/backend/api/system.py index 885c082c..72ece827 100755 --- a/backend/api/system.py +++ b/backend/api/system.py @@ -98,7 +98,7 @@ async def get_version_endpoint(logger: Any = Depends(get_logger)) -> JSONRespons except Exception as e: logger.error(f"Error getting version: {e}") return error( - f"Error getting version: {str(e)}", code="VERSION_ERROR", status_code=500 + "Error getting version", code="VERSION_ERROR", status_code=500 ) @@ -119,7 +119,7 @@ async def check_version_endpoint(logger: Any = Depends(get_logger)) -> JSONRespo except Exception as e: logger.error(f"Error checking for updates: {e}") return error( - f"Error checking for updates: {str(e)}", + "Error checking for updates", code="VERSION_CHECK_ERROR", status_code=500, ) @@ -281,17 +281,11 @@ async def list_directory( Dictionary containing directories list and path metadata """ try: - # Restrict to configured allowed roots - try: - config = load_config() - except ConfigError: - config = None - - # Fail closed: if the config can't be loaded (corrupt/invalid existing - # file), deny rather than allowing arbitrary directory enumeration or - # creation. A fresh no-file install returns a default config (not None), - # so the setup flow is unaffected. - if config is None or not is_path_allowed(path, config): + # Restrict to configured allowed roots. A corrupt config propagates as + # CONFIG_INVALID rather than masquerading as "path not allowed"; a fresh + # no-file install loads defaults, so the setup flow is unaffected. + config = load_config() + if not is_path_allowed(path, config): return error( "Access denied — path outside allowed directories", code="PATH_NOT_ALLOWED", @@ -322,10 +316,12 @@ async def list_directory( "writable": os.access(resolved, os.W_OK), }, ) + except ConfigError: + raise except Exception as e: logger.error(f"Error listing directory {path}: {e}") return error( - f"Error listing directory: {str(e)}", + "Error listing directory", code="DIRECTORY_LIST_ERROR", status_code=500, ) @@ -369,17 +365,11 @@ async def create_directory( try: path = request_data.path - # Restrict to configured allowed roots - try: - config = load_config() - except ConfigError: - config = None - - # Fail closed: if the config can't be loaded (corrupt/invalid existing - # file), deny rather than allowing arbitrary directory enumeration or - # creation. A fresh no-file install returns a default config (not None), - # so the setup flow is unaffected. - if config is None or not is_path_allowed(path, config): + # Restrict to configured allowed roots. A corrupt config propagates as + # CONFIG_INVALID rather than masquerading as "path not allowed"; a fresh + # no-file install loads defaults, so the setup flow is unaffected. + config = load_config() + if not is_path_allowed(path, config): return error( "Access denied — path outside allowed directories", code="PATH_NOT_ALLOWED", @@ -398,10 +388,12 @@ async def create_directory( code="FOLDER_EXISTS", status_code=400, ) + except ConfigError: + raise except Exception as e: logger.error(f"Error creating folder {path}: {e}") return error( - f"Error creating folder: {str(e)}", + "Error creating folder", code="FOLDER_CREATION_ERROR", status_code=500, ) @@ -429,10 +421,9 @@ async def create_directory( async def list_allowed_roots(logger: Any = Depends(get_logger)) -> JSONResponse: """Return the configured allowed roots for the directory picker.""" try: - try: - config = load_config() - except ConfigError: - return ok("0 allowed roots", {"roots": []}) + # A malformed config must surface as CONFIG_INVALID, not an empty + # picker list. A *missing* config loads defaults and never raises. + config = load_config() # The picker only wants top-level directories — file paths and # nested subdirs of an already-allowed root just create noise. @@ -440,10 +431,12 @@ async def list_allowed_roots(logger: Any = Depends(get_logger)) -> JSONResponse: # so write checks aren't affected by this filtering. roots = sorted({str(p) for p in get_browse_roots(config)}) return ok(f"{len(roots)} allowed roots", {"roots": roots}) + except ConfigError: + raise except Exception as e: logger.error(f"Error listing allowed roots: {e}") return error( - f"Error listing allowed roots: {str(e)}", + "Error listing allowed roots", code="ALLOWED_ROOTS_ERROR", status_code=500, ) @@ -494,7 +487,7 @@ async def list_gdrive_presets(logger: Any = Depends(get_logger)) -> JSONResponse except Exception as e: logger.error(f"Error loading gdrive presets: {e}") return error( - f"Error loading gdrive presets: {str(e)}", + "Error loading gdrive presets", code="GDRIVE_PRESETS_ERROR", status_code=500, ) @@ -544,7 +537,7 @@ async def test( except Exception as e: logger.error(f"Error processing test request: {e}") return error( - f"Error processing test request: {str(e)}", + "Error processing test request", code="TEST_ENDPOINT_ERROR", status_code=400, ) @@ -585,7 +578,7 @@ def create_backup( except Exception as e: logger.error(f"Backup creation failed: {e}") return error( - f"Backup creation failed: {str(e)}", + "Backup creation failed", code="BACKUP_ERROR", status_code=500, ) @@ -706,8 +699,9 @@ def _do_restore() -> JSONResponse: parsed = yaml.safe_load(raw_config) ChubConfig.model_validate(parsed) except Exception as e: + logger.error(f"config.yml in backup is invalid: {e}") return error( - f"config.yml in backup is invalid: {e}", + "config.yml in backup is invalid", code="INVALID_BACKUP_CONFIG", status_code=400, ) @@ -781,7 +775,7 @@ async def get_health_snapshots( except Exception as e: logger.error(f"Error fetching health snapshots: {e}") return error( - f"Error fetching health snapshots: {str(e)}", + "Error fetching health snapshots", code="HEALTH_SNAPSHOT_ERROR", status_code=500, ) @@ -877,7 +871,7 @@ async def get_system_digest( except Exception as e: logger.error(f"Error building digest: {e}") return error( - f"Error building digest: {str(e)}", + "Error building digest", code="DIGEST_ERROR", status_code=500, ) @@ -921,7 +915,7 @@ async def get_cleanup_candidates( except Exception as e: logger.error(f"Error building cleanup candidates: {e}") return error( - f"Error building cleanup candidates: {str(e)}", + "Error building cleanup candidates", code="CLEANUP_CANDIDATES_ERROR", status_code=500, ) @@ -1023,7 +1017,7 @@ async def get_db_stats( except Exception as e: logger.error(f"Error building db stats: {e}") return error( - f"Error building db stats: {str(e)}", + "Error building db stats", code="DB_STATS_ERROR", status_code=500, ) @@ -1081,7 +1075,7 @@ def vacuum_database( except Exception as e: logger.error(f"Error running VACUUM: {e}") return error( - f"Error running VACUUM: {str(e)}", + "Error running VACUUM", code="DB_VACUUM_ERROR", status_code=500, ) @@ -1123,7 +1117,7 @@ async def clear_poster_cache( except Exception as e: logger.error(f"Error clearing poster_cache: {e}") return error( - f"Error clearing poster_cache: {str(e)}", + "Error clearing poster_cache", code="DB_CLEAR_POSTER_CACHE_ERROR", status_code=500, ) @@ -1160,7 +1154,7 @@ async def clear_artwork_matches( except Exception as e: logger.error(f"Error clearing media_asset_matches: {e}") return error( - f"Error clearing media_asset_matches: {str(e)}", + "Error clearing media_asset_matches", code="DB_CLEAR_ARTWORK_MATCHES_ERROR", status_code=500, ) @@ -1202,7 +1196,7 @@ async def reset_poster_matches( except Exception as e: logger.error(f"Error resetting poster match state: {e}") return error( - f"Error resetting poster match state: {str(e)}", + "Error resetting poster match state", code="DB_RESET_POSTER_MATCHES_ERROR", status_code=500, ) @@ -1240,7 +1234,7 @@ async def reset_artwork_matches( except Exception as e: logger.error(f"Error resetting artwork match state: {e}") return error( - f"Error resetting artwork match state: {str(e)}", + "Error resetting artwork match state", code="DB_RESET_ARTWORK_MATCHES_ERROR", status_code=500, ) diff --git a/backend/api/utils.py b/backend/api/utils.py index b9eafbd6..cd59115c 100755 --- a/backend/api/utils.py +++ b/backend/api/utils.py @@ -81,6 +81,17 @@ def error( return JSONResponse(status_code=status_code, content=payload) +def worker_error( + result: Any, logger: Any, message: str, code: str, status_code: int = 500 +) -> Optional[JSONResponse]: + """Stable response for a failed worker envelope (detail to the log), else None.""" + # Worker messages embed raw exception text, so they never reach the client. + if not isinstance(result, dict) or result.get("success"): + return None + logger.error(f"{message}: {result.get('message')}") + return error(message, code=code, status_code=status_code) + + MAX_REQUEST_BODY_BYTES = 1024 * 1024 # Sentinel outcome, distinct from None (unparseable) and {} (no body). diff --git a/backend/api/webhooks.py b/backend/api/webhooks.py index 7d7084b5..bdf4b2f7 100755 --- a/backend/api/webhooks.py +++ b/backend/api/webhooks.py @@ -35,14 +35,9 @@ def verify_webhook_secret(request: Request) -> None: is unset, webhooks are accepted unauthenticated. If it is set, callers must send it via `X-Webhook-Secret` header or `?secret=` query param. """ - try: - cfg = load_config() - except ConfigError as exc: - # These endpoints are AuthMiddleware-exempt and gated ONLY by this - # secret; if config can't load we can't verify it — fail CLOSED. - raise HTTPException( - status_code=503, detail="Configuration unavailable" - ) from exc + # ConfigError propagates to the shared CONFIG_INVALID handler — these + # endpoints are secret-gated only, and an unverifiable secret fails closed. + cfg = load_config() expected = (cfg.general.webhook_secret or "").strip() if not expected: @@ -156,10 +151,9 @@ async def process_poster_webhook( # proxy (general.trusted_proxies) — this recovers the arr's real IP for # instance matching. A forged XFF from an untrusted caller is ignored. peer_host = request.client.host if request.client else None - try: - trusted_proxies = load_config().general.trusted_proxies - except Exception: - trusted_proxies = [] + # Unguarded on purpose: this decides whether XFF is honoured, so an + # indeterminate trust model must reject the webhook, not guess at it. + trusted_proxies = load_config().general.trusted_proxies client_info = { "client_host": resolve_client_host( peer_host, @@ -259,10 +253,12 @@ async def process_poster_webhook( {"job_id": job_id, "status": "enqueued"}, ) + except ConfigError: + raise except Exception as e: logger.error(f"Exception in webhook processing: {e}", exc_info=True) return error( - f"Webhook processing error: {str(e)}", + "Webhook processing error", code="WEBHOOK_PROCESSING_ERROR", status_code=500, ) @@ -395,12 +391,12 @@ def _is_test_event(data: Dict[str, Any]) -> bool: async def get_webhook_wiring( logger: Any = Depends(get_logger), ) -> JSONResponse: + """Return the inbound webhook path and secret for the UI's paste-ready URLs.""" try: logger.debug("Serving GET /api/webhooks/wiring") - try: - secret = (load_config().general.webhook_secret or "").strip() - except ConfigError: - secret = "" + # A malformed config must not render as "no secret configured" — that + # reads to the admin as a working unauthenticated setup. + secret = (load_config().general.webhook_secret or "").strip() resp = ok( "Webhook wiring retrieved", @@ -415,10 +411,12 @@ async def get_webhook_wiring( # Forbid caching so it never lands in browser/proxy caches. resp.headers["Cache-Control"] = "no-store" return resp + except ConfigError: + raise except Exception as e: logger.error(f"Error retrieving webhook wiring: {e}") return error( - f"Error retrieving webhook wiring: {str(e)}", + "Error retrieving webhook wiring", code="WEBHOOK_WIRING_ERROR", status_code=500, ) @@ -458,6 +456,7 @@ async def get_provision_status( request: Request, logger: Any = Depends(get_logger), ) -> JSONResponse: + """Report per-instance CHUB webhook wiring state (never the secret URL).""" try: cfg = load_config() secret = (cfg.general.webhook_secret or "").strip() or None @@ -483,16 +482,12 @@ async def get_provision_status( resp = ok("Webhook provisioning status", payload) resp.headers["Cache-Control"] = "no-store" return resp - except ConfigError as e: - return error( - f"Configuration unavailable: {e}", - code="CONFIG_UNAVAILABLE", - status_code=503, - ) + except ConfigError: + raise # shared CONFIG_INVALID contract in main.py except Exception as e: logger.error(f"Error retrieving provisioning status: {e}", exc_info=True) return error( - f"Error retrieving provisioning status: {str(e)}", + "Error retrieving provisioning status", code="WEBHOOK_PROVISION_STATUS_ERROR", status_code=500, ) @@ -517,6 +512,7 @@ async def provision_webhooks( request: Request, logger: Any = Depends(get_logger), ) -> JSONResponse: + """Create or repair the CHUB notification on each selected ARR instance.""" try: cfg = load_config() body = await request.json() @@ -547,16 +543,12 @@ async def provision_webhooks( ) resp.headers["Cache-Control"] = "no-store" return resp - except ConfigError as e: - return error( - f"Configuration unavailable: {e}", - code="CONFIG_UNAVAILABLE", - status_code=503, - ) + except ConfigError: + raise # shared CONFIG_INVALID contract in main.py except Exception as e: logger.error(f"Error provisioning webhooks: {e}", exc_info=True) return error( - f"Error provisioning webhooks: {str(e)}", + "Error provisioning webhooks", code="WEBHOOK_PROVISION_ERROR", status_code=500, ) @@ -576,6 +568,7 @@ async def remove_webhooks( request: Request, logger: Any = Depends(get_logger), ) -> JSONResponse: + """Delete the CHUB notification from each selected ARR instance.""" try: cfg = load_config() body = await request.json() @@ -592,16 +585,12 @@ async def remove_webhooks( resp = ok("Webhook removal complete", {"instances": results}) resp.headers["Cache-Control"] = "no-store" return resp - except ConfigError as e: - return error( - f"Configuration unavailable: {e}", - code="CONFIG_UNAVAILABLE", - status_code=503, - ) + except ConfigError: + raise # shared CONFIG_INVALID contract in main.py except Exception as e: logger.error(f"Error removing webhooks: {e}", exc_info=True) return error( - f"Error removing webhooks: {str(e)}", + "Error removing webhooks", code="WEBHOOK_REMOVE_ERROR", status_code=500, ) @@ -653,7 +642,7 @@ async def get_unmatched_webhook_status( except Exception as e: logger.error(f"Error retrieving unmatched status: {e}") return error( - f"Error retrieving unmatched status: {str(e)}", + "Error retrieving unmatched status", code="UNMATCHED_STATUS_ERROR", status_code=500, ) @@ -710,7 +699,7 @@ async def process_unmatched_webhook( except Exception as e: logger.error(f"Error processing unmatched webhook: {e}") return error( - f"Error processing unmatched webhook: {str(e)}", + "Error processing unmatched webhook", code="UNMATCHED_PROCESS_ERROR", status_code=500, ) diff --git a/backend/util/config.py b/backend/util/config.py index c0730eef..23fd552b 100755 --- a/backend/util/config.py +++ b/backend/util/config.py @@ -1080,7 +1080,11 @@ def resolve_secret_path(data: Any, path: str) -> str: class ConfigError(Exception): - """Base class for configuration errors.""" + """Base class for configuration errors; `.message` is response-safe text.""" + + def __init__(self, message: str = "") -> None: + super().__init__(message) + self.message = message class ConfigNotFoundError(ConfigError): @@ -1156,28 +1160,31 @@ def clear_config_cache() -> None: _config_cache.clear() +def _humanize_validation_msg(msg: str) -> str: + """Rewrite common Pydantic phrasing for non-developer readers.""" + if "field required" in msg or "Field required" in msg: + return "missing required field" + if "not a valid integer" in msg or "valid integer" in msg: + return "must be a number" + if "not a valid boolean" in msg or "valid boolean" in msg: + return "must be true or false" + if "not a valid string" in msg or "valid string" in msg: + return "must be text" + if "invalid or missing URL scheme" in msg: + return "must be a valid URL (http:// or https://)" + return msg + + def format_validation_errors(validation_error: ValidationError) -> List[str]: - """Return one humanized "loc: msg" line per Pydantic field error. + """Return one humanized "loc: msg (got: value)" line per Pydantic field error. - Used by both the CLI error printer and runtime loggers so the user sees - the same per-field detail regardless of which path surfaced the failure. + Server-side only — the input values make this unsafe to return in a + response. Use ``format_validation_errors_public`` for that. """ lines: List[str] = [] for error in validation_error.errors(): location = " -> ".join(str(loc) for loc in error["loc"]) - msg = error["msg"] - - # Simplify common Pydantic phrasing for non-developer readers - if "field required" in msg or "Field required" in msg: - msg = "missing required field" - elif "not a valid integer" in msg or "valid integer" in msg: - msg = "must be a number" - elif "not a valid boolean" in msg or "valid boolean" in msg: - msg = "must be true or false" - elif "not a valid string" in msg or "valid string" in msg: - msg = "must be text" - elif "invalid or missing URL scheme" in msg: - msg = "must be a valid URL (http:// or https://)" + msg = _humanize_validation_msg(error["msg"]) input_value = error.get("input") if input_value is not None and not isinstance(input_value, (dict, list)): @@ -1187,6 +1194,24 @@ def format_validation_errors(validation_error: ValidationError) -> List[str]: return lines +def format_validation_errors_public(validation_error: ValidationError) -> List[str]: + """Return response-safe "loc: msg" lines — no input values, secrets masked.""" + lines: List[str] = [] + for error in validation_error.errors(): + loc = error["loc"] + location = " -> ".join(str(part) for part in loc) + leaf = str(loc[-1]) if loc else "" + # A sensitive leaf gets a fixed msg too: Pydantic phrasing can quote the + # offending value (enum/pattern errors), which would echo the secret. + msg = ( + "invalid value" + if leaf in SENSITIVE_FIELD_NAMES + else _humanize_validation_msg(error["msg"]) + ) + lines.append(f"{location}: {msg}") + return lines + + def _print_cli_validation_errors(validation_error: ValidationError) -> None: """Print simplified validation errors for CLI users.""" print("❌ Configuration validation failed:") @@ -1258,9 +1283,17 @@ def load_config(path: Optional[str] = None) -> ChubConfig: with open(config_path, "r") as f: raw = yaml.safe_load(f) except yaml.YAMLError as e: - raise ConfigParseError(f"Invalid YAML syntax in {config_path}: {e}") from e + # Position only — the raw text carries parser internals, plus the + # offending source line whenever YAML is parsed from a string. + mark = getattr(e, "problem_mark", None) + where = ( + f" at line {mark.line + 1}, column {mark.column + 1}" + if mark is not None + else "" + ) + raise ConfigParseError(f"Invalid YAML syntax in {config_path}{where}") from e except Exception as e: - raise ConfigParseError(f"Failed to read {config_path}: {e}") from e + raise ConfigParseError(f"Failed to read {config_path}") from e if raw is None: raise ConfigParseError(f"Configuration file is empty: {config_path}") @@ -1278,7 +1311,7 @@ def load_config(path: Optional[str] = None) -> ChubConfig: validation_error=e, ) from e except Exception as e: - raise ConfigError(f"Unexpected configuration error: {e}") from e + raise ConfigError(f"Unexpected configuration error in {config_path}") from e _cache_config(config_path, version, config) return config @@ -1425,7 +1458,7 @@ def save_config(config: ChubConfig, path: Optional[str] = None) -> None: pass # Best-effort cleanup; re-raise original error raise except Exception as e: - raise ConfigError(f"Failed to save configuration: {e}") from e + raise ConfigError("Failed to save configuration") from e def seed_plex_enabled_libraries( diff --git a/backend/util/helper.py b/backend/util/helper.py index ae13b3b3..cdb79265 100755 --- a/backend/util/helper.py +++ b/backend/util/helper.py @@ -2,7 +2,6 @@ import copy import json -import math import os import re from pathlib import Path @@ -24,6 +23,7 @@ tvdb_id_regex, year_regex, ) +from backend.util.logger import create_bar # re-exported: callers import it from here from backend.util.normalization import ( normalize_titles, ) @@ -249,29 +249,6 @@ def create_table(data: List[List[Any]]) -> str: return table -def create_bar(middle_text: str) -> str: - """ - Create a horizontal separator bar with centered text (80 chars total). - - Args: - middle_text: Text to center in the bar - - Returns: - Formatted horizontal bar string with centered text - """ - total_length = 80 - if len(middle_text) == 1: - remaining_length = total_length - len(middle_text) - 2 - left_side_length = 0 - right_side_length = remaining_length - return f"\n{middle_text * left_side_length}{middle_text}{middle_text * right_side_length}\n" - - remaining_length = total_length - len(middle_text) - 4 - left_side_length = math.floor(remaining_length / 2) - right_side_length = remaining_length - left_side_length - return f"\n{'*' * left_side_length} {middle_text} {'*' * right_side_length}\n" - - def progress( iterable: Any, desc: Optional[str] = None, diff --git a/backend/util/logger.py b/backend/util/logger.py index b1ff566a..7f92c858 100755 --- a/backend/util/logger.py +++ b/backend/util/logger.py @@ -1,4 +1,5 @@ import logging +import math import os import re from datetime import datetime @@ -6,13 +7,27 @@ from pathlib import Path from typing import Optional -from backend.util.helper import create_bar from backend.util.version import get_version # Size cap per log file. Set LOG_MAX_BYTES=0 to disable the cap. DEFAULT_MAX_LOG_BYTES = 10 * 1024 * 1024 +def create_bar(middle_text: str) -> str: + """Build an 80-char separator bar with `middle_text` centered.""" + total_length = 80 + if len(middle_text) == 1: + remaining_length = total_length - len(middle_text) - 2 + left_side_length = 0 + right_side_length = remaining_length + return f"\n{middle_text * left_side_length}{middle_text}{middle_text * right_side_length}\n" + + remaining_length = total_length - len(middle_text) - 4 + left_side_length = math.floor(remaining_length / 2) + right_side_length = remaining_length - left_side_length + return f"\n{'*' * left_side_length} {middle_text} {'*' * right_side_length}\n" + + def _max_log_bytes() -> int: # A negative value must NOT collapse to 0 — 0 is the documented "no cap" # opt-out, so that would silently restore unbounded growth. @@ -393,8 +408,6 @@ def log_outro(self) -> None: formatted_duration = f"{int(hours)}h {int(minutes)}m {int(seconds)}s" module_name = underlying.name.replace("_", " ").upper() - from backend.util.helper import create_bar # noqa: E402 — lazy import to break circular dependency - underlying.info(create_bar(f"{module_name} | Run Time: {formatted_duration}")) def get_adapter(self, extra=None): diff --git a/tests/test_api_config_errors.py b/tests/test_api_config_errors.py index aea6ff49..5f0e3582 100644 --- a/tests/test_api_config_errors.py +++ b/tests/test_api_config_errors.py @@ -93,6 +93,37 @@ def test_parse_error_returns_clear_envelope(): assert any("line 309" in str(m) for m in logger.errors) +def test_parse_error_omits_offending_line_content(tmp_path): + """Position reaches the operator; the broken line's content never does.""" + from backend.util.config import load_config + + bad = tmp_path / "config.yml" + # Unterminated quoted value — PyYAML's error text quotes this line verbatim, + # so a real config would leak whatever secret sat on the broken line. + bad.write_text('general:\n log_level: info\nSECRET_MARKER: "unterminated\n') + + app = FastAPI() + app.state.logger = StubLogger() + app.add_exception_handler(ConfigError, handle_config_error) + + @app.get("/load") + def _load(): + load_config(path=str(bad)) + + client = TestClient(app, raise_server_exceptions=False) + resp = client.get("/load") + + assert resp.status_code == 500 + body = resp.json() + assert body["error_code"] == "CONFIG_INVALID" + assert "while scanning" not in resp.text # no parser text + # PyYAML embeds the source snippet only for string input, but a future + # read-then-parse refactor would leak the line — guard it now. + assert "SECRET_MARKER" not in resp.text + # ...position is still surfaced so the file stays fixable. + assert "at line" in body["message"] + + def test_validation_error_includes_per_field_detail(): client, logger = _client() resp = client.get("/validate") diff --git a/tests/test_api_smoke.py b/tests/test_api_smoke.py index 3e9b553e..b9def3b9 100644 --- a/tests/test_api_smoke.py +++ b/tests/test_api_smoke.py @@ -21,6 +21,7 @@ from backend.api import auth as auth_router # noqa: E402 from backend.api import border_replacerr as border_router # noqa: E402 +from backend.api import posters as posters_router # noqa: E402 from backend.api import system as system_router # noqa: E402 from backend.util.config import ChubConfig # noqa: E402 @@ -348,16 +349,241 @@ def test_allowed_roots_returns_configured_paths(monkeypatch, app_with_router, tm assert str(posters.resolve()) in roots -def test_allowed_roots_returns_empty_when_no_config(monkeypatch, app_with_router): - """Pre-onboarding (no config yet) the endpoint returns [] without crashing.""" +def test_allowed_roots_ok_when_config_file_absent(monkeypatch, app_with_router, tmp_path): + """Real pre-onboarding: a missing file loads defaults, so the picker still answers.""" + monkeypatch.setenv("CONFIG_DIR", str(tmp_path)) + app = app_with_router(system_router.router) + client = TestClient(app) + resp = client.get("/api/allowed-roots") + assert resp.status_code == 200 + assert isinstance(resp.json()["data"]["roots"], list) + + +def test_allowed_roots_propagates_config_error(monkeypatch, app_with_router): + """A malformed config must not masquerade as an empty allow-list.""" from backend.util.config import ConfigError def _raise(*_a, **_kw): - raise ConfigError("no config") + """Stand in for load_config on a malformed config file.""" + raise ConfigError("corrupt config") + + from backend.api.main import handle_config_error monkeypatch.setattr(system_router, "load_config", _raise) app = app_with_router(system_router.router) - client = TestClient(app) + app.add_exception_handler(ConfigError, handle_config_error) + client = TestClient(app, raise_server_exceptions=False) resp = client.get("/api/allowed-roots") + assert resp.status_code == 500 + assert resp.json()["error_code"] == "CONFIG_INVALID" + + +def test_delete_poster_aborts_before_row_delete_on_bad_config( + monkeypatch, app_with_router, tmp_path +): + """deleteFile + malformed config must abort BEFORE the irreversible row delete.""" + from backend.api.main import handle_config_error + from backend.util.config import ConfigError, clear_config_cache + + (tmp_path / "config.yml").mkdir() # OSError on open -> ConfigParseError + monkeypatch.setenv("CONFIG_DIR", str(tmp_path)) + clear_config_cache() + + deleted = [] + + class _Poster: + """Tripwire recording any row deletion the handler attempts.""" + + def delete_by_integer_id(self, poster_id): + """Record the destructive call so the test can assert it never ran.""" + deleted.append(poster_id) + return {"file": "x.jpg", "folder": "/tmp", "normalized_title": None} + + class _DB: + """Minimal db stub exposing only the poster repository.""" + + poster = _Poster() + + app = app_with_router(posters_router.router) + app.state.db = _DB() + app.add_exception_handler(ConfigError, handle_config_error) + client = TestClient(app, raise_server_exceptions=False) + resp = client.request("DELETE", "/api/posters/1", json={"deleteFile": True}) + + assert resp.status_code == 500 + assert resp.json()["error_code"] == "CONFIG_INVALID" + assert deleted == [] # the row survives a broken config + clear_config_cache() + + +def test_delete_poster_refuses_file_outside_allowed_roots( + monkeypatch, app_with_router, tmp_path +): + """A poisoned folder/file row must not delete outside the configured roots.""" + monkeypatch.setenv("CONFIG_DIR", str(tmp_path / "cfg")) + victim_dir = tmp_path / "elsewhere" + victim_dir.mkdir() + victim = victim_dir / "victim.txt" + victim.write_text("do not delete") + + class _Poster: + """Stub returning a row that points outside every configured root.""" + + def delete_by_integer_id(self, _pid): + """Return the poisoned row and report it as deleted.""" + return { + "file": victim.name, + "folder": str(victim_dir), + "normalized_title": None, + } + + class _DB: + """Minimal db stub exposing only the poster repository.""" + + poster = _Poster() + + app = app_with_router(posters_router.router) + app.state.db = _DB() + client = TestClient(app) + resp = client.request("DELETE", "/api/posters/1", json={"deleteFile": True}) + assert resp.status_code == 200 - assert resp.json()["data"]["roots"] == [] + # The row is still cleaned up, but the out-of-root file survives. + assert resp.json()["data"]["file_deleted"] is False + assert victim.exists() + + +# --- Inbound webhook verification under a broken config --- + + +def test_webhook_rejected_when_config_malformed(monkeypatch, app_with_router): + """A malformed config must reject the webhook, never wave it through.""" + from backend.api import webhooks as webhooks_router + from backend.util.config import ConfigError + + def _raise(*_a, **_kw): + """Stand in for load_config on a malformed config file.""" + raise ConfigError("corrupt config") + + class _TripwireDB: + """Records any attribute touch — a touch means the body got processed.""" + + def __init__(self): + """Start with no touches recorded.""" + self.touched = [] + + def __getattr__(self, name): + """Record the access and honor the special-method contract.""" + self.touched.append(name) + raise AttributeError(name) + + monkeypatch.setattr(webhooks_router, "load_config", _raise) + app = app_with_router(webhooks_router.router) + db = _TripwireDB() + app.state.db = db + from backend.api.main import handle_config_error + from backend.util.config import ConfigError as _CfgErr + + app.add_exception_handler(_CfgErr, handle_config_error) + client = TestClient(app, raise_server_exceptions=False) + resp = client.post("/api/webhooks/poster/add", json={"eventType": "Download"}) + + # The shared contract: ConfigError propagates from the secret dependency. + assert resp.status_code == 500 + assert resp.json()["error_code"] == "CONFIG_INVALID" + assert db.touched == [], f"webhook body was processed: db.{db.touched}" + + +def test_webhook_config_read_failure_rejects_without_enqueue( + monkeypatch, app_with_router, tmp_path +): + """An unreadable config (OSError) rejects the webhook and enqueues nothing.""" + from backend.api import webhooks as webhooks_router + from backend.api.main import handle_config_error + from backend.util.config import ConfigError, clear_config_cache + + # A directory where config.yml belongs -> open() raises IsADirectoryError, + # which load_config wraps into ConfigParseError (a ConfigError). + (tmp_path / "config.yml").mkdir() + monkeypatch.setenv("CONFIG_DIR", str(tmp_path)) + clear_config_cache() + + enqueued = [] + + class _Worker: + """Tripwire: records any enqueue attempt instead of doing one.""" + + def enqueue_job(self, *a, **k): + """Record the call so the test can assert it never happened.""" + enqueued.append(a) + return {"success": True, "data": {"job_id": 1}} + + class _DB: + """Minimal db stub exposing only the job worker.""" + + worker = _Worker() + + app = app_with_router(webhooks_router.router) + app.state.db = _DB() + app.add_exception_handler(ConfigError, handle_config_error) + client = TestClient(app, raise_server_exceptions=False) + resp = client.post("/api/webhooks/poster/add", json={"eventType": "Download"}) + + # The secret dependency propagates ConfigError to the shared handler + # before the body runs — CONFIG_INVALID, and nothing was enqueued. + assert resp.status_code == 500 + assert resp.json()["error_code"] == "CONFIG_INVALID" + assert enqueued == [] + clear_config_cache() + + +def test_webhook_signed_path_still_accepted(monkeypatch): + """A valid config still accepts the right secret and rejects a wrong one.""" + from types import SimpleNamespace + + from fastapi import HTTPException + + from backend.api import webhooks as webhooks_router + + cfg = ChubConfig() + cfg.general.webhook_secret = "s3cret" + monkeypatch.setattr(webhooks_router, "load_config", lambda *a, **k: cfg) + + good = SimpleNamespace(headers={"X-Webhook-Secret": "s3cret"}, query_params={}) + assert webhooks_router.verify_webhook_secret(good) is None + + bad = SimpleNamespace(headers={"X-Webhook-Secret": "nope"}, query_params={}) + with pytest.raises(HTTPException) as excinfo: + webhooks_router.verify_webhook_secret(bad) + assert excinfo.value.status_code == 401 + + +# --- Error-response hygiene --- + + +def test_error_body_omits_exception_text(app_with_router): + """A raising dependency yields the stable message — never the exception text.""" + + class _BoomStats: + """Stats interface that raises like a real internal failure.""" + + def get_matched_posters_stats(self, *a, **kw): + """Simulate an arbitrary internal error carrying sensitive text.""" + raise RuntimeError("LEAK_MARKER /srv/secret/chub.db") + + class _Boom: + """Stub db exposing the raising stats interface the route reads.""" + + stats = _BoomStats() + + app = app_with_router(posters_router.router) + app.state.db = _Boom() + client = TestClient(app) + resp = client.get("/api/posters/stats") + + assert resp.status_code == 500 + body = resp.json() + assert body["error_code"] == "POSTER_STATS_ERROR" + assert body["message"] == "Error retrieving poster statistics" + # The whole body, not just message — detail must not reach data/ either. + assert "LEAK_MARKER" not in resp.text diff --git a/tests/test_regression_review_2026.py b/tests/test_regression_review_2026.py index 4ba7c28e..5785f3e7 100644 --- a/tests/test_regression_review_2026.py +++ b/tests/test_regression_review_2026.py @@ -367,10 +367,8 @@ def health(): # 13. verify_webhook_secret must fail CLOSED on config error — the exempt webhook -# endpoints rely solely on it. +# endpoints rely solely on it; ConfigError propagates to the shared handler. def test_verify_webhook_secret_fails_closed_on_config_error(monkeypatch): - from fastapi import HTTPException - from backend.api.webhooks import verify_webhook_secret from backend.util.config import ConfigError @@ -383,9 +381,8 @@ class _Req: headers = {} query_params = {} - with pytest.raises(HTTPException) as ei: + with pytest.raises(ConfigError): verify_webhook_secret(_Req()) - assert ei.value.status_code == 503 # 14. GET /api/modules/{name} must redact secrets in the returned config section.