Skip to content

fix: gate SSL verification bypass on env flag in search_service.py - #97

Open
isla-brooks wants to merge 1 commit into
OpenNSWM-Lab:mainfrom
isla-brooks:fix/search-service-insecure-ssl
Open

fix: gate SSL verification bypass on env flag in search_service.py#97
isla-brooks wants to merge 1 commit into
OpenNSWM-Lab:mainfrom
isla-brooks:fix/search-service-insecure-ssl

Conversation

@isla-brooks

Copy link
Copy Markdown

Hi there! First time contributing here, so please let me know if I missed anything.

I noticed that backend/app/services/search_service.py silently downgrades SSL certificate verification for ArXiv and DBLP searches when the system lacks proper CA certificates. This is a security concern because it enables man-in-the-middle attacks without giving the operator any control over the behavior.

The issue:

  • ArXiv fallback (line 427): Uses FAROS_ALLOW_INSECURE_ARXIV_SSL env var, but defaults to True (insecure by default)
  • DBLP fallback (line 1091): No env var gating at all, always retries with unverified SSL on cert failure

What I changed:

  1. Renamed the ArXiv env var to FAROS_ALLOW_INSECURE_SSL (unified name for both services) and changed the default from True to False
  2. Added the same FAROS_ALLOW_INSECURE_SSL gating to the DBLP fallback
  3. Upgraded the DBLP log message from info to warning to make it more visible

Behavior after this PR:

  • By default (no env var set): SSL cert verification failures raise the original error, no silent downgrade
  • With FAROS_ALLOW_INSECURE_SSL=true: Logs a warning and retries with unverified context (useful for dev environments with broken CA bundles)

Testing: Verified the diff manually. The _env_bool helper already exists and handles the parsing correctly. The change is backwards compatible in the sense that operators who need the insecure fallback can opt in via the env var.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant