DRIVERS-3568 Define PSL support in the Initial DNS Seedlist Discovery Specification - #1972
DRIVERS-3568 Define PSL support in the Initial DNS Seedlist Discovery Specification#1972sleepyStick wants to merge 9 commits into
Conversation
| timeout-minutes: 5 | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
Semgrep identified an issue in your code:
actions/checkout@v4 uses a movable tag, so a repointed v4 release would run attacker-controlled code in this write-enabled workflow.
More details about this
actions/checkout@v4 pulls a GitHub Action by a movable tag, not a fixed commit. If the v4 tag is ever repointed, this scheduled workflow would run the new code automatically before python3 source/public-suffix-list/etc/sync-psl.py, with contents: write and pull-requests: write permissions.
A plausible attack looks like this:
- An attacker compromises the
actions/checkoutrelease process or gains control of the account that can move thev4tag. - They repoint
v4to a malicious commit while leaving the action name unchanged, so this step still saysuses: actions/checkout@v4. - On the next monthly run or any manual
workflow_dispatch, GitHub resolvesv4to the attacker's code and executes it in thesyncjob. - That code runs with this workflow's token permissions and workspace access, so it can modify the checked-out repository, read
$GITHUB_OUTPUT, or usegit push origin "$branch"behavior to push attacker-controlled changes. - It can then abuse the later PR flow to open a trusted-looking pull request from the automation branch, making the malicious change appear to come from your normal PSL sync job.
To resolve this comment:
✨ Commit fix suggestion
- Replace the mutable action reference
actions/checkout@v4with a full 40-character commit SHA for the same trusted release, for exampleuses: actions/checkout@<full-commit-sha>. - Keep the version in a comment if it helps with maintenance, such as
# actions/checkout v4, but do not use the tag inuses:. - Choose the SHA from the official
actions/checkoutrepository for thev4release you intend to keep using. Pinning to a commit SHA prevents the action owner from silently changing what runs in this workflow.
Alternatively, if you need an easier update path, use Dependabot or Renovate to keep pinned GitHub Action SHAs updated automatically while still keeping uses: pinned to a full commit SHA.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by github-actions-mutable-action-tag.
🛟 Help? Slack #semgrep-help or go/semgrep-help.
Resolution Options:
- Fix the code
- Reply
/fp $reason(if security gap doesn’t exist) - Reply
/ar $reason(if gap is valid but intentional; add mitigations/monitoring) - Reply
/other $reason(e.g., test-only)
You can view more details about this finding in the Semgrep AppSec Platform.
| # Public Suffix List tests | ||
|
|
||
| These tests verify that drivers parse [public_suffix_list.dat](../public_suffix_list.dat) correctly by exercising the | ||
| `srvAllowedHostsSuffix` connection string option, which MUST NOT accept a value that is itself a public suffix. |
There was a problem hiding this comment.
srvAllowedHostsSuffix introduced in #1950
Please complete the following before merging:
python pr: https://github.com/mongodb/mongo-python-driver/pull/2868/changes (contain both drivers-3568 and drivers-3329 changes)
clusters).