Skip to content

fix(detectors): report Todoist verifier errors#5003

Open
mangod12 wants to merge 1 commit into
trufflesecurity:mainfrom
mangod12:fix/todoist-verifier-errors
Open

fix(detectors): report Todoist verifier errors#5003
mangod12 wants to merge 1 commit into
trufflesecurity:mainfrom
mangod12:fix/todoist-verifier-errors

Conversation

@mangod12
Copy link
Copy Markdown

@mangod12 mangod12 commented Jun 1, 2026

Description:

Related to #4051. This updates the Todoist detector verifier to report indeterminate verification failures instead of silently ignoring them.

Before this change, Todoist verification only set Verified for 2xx responses. Network/client errors and unexpected API statuses were dropped, which made verifier failures indistinguishable from determinately invalid credentials.

This PR keeps the detector behavior narrow:

  • 2xx responses verify the secret.
  • 401 and 403 remain determinately unverified without a verification error.
  • client/request failures and unexpected statuses now populate VerificationError.
  • response bodies are drained and closed on verifier responses.

This intentionally does not use a closing keyword for #4051 because that issue tracks many detector verifier cleanups.

Local validation:

  • go test -count=1 ./pkg/detectors/todoist
  • go test -count=1 ./pkg/detectors
  • go test -timeout=5m ./pkg/detectors/...
  • go vet ./pkg/detectors/todoist
  • go run ./hack/checksecretparts -fail ./pkg/detectors
  • golangci-lint run --enable bodyclose,copyloopvar,misspell --timeout 10m ./pkg/detectors/todoist

Checklist:

  • Tests passing (make test-community equivalent is covered by GitHub Actions for fork PRs; local Docker full make test-community did not complete within 20m, so I ran the detector subtree plus changed package directly.)
  • Lint passing (golangci-lint passed for the changed package. Full local lint reported 0 issues but exited on timeout under Docker/Windows; GitHub Actions will run full lint on Linux.)

Note

Low Risk
Scoped to the Todoist detector verifier and tests; behavior change is clearer error reporting, not auth or data handling.

Overview
Refactors the Todoist detector verifier so indeterminate failures are visible instead of being dropped.

Verification moves into verifyMatch, with an injectable Scanner.client (replacing a package-level HTTP client). 2xx still marks secrets verified; 401/403 stay unverified with no error. Network/request failures and unexpected status codes now set VerificationError via SetVerificationError. Response bodies are drained and closed on verifier responses.

Tests inject the client on Scanner and add TestTodoist_VerificationResponses for OK, auth failures, client errors, and unexpected HTTP statuses.

Reviewed by Cursor Bugbot for commit 6085d0b. Bugbot is set up for automated code reviews on this repo. Configure here.

@mangod12 mangod12 requested review from a team and Copilot June 1, 2026 20:10
@mangod12 mangod12 requested a review from a team as a code owner June 1, 2026 20:10
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR refactors Todoist token verification into a dedicated helper and expands tests to cover additional verification response scenarios.

Changes:

  • Extracted HTTP verification logic into verifyMatch and began recording verification errors on results.
  • Improved HTTP response cleanup by draining and closing the response body.
  • Added table-driven tests to validate verification behavior across several HTTP statuses and client errors.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
pkg/detectors/todoist/todoist.go Extracts verification into verifyMatch, adds status handling + response body draining/closing, and plumbs verification errors back to results.
pkg/detectors/todoist/todoist_test.go Adds table-driven tests for verified/unverified/error outcomes across multiple HTTP response conditions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/detectors/todoist/todoist.go
Comment thread pkg/detectors/todoist/todoist_test.go Outdated
@mangod12 mangod12 force-pushed the fix/todoist-verifier-errors branch from 0f9db66 to 6085d0b Compare June 1, 2026 20:37
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.

2 participants