fix: multiple bug fixes across chatmaild and cmdeploy #836

Retengart · 2026-02-07T13:35:27Z

Summary

Bug fixes (round 1)

fsreport.py: fix AttributeError — FileEntry has path, not relpath. Crashes when --mdir flag is used
rdns.py: fix TypeError — get_dkim_entry returns bare None instead of (None, None) tuple on CalledProcessError, crashing cmdeploy dns on fresh servers
deployers.py: fix silent deploy failure — build_webpages returns None on exception, causing rsync("None/", ...)
cmdeploy.py: remove dead code and potential NameError in run_cmd — check_call always returns 0 or raises, making retcode != 0 branches unreachable; remote_data undefined with --skip-dns-check
rshell.py: fix IndexError in dovecot_recalc_quota — empty line at end of doveadm output produces parts=[]

Security fixes (round 2)

newemail.py: use secrets.choice instead of random.choices for username generation — per Python docs, secrets should be used for security-sensitive data
nginx.conf.j2: remove deprecated TLS 1.0/1.1 — RFC 8996, aligns with postfix (≥1.2) and dovecot (1.3) in same stack
doveauth.py: validate localpart characters to [a-z0-9._-] — prevents filesystem issues from crafted usernames via IMAP/SMTP auth
doveauth.py: fix TOCTOU race in account creation — use filelock to serialize concurrent creation for same address
turnserver.py: add 5s socket timeout — hung TURN daemon would block dict proxy thread indefinitely
metadata.py: handle turn_credentials() exceptions — return N (not found) instead of crashing the dict proxy connection

Formatting

chore: ruff formatting fixes in acmetool, dovecot, postfix deployers

Test plan

pytest chatmaild/src/ — 50 passed
pytest cmdeploy/src/cmdeploy/tests/ --ignore=online — 16 passed
ruff check + ruff format --check on all changed Python files — clean
CI staging deploy + online tests

FileEntry namedtuple has (path, mtime, size), not relpath. Crashes with AttributeError when --mdir flag is used.

Bare return yielded None, causing TypeError on tuple unpacking in perform_initial_checks on fresh servers without DKIM keys.

Exception in _build_webpages was silently caught, returning None. rsync then received "None/" as source path, silently breaking deploy.

check_call always returns 0 or raises, making retcode!=0 branches unreachable. Also remote_data was undefined with --skip-dns-check.

doveadm output ends with empty line, parts=[] causes parts[2] crash.

Per Python docs, secrets module should be used for security-sensitive data. random.choices uses Mersenne Twister PRNG which is predictable. secrets.choice was already used for password generation in the same file.

TLS 1.0/1.1 deprecated by RFC 8996. Nginx default is TLSv1.2 TLSv1.3. Aligns with postfix (>=TLSv1.2) and dovecot (TLSv1.3) in the same stack.

- Reject localparts with chars outside [a-z0-9._-] to prevent filesystem issues from crafted usernames via IMAP/SMTP auth - Use filelock to serialize concurrent account creation for same address, preventing TOCTOU race where two threads both create an account and last writer wins

Hung TURN daemon would block dict proxy handler thread indefinitely. Per Python docs, settimeout() raises TimeoutError on expiry.

ConnectionRefusedError/FileNotFoundError/TimeoutError from turn_credentials() would kill the dict proxy connection. Return N (not found) response instead and log the error.

- test_doveauth: invalid localpart chars rejected, concurrent same-account creation - test_expire: --mdir filtering uses msg.path correctly - test_metadata: TURN exception returns N\n, success returns credentials - test_turnserver: socket timeout, connection refused, happy path - test_dns: get_dkim_entry returns (None, None) on CalledProcessError - test_rshell: dovecot_recalc_quota handles empty/malformed output

missytake · 2026-02-08T08:09:59Z

Thanks for this giant PR! I will create a separate branch so the CI can deploy it to our test servers.

Usually we like to have one PR per change, so we can decide about them individually^^ let's see, if all of it is uncontroversial, it's probably fine, and at least the commits are properly separated :) I will take a look at your commits later today (or latest, on Tuesday).

missytake

Wow, it's really an impressive list of fixes! And thank you especially for adding tests, nice.

missytake · 2026-02-08T08:38:51Z

cmdeploy/src/cmdeploy/tests/test_dns.py

+        assert result == (None, None)
+        assert result[0] is None and result[1] is None


Out of interest: Is there a scenario in which the first statement passes and the second doesn't?

I guess I overlooked it... Of course, you're right — it's redundant. None is a singleton so == None and is None are equivalent. Removed in the next push.

None is a singleton; == already covers is None check.

- Handle filelock.Timeout gracefully in lookup_passdb: return N (not found) instead of killing the entire dict connection - Remove unused call_count variables from test_rshell.py

Retengart · 2026-02-08T10:17:33Z

Two follow-up fixes based on self-review:

doveauth.py: filelock.Timeout was uncaught — if the lock couldn't be acquired in 5s, the exception would propagate to DictProxy's generic handler and kill the entire Dovecot dict connection (affecting all in-flight requests). Now caught locally: logs a warning and returns N (not found) so only the single lookup fails gracefully and the client can retry.
test_rshell.py: removed unused call_count variables from 3 tests (declared and incremented but never asserted).

Retengart added 12 commits February 7, 2026 16:27

chore: fix ruff formatting in acmetool, dovecot, postfix deployers

ea52fde

fix: use msg.path instead of nonexistent msg.relpath in fsreport

0443965

FileEntry namedtuple has (path, mtime, size), not relpath. Crashes with AttributeError when --mdir flag is used.

fix: return tuple from get_dkim_entry on CalledProcessError

a5dc1d8

Bare return yielded None, causing TypeError on tuple unpacking in perform_initial_checks on fresh servers without DKIM keys.

fix: handle build_webpages returning None in WebsiteDeployer

3ad8e5a

Exception in _build_webpages was silently caught, returning None. rsync then received "None/" as source path, silently breaking deploy.

fix: remove dead code and potential NameError in run_cmd

0148ecd

check_call always returns 0 or raises, making retcode!=0 branches unreachable. Also remote_data was undefined with --skip-dns-check.

fix: guard against IndexError in dovecot_recalc_quota

93eb996

doveadm output ends with empty line, parts=[] causes parts[2] crash.

fix(security): use secrets.choice instead of random.choices for username

c48a7d8

Per Python docs, secrets module should be used for security-sensitive data. random.choices uses Mersenne Twister PRNG which is predictable. secrets.choice was already used for password generation in the same file.

fix(security): remove deprecated TLS 1.0/1.1 from nginx config

0d3cde9

TLS 1.0/1.1 deprecated by RFC 8996. Nginx default is TLSv1.2 TLSv1.3. Aligns with postfix (>=TLSv1.2) and dovecot (TLSv1.3) in the same stack.

fix: add 5s timeout to TURN credential socket

47c9586

Hung TURN daemon would block dict proxy handler thread indefinitely. Per Python docs, settimeout() raises TimeoutError on expiry.

fix: handle turn_credentials exceptions in metadata proxy

447c0ee

ConnectionRefusedError/FileNotFoundError/TimeoutError from turn_credentials() would kill the dict proxy connection. Return N (not found) response instead and log the error.

missytake mentioned this pull request Feb 8, 2026

WIP: run CI for #836 #839

Closed

missytake deployed to staging2.testrun.org February 8, 2026 08:13 — with GitHub Actions View deployment

missytake deployed to staging-ipv4.testrun.org February 8, 2026 08:13 — with GitHub Actions View deployment

missytake approved these changes Feb 8, 2026

View reviewed changes

Retengart added 2 commits February 8, 2026 12:55

fix: remove redundant assertion in test_dkim_entry

f9e2b57

None is a singleton; == already covers is None check.

fix: catch filelock.Timeout in doveauth and remove dead test vars

6a7adca

- Handle filelock.Timeout gracefully in lookup_passdb: return N (not found) instead of killing the entire dict connection - Remove unused call_count variables from test_rshell.py

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: multiple bug fixes across chatmaild and cmdeploy #836

fix: multiple bug fixes across chatmaild and cmdeploy #836

Uh oh!

Retengart commented Feb 7, 2026 •

edited by missytake

Loading

Uh oh!

missytake commented Feb 8, 2026 •

edited

Loading

Uh oh!

missytake left a comment

Uh oh!

missytake Feb 8, 2026

Uh oh!

Retengart Feb 8, 2026 •

edited

Loading

Uh oh!

Retengart commented Feb 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		assert result == (None, None)
		assert result[0] is None and result[1] is None

Uh oh!

fix: multiple bug fixes across chatmaild and cmdeploy #836

Are you sure you want to change the base?

fix: multiple bug fixes across chatmaild and cmdeploy #836

Uh oh!

Conversation

Retengart commented Feb 7, 2026 • edited by missytake Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Bug fixes (round 1)

Security fixes (round 2)

Formatting

Test plan

Uh oh!

missytake commented Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

missytake left a comment

Choose a reason for hiding this comment

Uh oh!

missytake Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Retengart Feb 8, 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!

Retengart commented Feb 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Retengart commented Feb 7, 2026 •

edited by missytake

Loading

missytake commented Feb 8, 2026 •

edited

Loading

Retengart Feb 8, 2026 •

edited

Loading