-
Notifications
You must be signed in to change notification settings - Fork 247
DRIVERS-3568 Define PSL support in the Initial DNS Seedlist Discovery Specification #1972
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
sleepyStick
wants to merge
9
commits into
mongodb:master
Choose a base branch
from
sleepyStick:DRIVERS-3568
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
64f96ba
add PSL
sleepyStick 2d22775
fix status
sleepyStick ce7d02e
fix linting
sleepyStick bc2bc4b
move tests into intial-dns-seedlist-discovery
sleepyStick f4a4d19
okay so some of those unified tests had to be prose tests,,
sleepyStick b741042
condense tests
sleepyStick 8d448d0
fix linting
sleepyStick 31e048a
fix lint
sleepyStick 997cdc9
reword prose tests
sleepyStick File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| name: Sync Public Suffix List | ||
|
|
||
| on: | ||
| schedule: | ||
| # 12:00 UTC on the first day of each month. | ||
| - cron: "0 12 1 * *" | ||
| workflow_dispatch: | ||
|
|
||
| permissions: | ||
| contents: write | ||
| pull-requests: write | ||
|
|
||
| env: | ||
| PSL: source/public-suffix-list/public_suffix_list.dat | ||
|
|
||
| jobs: | ||
| sync: | ||
| name: Sync PSL and open a PR | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 5 | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Sync the Public Suffix List | ||
| run: python3 source/public-suffix-list/etc/sync-psl.py | ||
| - name: Check whether the list changed | ||
| id: changed | ||
| run: | | ||
| if git diff --quiet -- "$PSL"; then | ||
| echo "The list is unchanged; nothing to do." | ||
| echo "changed=false" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "changed=true" >> "$GITHUB_OUTPUT" | ||
| fi | ||
| - name: Commit and push | ||
| id: push | ||
| if: steps.changed.outputs.changed == 'true' | ||
| run: | | ||
| git config user.name "github-actions[bot]" | ||
| git config user.email "41898282+github-actions[bot]@users.noreply.github.com" | ||
| # Each run gets its own branch, so nothing is ever force-pushed over. A run that | ||
| # collides with an earlier branch from the same day is rejected rather than | ||
| # overwriting it. | ||
| today=$(date -u +%Y-%m-%d) | ||
| branch="sync-psl-$today" | ||
| git switch -c "$branch" | ||
| git add -- "$PSL" | ||
| git commit -m "[$today] Sync the Public Suffix List" | ||
| git push origin "$branch" | ||
| echo "branch=$branch" >> "$GITHUB_OUTPUT" | ||
| echo "today=$today" >> "$GITHUB_OUTPUT" | ||
| - name: Open a pull request | ||
| if: steps.changed.outputs.changed == 'true' | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| BRANCH: ${{ steps.push.outputs.branch }} | ||
| TODAY: ${{ steps.push.outputs.today }} | ||
| run: | | ||
| body=$(cat <<'EOF' | ||
| The upstream [Public Suffix List](https://publicsuffix.org/list/) has changed. | ||
|
|
||
| This pull request was opened automatically by the `sync-psl` workflow, which regenerates | ||
| `source/public-suffix-list/public_suffix_list.dat` via `source/public-suffix-list/etc/sync-psl.py`. | ||
|
|
||
| Please review the diff before merging. If an earlier sync pull request is still open, | ||
| merge or close this one and that one together -- they change the same file. | ||
| EOF | ||
| ) | ||
| gh pr create \ | ||
| --base master \ | ||
| --head "$BRANCH" \ | ||
| --title "[$TODAY] Sync the Public Suffix List" \ | ||
| --body "$body" | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
17 changes: 17 additions & 0 deletions
17
...dns-seedlist-discovery/tests/replica-set/srvAllowedHostsSuffix-psl-not-public-suffix.json
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
15 changes: 15 additions & 0 deletions
15
...-dns-seedlist-discovery/tests/replica-set/srvAllowedHostsSuffix-psl-not-public-suffix.yml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| # "10gen.cc" is not in the Public Suffix List -- only its parent "cc" is -- so it is | ||
| # not a public suffix and must be accepted. The SRV hosts end with it, so resolution | ||
| # succeeds. | ||
| uri: "mongodb+srv://test1.test.build.10gen.cc/?srvAllowedHostsSuffix=10gen.cc" | ||
| seeds: | ||
| - localhost.test.build.10gen.cc:27017 | ||
| - localhost.test.build.10gen.cc:27018 | ||
| hosts: | ||
| - localhost:27017 | ||
| - localhost:27018 | ||
| - localhost:27019 | ||
| options: | ||
| ssl: true | ||
| srvAllowedHostsSuffix: "10gen.cc" | ||
| ping: true |
6 changes: 6 additions & 0 deletions
6
...ial-dns-seedlist-discovery/tests/replica-set/srvAllowedHostsSuffix-psl-public-suffix.json
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
7 changes: 7 additions & 0 deletions
7
...tial-dns-seedlist-discovery/tests/replica-set/srvAllowedHostsSuffix-psl-public-suffix.yml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| # "cc" is an ordinary rule in the Public Suffix List, so it is itself a public suffix | ||
| # and must be rejected. The SRV hosts do end with "cc", so the host suffix check | ||
| # passes and the public suffix check is the only thing that can fail here. | ||
| uri: "mongodb+srv://test1.test.build.10gen.cc/?srvAllowedHostsSuffix=cc" | ||
| seeds: [] | ||
| hosts: [] | ||
| error: true |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| """Sync the Public Suffix List from publicsuffix.org into this specification. | ||
|
|
||
| Usage: | ||
|
|
||
| python source/public-suffix-list/etc/sync-psl.py [--check] | ||
|
|
||
| Downloads the upstream list, strips comment and blank lines, and writes the result | ||
| to source/public-suffix-list/public_suffix_list.dat. With --check, does not write | ||
| anything and exits non-zero if the committed file is out of date. | ||
| """ | ||
|
|
||
| import argparse | ||
| import sys | ||
| import urllib.request | ||
| from pathlib import Path | ||
|
|
||
| PSL_URL = "https://publicsuffix.org/list/public_suffix_list.dat" | ||
|
|
||
| # source/public-suffix-list/etc/sync-psl.py -> source/public-suffix-list | ||
| SPEC_DIR = Path(__file__).resolve().parent.parent | ||
| DEST = SPEC_DIR / "public_suffix_list.dat" | ||
|
|
||
|
|
||
| def fetch(): | ||
| request = urllib.request.Request(PSL_URL, headers={"User-Agent": "mongodb-specifications-sync-psl"}) | ||
| with urllib.request.urlopen(request) as response: | ||
| data = response.read() | ||
|
|
||
| text = data.decode("utf-8") | ||
|
|
||
| # Sanity check: the upstream file always carries these section markers. | ||
| for marker in ("// ===END ICANN DOMAINS===", "// ===END PRIVATE DOMAINS==="): | ||
| if marker not in text: | ||
| sys.exit(f"Downloaded file is missing expected markers {marker!r}; refusing to write.") | ||
|
|
||
| return text | ||
|
|
||
|
|
||
| def preprocess(text): | ||
| """Reduce the upstream list to one rule per line. | ||
|
|
||
| Comment lines (those beginning with "//") and blank lines are both removed, so | ||
| every line will be a rule. | ||
| """ | ||
| rules = [] | ||
| for line in text.splitlines(): | ||
| # Upstream rules are not indented, but strip anyway so a stray trailing \r or | ||
| # space does not end up inside a rule. | ||
| line = line.strip() | ||
| if not line or line.startswith("//"): | ||
| continue | ||
| rules.append(line) | ||
|
|
||
| if not rules: | ||
| sys.exit("No rules found after stripping comments; refusing to write.") | ||
|
|
||
| # End the file with exactly one newline. | ||
| return "\n".join(rules) + "\n" | ||
|
|
||
|
|
||
| def main(): | ||
| parser = argparse.ArgumentParser(description=__doc__) | ||
| parser.add_argument( | ||
| "--check", | ||
| action="store_true", | ||
| help="exit non-zero if the committed list differs from upstream, without writing", | ||
| ) | ||
| args = parser.parse_args() | ||
|
|
||
| new_text = preprocess(fetch()) | ||
|
|
||
| old_text = DEST.read_text(encoding="utf-8") if DEST.exists() else None | ||
|
|
||
| if args.check: | ||
| if old_text is None: | ||
| sys.exit(f"{DEST} does not exist; run this script without --check.") | ||
| if old_text != new_text: | ||
| sys.exit(f"{DEST} is out of date; run source/public-suffix-list/etc/sync-psl.py.") | ||
| print(f"{DEST.name} is up to date.") | ||
| return | ||
|
|
||
| if old_text == new_text: | ||
| print(f"{DEST.name} is already up to date ({len(new_text.splitlines())} lines).") | ||
| return | ||
|
|
||
| DEST.write_text(new_text, encoding="utf-8") | ||
| print(f"Wrote {DEST} ({len(new_text.splitlines())} lines).") | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Semgrep identified an issue in your code:
actions/checkout@v4uses a movable tag, so a repointedv4release would run attacker-controlled code in this write-enabled workflow.More details about this
actions/checkout@v4pulls a GitHub Action by a movable tag, not a fixed commit. If thev4tag is ever repointed, this scheduled workflow would run the new code automatically beforepython3 source/public-suffix-list/etc/sync-psl.py, withcontents: writeandpull-requests: writepermissions.A plausible attack looks like this:
actions/checkoutrelease process or gains control of the account that can move thev4tag.v4to a malicious commit while leaving the action name unchanged, so this step still saysuses: actions/checkout@v4.workflow_dispatch, GitHub resolvesv4to the attacker's code and executes it in thesyncjob.$GITHUB_OUTPUT, or usegit push origin "$branch"behavior to push attacker-controlled changes.To resolve this comment:
✨ Commit fix suggestion
actions/checkout@v4with a full 40-character commit SHA for the same trusted release, for exampleuses: actions/checkout@<full-commit-sha>.# actions/checkout v4, but do not use the tag inuses:.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 reasonsAlternatively, 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:
/fp $reason(if security gap doesn’t exist)/ar $reason(if gap is valid but intentional; add mitigations/monitoring)/other $reason(e.g., test-only)You can view more details about this finding in the Semgrep AppSec Platform.