Skip to content

Conversation

@MuneebUllahKhan222
Copy link

🛠 Summary of Fixes

  • Resolved timeout issue:
    The verification logic was unintentionally using the default timeout because the detector context's remaining time wasn’t being forwarded.
    Updated the implementation to pass the remaining context deadline into verifyFTP, ensuring correct timeout behavior.

  • Fixed failing tests:
    Tests began failing due to the introduction of the unexported primarySecret field in detectors.Result.
    Updated the test comparison to ignore this unexported field, restoring expected test behavior.

@MuneebUllahKhan222 MuneebUllahKhan222 requested a review from a team December 8, 2025 11:09
@MuneebUllahKhan222 MuneebUllahKhan222 requested a review from a team as a code owner December 8, 2025 11:09
@CLAassistant
Copy link

CLAassistant commented Dec 8, 2025

CLA assistant check
All committers have signed the CLA.

Comment on lines +83 to +89
if dl, ok := ctx.Deadline(); ok {
// Here we assign the remaining time before our context expires
// This help us cater the timelimit set through --detector-timeout flag
timeout = time.Until(dl)
} else {
timeout = defaultVerificationTimeout
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks fine. This is what I suggested here, but as per @rgmz comment, ftp.Dial can still hang for a longer time. Could you test it with this change?

@kashifkhan0771 kashifkhan0771 linked an issue Dec 9, 2025 that may be closed by this pull request
@kashifkhan0771
Copy link
Contributor

kashifkhan0771 commented Dec 9, 2025

pkg/detectors/ftp/ftp.go:134:14: Error return value of c.Quit is not checked (errcheck)

@kashifkhan0771
Copy link
Contributor

@trufflesecurity/product-eng please review

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.

FTP detector ignores context timeout

3 participants