DRIVERS-3568 Define PSL support in the Initial DNS Seedlist Discovery Specification by sleepyStick · Pull Request #1972 · mongodb/specifications

sleepyStick · 2026-08-11T23:35:24Z

Please complete the following before merging:

Is the relevant DRIVERS ticket in the PR title?

Update changelog.
Test changes in at least one language driver.
python pr: https://github.com/mongodb/mongo-python-driver/pull/2868/changes (contain both drivers-3568 and drivers-3329 changes)
Test these changes against all server versions and topologies (including standalone, replica set, and sharded
clusters).

semgrep-code-mongodb · 2026-08-11T23:38:23Z

+    timeout-minutes: 5
+
+    steps:
+      - uses: actions/checkout@v4


GitHub Actions step uses a mutable tag or branch reference. Tags and branch names can be silently repointed by the action owner, enabling supply-chain attacks — as seen in the trivy-action and kics-github-action compromises. Pin the reference to a full 40-character commit SHA instead, e.g. uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608.

🎈 Fixed in commit 481303b 🎈

Backing the Semgrep finding here, and @matthewdale's per-step permissions suggestion with it — I think they're worth taking rather than triaging away, specifically because of what this workflow is.

The supply-chain concern raised when this approach was agreed was "what if a compromised upstream list lands automatically," and the answer was that the sync arrives as a reviewable PR. That answer depends on the workflow that opens the PR being trustworthy. Leaving a mutable tag in a job with contents: write and pull-requests: write undercuts it: a repointed v4 runs before sync-psl.py and can push a branch and open a PR that looks exactly like a routine monthly sync.

Pinning the SHA and scoping the write permissions to the "Commit and push" and "Open a pull request" steps is cheap and makes the review-gate argument actually hold.

Hmm I previously did a quick attempt to split up the permissions and very quickly realized that permissions are scoped at either the workflow or job level (and cant be scoped to steps within a job). Splitting up these steps into multiple jobs resulted in either re-doing some work, or passing around files which felt clunky and honestly felt a bit unreadable to me (but i'm no actions script expert so maybe that's a skill issue for me haha). But I believe that the "open a pull request" step would also require using checkout v4 and still need write permissions?
I've gone ahead and pinned the SHA in 481303b (thank you for pointing it out!). Let me know what you think about the permissions though? (It's highly possible I've overlooked something >.<)

You're right and I was wrong to back that as worded — permissions is workflow- or job-level only, there's no step scope. The SHA pin was the part that mattered.

sleepyStick · 2026-08-11T23:42:49Z

+# 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.


srvAllowedHostsSuffix introduced in #1950

codeowners-service-app · 2026-08-18T05:26:09Z

Assigned Sgrinfy for team dbx-leadership because rozza is out of office.

matthewdale · 2026-08-19T07:42:54Z

+### Determining the public suffix
+
+To determine the public suffix of a domain, a driver using the vendored file MUST follow the
+[algorithm published by publicsuffix.org](https://publicsuffix.org/list/):


I can't find an algorithm for determining the public suffix list at that link. Did you mean to link to something like https://github.com/publicsuffix/list/wiki/format#algorithm ?

yes, good catch! thanks!

fixed in 5a85f77

matthewdale · 2026-08-19T07:52:56Z

+permissions:
+  contents: write
+  pull-requests: write
+


Optional: Require these permissions only for the "Commit and push" and "Open a pull request" steps.

matthewdale · 2026-08-19T08:01:29Z

+
+def fetch():
+    request = urllib.request.Request(PSL_URL, headers={"User-Agent": "mongodb-specifications-sync-psl"})
+    with urllib.request.urlopen(request) as response:


Optional: urlopen will follow redirects, including to an HTTP protocol URL. Validate that the response came from an HTTPS protocol URL to reduce the possibility the data was tampered with by a man-in-the-middle.

Suggested change

with urllib.request.urlopen(request) as response:

with urllib.request.urlopen(request) as response:

if not response.url.startswith("https://"):

sys.exit(f"Refusing to use non-HTTPS response URL {response.url!r}.")

Do we also need a timeout for local run?

good suggestion and i honestly didn't consider running the script locally (as opposed to always running the script via the action) but it doesn't hurt to add a timeout. applied these suggestions in 43cc617

matthewdale · 2026-08-19T08:06:55Z

+          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.


I don't understand what this PR note is trying to convey. Should reviewers close all PSL update PRs except for the latest? Or merge all PSL update PRs in a specific order?

umm that's a really good point. I'm sure at the time i wrote it, it made sense to me. But reading it now, it doesn't make sense HAHA. I've reworded it in 746e525 to be more clear. Let me know what you think!

qingyang-hu · 2026-08-19T19:40:56Z

+
+def fetch():
+    request = urllib.request.Request(PSL_URL, headers={"User-Agent": "mongodb-specifications-sync-psl"})
+    with urllib.request.urlopen(request) as response:


Do we also need a timeout for local run?

qingyang-hu

LGTM

matthewdale

Looks good! 👍

adelinowona · 2026-08-24T23:30:44Z

+### Determining the public suffix
+
+To determine the public suffix of a domain, a driver using the vendored file MUST follow the
+[algorithm published by the publicsuffix folks](https://github.com/publicsuffix/list/wiki/Format#algorithm):
+
+1. Compare the domain's labels against each rule's labels from right to left, treating `*` as matching any single label.
+    Collect every rule that matches.
+2. If no rule matches, the prevailing rule is `*` -- that is, the rightmost label alone is the public suffix.
+3. If any matching rule is an exception rule, it prevails. Otherwise, the matching rule with the most labels prevails.
+4. If the prevailing rule is an exception rule, remove its leftmost label.
+5. The public suffix is the set of the domain's labels matched by the prevailing rule.


This section restates the upstream algorithm rather than deferring to it, and the restatement drops a precondition that the linked section carries:

The domain, as well as all rules from the Public Suffix List, must be canonicalized in the normal way for hostnames - lower-case, Punycode (RFC 3492) - prior to being compared.

Upstream's Definitions also note that empty labels are not permitted, which rules out a leading . on the input. As written, these five steps read as self-contained, so a driver implementing from them alone does a case-sensitive comparison — and since #1950's rule is "the value MUST NOT be a public suffix," srvAllowedHostsSuffix=COM would be accepted. The Rule syntax section above doesn't close this: it states that rules are lowercase, and its MUST covers only Punycode↔Unicode conversion, not case-folding the input.

I suggest adding before step 1: "Before applying this algorithm, the domain MUST be canonicalized in the normal way for hostnames — lower-case and Punycode — and MUST NOT contain empty labels, including a leading or trailing .."

Ah, good catch! the normalization was mentioned in #1950 and it slipped my mind to restate it here before the algorithm. Added in 1605d3a thanks!

adelinowona · 2026-08-24T23:34:55Z

+    timeout-minutes: 5
+
+    steps:
+      - uses: actions/checkout@v4


Backing the Semgrep finding here, and @matthewdale's per-step permissions suggestion with it — I think they're worth taking rather than triaging away, specifically because of what this workflow is.

The supply-chain concern raised when this approach was agreed was "what if a compromised upstream list lands automatically," and the answer was that the sync arrives as a reviewable PR. That answer depends on the workflow that opens the PR being trustworthy. Leaving a mutable tag in a job with contents: write and pull-requests: write undercuts it: a repointed v4 runs before sync-psl.py and can push a branch and open a PR that looks exactly like a routine monthly sync.

Pinning the SHA and scoping the write permissions to the "Commit and push" and "Open a pull request" steps is cheap and makes the review-gate argument actually hold.

adelinowona · 2026-08-24T23:40:29Z

+`xn--` labels) and the Unicode rules it is compared against are in the same form. Comparing the two forms directly will
+fail to match rules that should match.
+
+### Determining the public suffix


The Test Plan and the consuming rule in #1950 are both phrased as a boolean ("is this domain a public suffix"), but this section defines an algorithm that returns a suffix, and nothing connects the two.

I suggest adding after step 5: "A domain is itself a public suffix if and only if the public suffix determined by this algorithm is equal to the domain." That's what is_public_suffix in the Test Plan means, and it's what "MUST NOT be a public suffix" in the Initial DNS Seedlist Discovery spec is testing — worth stating rather than leaving to inference.

Good call out, added in 1605d3a

add PSL

64f96ba

semgrep-code-mongodb Bot reviewed Aug 11, 2026

View reviewed changes

sleepyStick commented Aug 11, 2026

View reviewed changes

sleepyStick added 8 commits August 12, 2026 09:09

fix status

2d22775

fix linting

ce7d02e

move tests into intial-dns-seedlist-discovery

bc2bc4b

okay so some of those unified tests had to be prose tests,,

f4a4d19

condense tests

b741042

fix linting

8d448d0

fix lint

31e048a

reword prose tests

997cdc9

sleepyStick requested a review from rozza August 12, 2026 23:11

sleepyStick marked this pull request as ready for review August 12, 2026 23:11

sleepyStick requested review from a team as code owners August 12, 2026 23:11

sleepyStick requested review from matthewdale and qingyang-hu August 12, 2026 23:11

codeowners-service-app Bot requested a review from Sgrinfy August 18, 2026 05:26

matthewdale reviewed Aug 19, 2026

View reviewed changes

qingyang-hu requested changes Aug 19, 2026

View reviewed changes

sleepyStick added 4 commits August 19, 2026 16:33

fix link

5a85f77

check link url and add timeout

43cc617

move permissions to be in the job

323e0ac

fix PR wording

746e525

sleepyStick requested review from matthewdale and qingyang-hu August 20, 2026 00:09

qingyang-hu approved these changes Aug 20, 2026

View reviewed changes

matthewdale approved these changes Aug 20, 2026

View reviewed changes

sleepyStick requested a review from Jibola August 24, 2026 18:04

adelinowona mentioned this pull request Aug 24, 2026

DRIVERS-3329: Configurable DNS domain validation for SRV records #1950

Open

4 tasks

adelinowona requested changes Aug 24, 2026

View reviewed changes

sleepyStick added 2 commits August 24, 2026 17:23

add language around the public suffix algorithm

1605d3a

add sha for actions/checkoutv4

481303b

sleepyStick requested a review from adelinowona August 25, 2026 00:36

add test with capatilization

9c42a3c

Jibola approved these changes Aug 26, 2026

View reviewed changes

adelinowona approved these changes Aug 26, 2026

View reviewed changes

Jibola merged commit 6f64d0e into mongodb:master Aug 26, 2026
6 checks passed

		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.

Uh oh!

Conversation

sleepyStick commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

semgrep-code-mongodb Bot Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sleepyStick Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sleepyStick Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codeowners-service-app Bot commented Aug 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qingyang-hu left a comment

Choose a reason for hiding this comment

Uh oh!

matthewdale left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sleepyStick commented Aug 11, 2026 •

edited

Loading

semgrep-code-mongodb Bot Aug 11, 2026 •

edited

Loading

sleepyStick Aug 25, 2026 •

edited

Loading

sleepyStick Aug 11, 2026 •

edited

Loading