[Sprint 8] Setup Robustness, CI & Documentation by uzyn · Pull Request #10 · uzyn/aimx

uzyn · 2026-04-10T03:01:39Z

Summary

S8.1 Improve DNS verification accuracy: SPF now uses exact IP matching via mechanism parsing (rejects prefix/suffix collisions like 1.2.3.4 vs 1.2.3.45), DKIM verification checks the p= public key matches the local key, DMARC warns when policy is p=none (too permissive)
S8.2 Propagate --data-dir to OpenSMTPD ingest command: generate_smtpd_conf() now accepts a data directory parameter and includes --data-dir in the MDA command when non-default
S8.3 Fix SPF verification domain fallback: returns spf: none when sender domain cannot be determined instead of incorrectly evaluating the recipient's domain; extracted standalone spf_domain() function (resolves Sprint 5.5 backlog item)
S8.4 Configurable verify service URLs: added probe_url and verify_address optional fields to config.yaml and Config struct, with defaults to check.aimx.email and verify@aimx.email
S8.5 CI coverage for verify service: added check-verify-service job running fmt, clippy, and test in services/verify/
S8.6 Documentation and status fixes: fixed GitHub URLs (nicholasgasior -> uzyn), removed DigitalOcean from compatible providers (SMTP permanently blocked), added "Recent activity" section to aimx status output

Test plan

All 258 unit tests pass
All 35 integration tests pass
cargo clippy -- -D warnings clean
cargo fmt -- --check clean
Verify service: all 17 tests pass, clippy and fmt clean
CI workflow runs both jobs on PR

- S8.1: Improve DNS verification accuracy (SPF exact IP matching, DKIM public key comparison, DMARC p=none warning) - S8.2: Propagate --data-dir to OpenSMTPD ingest command - S8.3: Fix SPF verification domain fallback (return none instead of using recipient domain; extract spf_domain() function) - S8.4: Add configurable probe_url and verify_address to config.yaml - S8.5: Add CI job for verify service (fmt, clippy, test) - S8.6: Fix GitHub URLs, remove DigitalOcean from compatible providers, add recent activity to aimx status output

uzyn

Sprint 8 Code Review

Sprint Goal Assessment

Goal: Fix medium and low-severity issues: strengthen DNS verification, propagate configuration correctly, add CI coverage for the verify service, and resolve documentation inconsistencies.

Verdict: Goal fully met. All 6 stories are implemented correctly with thorough test coverage. The changes are well-scoped and focused.

Acceptance Criteria Checklist

S8.1 -- Improve DNS Verification Accuracy

Criterion	Status
SPF verification checks exact IP via mechanism parsing (not bare substring)	Met -- `spf_contains_ip()` parses `ip4:` and `+ip4:` mechanisms, strips CIDR, compares exact string
DKIM verification extracts `p=` value and confirms it matches local public key	Met -- `extract_dkim_public_key()` + comparison in `verify_dkim()`
DMARC verification warns if policy is `p=none`	Met -- new `DnsVerifyResult::Warn` variant, displayed as WARN (does not block setup)
Unit test: SPF with similar-prefix IP correctly fails	Met -- `verify_spf_rejects_prefix_match` and `verify_spf_rejects_suffix_match`
Unit test: DKIM record with mismatched public key reports failure	Met -- `verify_dkim_fail_mismatched_key`
Unit test: DMARC with `p=none` reports warning	Met -- `verify_dmarc_warns_on_p_none`

S8.2 -- Propagate --data-dir to OpenSMTPD Ingest Command

Criterion	Status
`generate_smtpd_conf()` accepts a data directory parameter	Met -- `data_dir: Option<&Path>` parameter added
When data dir is non-default, MDA command includes `--data-dir <path>`	Met -- verified in `smtpd_conf_custom_data_dir_includes_flag`
Default path omits the flag for cleaner config	Met -- verified in `smtpd_conf_default_data_dir_omits_flag`
Unit test: custom data dir produces `--data-dir` in smtpd.conf	Met
Unit test: default data dir omits `--data-dir` in smtpd.conf	Met

S8.3 -- Fix SPF Verification Domain Fallback

Criterion	Status
SPF verification returns `spf: none` when sender domain cannot be determined	Met -- returns `"none"` early instead of falling back to rcpt domain
SPF domain-selection logic extracted into standalone testable function	Met -- `pub fn spf_domain(mail_from: &str) -> Option<&str>`
Unit test: empty sender domain produces `spf: none`	Met -- `spf_domain_empty_sender_returns_none`, `spf_domain_no_at_returns_none`, `spf_domain_empty_domain_part_returns_none`
Unit test: valid sender domain is used correctly	Met -- `spf_domain_with_valid_sender`

S8.4 -- Configurable Verify Service URLs

Criterion	Status
`config.yaml` supports optional `probe_url` and `verify_address` fields	Met -- added to `Config` struct with serde defaults and `skip_serializing_if`
`check_inbound_port25()` uses configured probe URL, defaulting to `check.aimx.email`	Met -- `RealNetworkOps` now carries `probe_url` field, threaded from config in `main.rs`
`verify::run()` uses configured verify address, defaulting to `verify@aimx.email`	Met -- `resolve_verify_address()` reads from config
Unit test: custom probe URL is used when configured	Met -- `real_network_ops_custom_probe_url`
Unit test: custom verify address is used when configured	Met -- `resolve_verify_address_uses_custom`
Update verify service README to document config fields	Met -- README updated with `config.yaml` example

S8.5 -- CI Coverage for Verify Service

Criterion	Status
CI workflow runs `cargo test` in `services/verify/`	Met
CI workflow runs `cargo clippy -- -D warnings` for `services/verify/`	Met
CI workflow runs `cargo fmt -- --check` for `services/verify/`	Met

New check-verify-service job added as a separate parallel job with proper working-directory, rust-cache scoping, and stable toolchain. Clean implementation.

S8.6 -- Documentation & Status Fixes

Criterion	Status
`aimx status` includes "Recent activity" section	Met -- `gather_recent_activity()` reads last 3 emails per mailbox, sorts by date, truncates to 5
`StatusInfo` struct extended with recent activity data	Met -- `RecentEmail` struct, `recent_activity` field
README.md and docs/idea.md are consistent on DigitalOcean	Met -- DigitalOcean removed from README's compatible providers table; idea.md already lists it as permanently blocked
Fix GitHub URLs in README.md and services/verify/README.md	Met -- `nicholasgasior` -> `uzyn` in both files
Unit test: `format_status` output includes recent activity section	Met -- `format_status_includes_recent_activity`, `format_status_no_recent_activity`, `gather_recent_activity_reads_emails`

Test Coverage

258 unit tests pass (up from baseline -- new tests for SPF IP matching, DKIM key comparison, DMARC warning, smtpd.conf data-dir, spf_domain extraction, recent activity, config fields, verify address resolution)
35 integration tests pass
cargo clippy -- -D warnings clean
cargo fmt -- --check clean
New test coverage adequately exercises edge cases (prefix/suffix IP collisions, CIDR notation, empty sender domain, mismatched DKIM keys)

No significant test coverage gaps identified.

Potential Issues (Non-Blocking)

spf_contains_ip only handles ip4: mechanisms, not ip6: -- If a server has an IPv6 address, SPF records with ip6: mechanisms won't be matched. This is a pre-existing limitation (the old contains() approach was equally broken for IPv6), so not a regression. Worth noting for a future improvement.
Unquoted data dir path in MDA command -- generate_smtpd_conf produces --data-dir {path} without quoting. If the path contains spaces, sh -c would split it. This is an extremely unlikely edge case for a server data directory and is non-blocking.
DnsVerifyResult::Warn does not fail all_pass -- By design, DMARC p=none is a warning that doesn't block setup. This is the correct behavior per the acceptance criteria ("warns" vs "fails"), but operators should be aware that a p=none DMARC record will still let setup proceed.

Security Issues

No security issues found. The changes are configuration/verification improvements that strengthen the existing security posture:

More accurate SPF IP matching prevents false positives
DKIM public key verification catches mismatched keys
DMARC permissive policy warning alerts operators

Alignment with PRD

All changes align with the PRD:

FR-7 (Verify DNS records correctly set) -- strengthened by S8.1
FR-4 (Configure OpenSMTPD with MDA delivery) -- fixed for custom data dirs by S8.2
FR-33/FR-34 (SPF verification) -- fixed domain fallback by S8.3
FR-38/FR-40 (Verify service, self-hostable) -- made configurable by S8.4
FR-47 (aimx status shows recent activity) -- implemented by S8.6
NFR-7 (Filesystem-based storage) -- consistent throughout

Summary

Verdict: Ready to merge (APPROVED)

All 6 stories fully implemented with comprehensive test coverage. The code is clean, well-structured, and addresses all acceptance criteria. No bugs or blockers found. The 3 non-blocking observations are pre-existing limitations or cosmetic edge cases, not regressions.

Recommended merge commit message:

[Sprint 8] Setup robustness, CI coverage & documentation fixes

- Improve DNS verification accuracy: exact IP matching for SPF, public key
  comparison for DKIM, permissive policy warning for DMARC
- Propagate --data-dir to OpenSMTPD MDA command for custom data directories
- Fix SPF domain fallback: return none instead of evaluating recipient domain
- Add configurable probe_url and verify_address to config.yaml for self-hosted
  verify service
- Add CI job for services/verify (fmt, clippy, test)
- Fix GitHub URLs, remove DigitalOcean from compatible providers, add recent
  activity section to aimx status

…lans Code-review-backed fix plans for each of the 10 findings, with file:line refs, effort estimates, and a priority order. No code changes yet — this consolidates the investigation so fixes can be sequenced. - #10 DKIM mismatch: not a code bug; DNS republish + optional startup check - #9 shell injection: pass trigger vars via env, not string substitution - #8 MCP writes: route state mutations through daemon UDS - #7 claude-code hint: print `claude mcp add` command post-install - #4 send config read: move mailbox resolution to daemon side - #2 SPF: plumb envelope MAIL FROM from smtp session through ingest - #5 wildcard send: remove wildcard branch from resolve_from_mailbox - #1 mailbox create: add restart hint (or route via daemon) - #3 plan wording: clarify "compose new" in docs/manual-test.md

* docs: add manual test results with findings Full execution of docs/manual-test.md against agent.zeroshot.lol. 10 findings recorded with severity and fix direction, notably: - P0 DKIM key on disk does not match DNS TXT (root cause of outbound dkim=fail at Gmail) - P0 Shell injection in on_receive cmd template expansion - P1 MCP write ops (email_mark_read, etc.) fail when MCP runs as non-root due to root:root 0644 mailbox files * docs: add Recommended fixes section with per-finding implementation plans Code-review-backed fix plans for each of the 10 findings, with file:line refs, effort estimates, and a priority order. No code changes yet — this consolidates the investigation so fixes can be sequenced. - #10 DKIM mismatch: not a code bug; DNS republish + optional startup check - #9 shell injection: pass trigger vars via env, not string substitution - #8 MCP writes: route state mutations through daemon UDS - #7 claude-code hint: print `claude mcp add` command post-install - #4 send config read: move mailbox resolution to daemon side - #2 SPF: plumb envelope MAIL FROM from smtp session through ingest - #5 wildcard send: remove wildcard branch from resolve_from_mailbox - #1 mailbox create: add restart hint (or route via daemon) - #3 plan wording: clarify "compose new" in docs/manual-test.md

Sprint 44 (post-launch security + quick fixes) addresses findings #9, #10, #7, #1-tier-1, #3. Sprint 45 (strict outbound + MCP writes via daemon addresses #4, #5, #8 and introduces UDS MARK-READ/MARK-UNREAD verbs. Sprint 46 (mailbox CRUD via UDS) addresses #1-tier-2 with MAILBOX-CREATE/MAILBOX-DELETE verbs so daemon picks up changes live. PRD FR-18d tightened: outbound send now requires a concrete non-wildcard mailbox under config.domain; catchall is inbound-only. PRD FR-18e added to cover the new state-mutation verbs on the UDS socket. Finding #2 (SPF envelope MAIL FROM) excluded — already shipped in cd22428. EOF )

[Sprint 8] Setup robustness, CI coverage & documentation fixes - Improve DNS verification accuracy: exact IP matching for SPF, public key comparison for DKIM, permissive policy warning for DMARC - Propagate --data-dir to OpenSMTPD MDA command for custom data directories - Fix SPF domain fallback: return none instead of evaluating recipient domain - Add configurable probe_url and verify_address to config.yaml for self-hosted verify service - Add CI job for services/verify (fmt, clippy, test) - Fix GitHub URLs, remove DigitalOcean from compatible providers, add recent activity section to aimx status

…lans Code-review-backed fix plans for each of the 10 findings, with file:line refs, effort estimates, and a priority order. No code changes yet — this consolidates the investigation so fixes can be sequenced. - #10 DKIM mismatch: not a code bug; DNS republish + optional startup check - #9 shell injection: pass trigger vars via env, not string substitution - #8 MCP writes: route state mutations through daemon UDS - #7 claude-code hint: print `claude mcp add` command post-install - #4 send config read: move mailbox resolution to daemon side - #2 SPF: plumb envelope MAIL FROM from smtp session through ingest - #5 wildcard send: remove wildcard branch from resolve_from_mailbox - #1 mailbox create: add restart hint (or route via daemon) - #3 plan wording: clarify "compose new" in docs/manual-test.md

* docs: add manual test results with findings Full execution of docs/manual-test.md against agent.zeroshot.lol. 10 findings recorded with severity and fix direction, notably: - P0 DKIM key on disk does not match DNS TXT (root cause of outbound dkim=fail at Gmail) - P0 Shell injection in on_receive cmd template expansion - P1 MCP write ops (email_mark_read, etc.) fail when MCP runs as non-root due to root:root 0644 mailbox files * docs: add Recommended fixes section with per-finding implementation plans Code-review-backed fix plans for each of the 10 findings, with file:line refs, effort estimates, and a priority order. No code changes yet — this consolidates the investigation so fixes can be sequenced. - #10 DKIM mismatch: not a code bug; DNS republish + optional startup check - #9 shell injection: pass trigger vars via env, not string substitution - #8 MCP writes: route state mutations through daemon UDS - #7 claude-code hint: print `claude mcp add` command post-install - #4 send config read: move mailbox resolution to daemon side - #2 SPF: plumb envelope MAIL FROM from smtp session through ingest - #5 wildcard send: remove wildcard branch from resolve_from_mailbox - #1 mailbox create: add restart hint (or route via daemon) - #3 plan wording: clarify "compose new" in docs/manual-test.md

Sprint 44 (post-launch security + quick fixes) addresses findings #9, #10, #7, #1-tier-1, #3. Sprint 45 (strict outbound + MCP writes via daemon addresses #4, #5, #8 and introduces UDS MARK-READ/MARK-UNREAD verbs. Sprint 46 (mailbox CRUD via UDS) addresses #1-tier-2 with MAILBOX-CREATE/MAILBOX-DELETE verbs so daemon picks up changes live. PRD FR-18d tightened: outbound send now requires a concrete non-wildcard mailbox under config.domain; catchall is inbound-only. PRD FR-18e added to cover the new state-mutation verbs on the UDS socket. Finding #2 (SPF envelope MAIL FROM) excluded — already shipped in f5cebd2. EOF )

uzyn commented Apr 10, 2026

View reviewed changes

uzyn merged commit 8928452 into main Apr 10, 2026
4 checks passed

uzyn deleted the sprint-8 branch April 10, 2026 03:06

uzyn mentioned this pull request Apr 17, 2026

docs: manual test plan + full test-run results (10 findings) #78

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Sprint 8] Setup Robustness, CI & Documentation#10

[Sprint 8] Setup Robustness, CI & Documentation#10
uzyn merged 1 commit into
mainfrom
sprint-8

uzyn commented Apr 10, 2026

Uh oh!

uzyn left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

uzyn commented Apr 10, 2026

Summary

Test plan

Uh oh!

uzyn left a comment

Choose a reason for hiding this comment

Sprint 8 Code Review

Sprint Goal Assessment

Acceptance Criteria Checklist

S8.1 -- Improve DNS Verification Accuracy

S8.2 -- Propagate --data-dir to OpenSMTPD Ingest Command

S8.3 -- Fix SPF Verification Domain Fallback

S8.4 -- Configurable Verify Service URLs

S8.5 -- CI Coverage for Verify Service

S8.6 -- Documentation & Status Fixes

Test Coverage

Potential Issues (Non-Blocking)

Security Issues

Alignment with PRD

Summary

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant