Skip to content

Fix all failing CI checks on main#126

Open
amrit110 wants to merge 1 commit intoBuildCanada:mainfrom
amrit110:fix/ci-failures
Open

Fix all failing CI checks on main#126
amrit110 wants to merge 1 commit intoBuildCanada:mainfrom
amrit110:fix/ci-failures

Conversation

@amrit110
Copy link
Copy Markdown

@amrit110 amrit110 commented Apr 6, 2026

Summary

CI has been failing on every push to main since March 24. This PR fixes all three root causes across the scan_ruby and test jobs.

Brakeman (scan_ruby job)

  • XSS warning in scraping_health viewentry.url was used directly as a link_to href, which Brakeman flags as a potential javascript: URL injection. Fixed by validating the URL starts with http(s):// before using it as the href, falling back to "#".

Test failures (test job)

  • StatcanDatasetsController (2 failures) — The controller looked up datasets by name (find_by(name: params[:id])), but the Rails URL helper generates paths using the integer primary key by default. Added to_param to StatcanDataset to use name as the URL segment (consistent with the controller's lookup strategy). Also added a proper 404 response when the dataset is not found — previously it rendered null with a 200.

  • CommitmentRelevanceFilterJob test — Test created a Commitment with status: :abandoned, but that status was removed from the enum (current terminal status is :broken). Updated the test to use :broken, which is what the job actually filters out.

  • StatcanDataset#needs_sync? / StatcanCronJob tests (timezone bug) — Tests used Time.parse("...") which produces local-timezone Time objects, but Fugit::Cron#previous_time evaluates schedules in UTC (Rails' default timezone). This caused the tests to pass in CI (UTC servers) but fail locally in non-UTC timezones. Fixed by using Time.utc(...) in the affected tests, making them timezone-independent.

Test plan

  • All 84 tests pass: bin/rails db:test:prepare test
  • Brakeman clean: bin/brakeman --no-pager
  • Rubocop clean: bin/rubocop -f github

Three issues were causing CI failures:

**Brakeman (scan_ruby):**
- Sanitize `entry.url` in link_to href in scraping_health view to reject
  non-http(s) URLs, fixing a Weak XSS warning (LinkToHref check)

**Test failures:**
- StatcanDatasetsController#show used find_by(:name) but the URL helper
  generated paths using the integer primary key; added `to_param` to
  StatcanDataset to use `name` as the URL param, and added a proper 404
  response when the dataset is not found
- CommitmentRelevanceFilterJob test used `:abandoned` status which was
  removed from the Commitment enum; updated to use `:broken` (the current
  terminal status that the job filters out)
- StatcanDataset#needs_sync? and related tests used Time.parse which
  produces local-timezone times, but Fugit evaluates cron schedules in
  UTC (Rails default); tests now use Time.utc to be timezone-independent,
  fixing failures on non-UTC machines
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