tildetown/town
4
2
Fork 2
Code Issues Pull Requests 1 Releases Wiki Activity

signup: add DNS checker and run against suspicious hosts #3

Open
nbsp wants to merge 2 commits from nbsp/town:suspicious-hosts into trunk
pull from: nbsp/town:suspicious-hosts
merge into: tildetown:trunk
Conversation 1 Commits 2 Files Changed 3 +103 -1
nbsp commented 2025-11-22 01:41:48 +00:00
First-time contributor
Copy Link

extremely WIP, proof of concept, entirely untested end-to-end, but the function part works wonders
a few open questions:

  1. i have this run on the join@tilde.town process as opposed to it being postprocessing, both for backwards compatibility with pending signups without a database migration, and because this check also gives us useful info on valid domains / connected mail servers.
  2. adding the notes is a bit weird because we don't have an ID yet? dunno what to do there.
  3. definitely need to move the suspicious hosts list into an admin-only file if we're to add any more of these moved to a table on signups.sql (a non-destructive migration is needed)

as mentioned this also adds some as-yet-unused errors that will prove useful for notifying the user that their email doesn't exist. implementing that is out of scope for this PR i think

extremely WIP, proof of concept, entirely untested end-to-end, but the function part works wonders a few open questions: 1. i have this run on the join@tilde.town process as opposed to it being postprocessing, both for backwards compatibility with pending signups without a database migration, and because this check also gives us useful info on valid domains / connected mail servers. 2. adding the notes is a bit weird because we don't have an ID yet? dunno what to do there. 3. ~~definitely need to move the suspicious hosts list into an admin-only file if we're to add any more of these~~ moved to a table on signups.sql (a non-destructive migration is needed) as mentioned this also adds some as-yet-unused errors that will prove useful for notifying the user that their email doesn't exist. implementing that is out of scope for this PR i think
nbsp added 1 commit 2025-11-22 01:41:49 +00:00
nbsp force-pushed suspicious-hosts from fde4ee9ccc to b80c5cd8cc 2025-11-22 01:45:42 +00:00 Compare
nbsp force-pushed suspicious-hosts from b80c5cd8cc to 0f73197728 2025-11-22 01:47:49 +00:00 Compare
nbsp changed title from add DNS checker and run against suspicious hosts to signup: add DNS checker and run against suspicious hosts 2025-11-22 01:47:53 +00:00
nbsp added 1 commit 2025-11-22 11:41:44 +00:00
nbsp force-pushed suspicious-hosts from 521af4dfd5 to 49bf74e692 2025-11-22 11:44:21 +00:00 Compare
vilmibm requested changes 2025-11-22 17:50:21 +00:00
vilmibm left a comment
Owner
Copy Link

I like this direction <3 thank you! extremely minor change requested

I like this direction <3 thank you! extremely minor change requested
external/cmd/signup/main.go
@ -205,0 +242,4 @@
su.Email = string(s.Input.Bytes())
suspiciousHosts, err := models.SuspiciousHosts(db)
if err != nil {
// XXX: maybe log somewhere that the database failed
vilmibm commented 2025-11-22 17:47:12 +00:00
Owner
Copy Link

logging good, see lockingwriter package and its use

logging good, see lockingwriter package and its use
external/cmd/signup/main.go
@ -205,0 +252,4 @@
if records, err := DigMX(su.Email); err == nil {
for _, record := range records {
if slices.Contains(shDomains, record) {
su.Notes = append(su.Notes, models.SignupNote{
vilmibm commented 2025-11-22 17:47:25 +00:00
Owner
Copy Link

I love using notes for this

I love using notes for this
external/cmd/signup/main.go
@ -205,2 +263,4 @@
},
func(s *scene, tv *tview.TextView, msg string) {
// TODO could check and see if it's email shaped and admonish if not
// NOTE(nbsp): DigMX call can see if email is invalid but this isn't used yet
vilmibm commented 2025-11-22 17:48:44 +00:00
Owner
Copy Link

I predict splitting out the regex from DigMX but it's fine the way it is now.

The signing up user should not have any notion of suspicious email hosts but we should gently prod them until we see an email shaped thing

I predict splitting out the regex from DigMX but it's fine the way it is now. The signing up user should not have any notion of suspicious email hosts but we should gently prod them until we see an email shaped thing
sql/create_signups_db.sql
@ -27,0 +30,4 @@
id INTEGER PRIMARY KEY,
domain TEXT,
-- unused but worth adding instead of another migration later
vilmibm commented 2025-11-22 17:46:00 +00:00
Owner
Copy Link

I find unused stuff more confusing than later need to migrate so I'd leave out unused columns for now

I find unused stuff more confusing than later need to migrate so I'd leave out unused columns for now
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u suspicious-hosts:nbsp-suspicious-hosts
git checkout nbsp-suspicious-hosts
Sign in to join this conversation.
Reviewers
No Reviewers
vilmibm
Labels
No items
No Label
Milestone
No items
No Milestone
Assignees
Clear assignees
vilmibm
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: tildetown/town#3
No description provided.
Delete Branch "nbsp/town:suspicious-hosts"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?