main
7
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
9eddd49602 |
audit 2c: findings of the two reviews
test / test (push) Canceled after 0s
Technical review (Opus, 11 findings, 2 medium) and security review (Fable, no high or medium) of 3c86280..HEAD. Both confirm the port gives dkim_mail_parse()'s result on every input it parses, and that no line the check passes is signed with another domain's key. - F1 (medium): OpenDKIM's parser takes lines no mail program reads as one address - "ceo@bank.com:me@mine.com", "Name <ceo@bank.com:me@mine.com>", "<b@evil.com, a@mine.com>", "ceo@bank.com <me@mine.com>", "ceo@bank.com@mine.com" - and signs for the last while a lenient reader shows the first. The line must now also be one address in RFC 5322's grammar (net/mail, display names in any charset passed through), at the domain OpenDKIM signs for. A quoted display name may still hold anything. - F2 (medium): the bindings were asked about OpenDKIM's form of the address, quotes and all, so "x"@mine.com passed for an application holding @mine.com while x@mine.com belonged to another. They are now asked about the reader's address, its local part unquoted, as Postfix holds the envelope's. - F3 / security (low): Bindings.Allowed and DomainOf fold case in ASCII only: Postfix folds as Unicode (casefold.c, ICU), but every key and login the panel writes is ASCII, and the Unicode fold let the Kelvin sign pass for "k" (net/mail then refuses such a line on the domain test). - F4 (low): a TERM between "postfix start-fg &" and "child=$!" is no longer lost: the wrapper stops Postfix as soon as the pid is known. - F5 (low): for startsecs after a start any failed reload passed as "not running". postfix-reload is now build/postfix-reload.sh: exit 75 when master -t says no master runs, else exec postfix reload; RunOnce returns EndError with the status, and only 75 is taken as done. supervisor.State goes. - F6 (low): crashexit spares postfix-reload too - a reload that could not be spawned is reported to whoever asked, not a reason to stop the container. - F7 (low): a certs/ that is itself a link is resolved before the walk (filepath.WalkDir does not follow its root). - F8 (low): at shutdown the tailer reads what is left of the rotated file before saving its place. Not changed: a refusal line still waiting on the old file at shutdown is not kept - the panel writes refused rows itself, only a kill loses one, and that window is the minute's grace. - F9 (low): pct read as text - empty or unreadable is 100, the rest held to 0..100. - F10 (low): the directory is synced after sasldb2's rename. - F11 (low): POSTFIX_STARTSECS by expr, so "010" or "08" in MILTER_WAIT_TIMEOUT are decimal, not octal or an error. - Security (low): pagehide also empties defaultValue - the input's value attribute and the textarea's text - not only .value. A sasldb2.new left by a killed panel is removed at start (entrypoint) and never archived. security.md says that links under certs/ are followed and by whom they can be placed. Tests: sender check cases for F1 (four lines), F2 (a bound address quoted, for its owner and for another), F3; EndError's state and status; pct cases. architecture.md and CHANGELOG describe the reader rule. Stand (2026-10-15, make e2e, exit 0); go vet and go test ./... pass on the server (24 packages). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
fbd0acd035 |
audit 2c: the rest the sources contradict, and the documents
test / test (push) Canceled after 0s
Code and config: - C-M7: a DMARC report's explicit <pct>0</pct> stays 0; only an absent pct takes RFC 7489 § 6.3's default 100 (Pct *int). - C-D1: docs/development.md now tables the Debian versions the citations were read at (postfix 3.7.11-0+deb12u1, opendkim and libopendkim11 2.11.0~beta2-8+deb12u1, libmilter1.0.1 8.17.1.9-2+deb12u2, libsasl2-2 and sasl2-bin 2.1.28+dfsg-10, supervisor 4.2.5-1, logrotate 3.21.0-1 - read off the image with dpkg-query, 2026-10-15); the e2e stand's new checkCitedPackageVersions fails when the image it built differs. - C-E2: the e2e zone's SOA serial only grows (CoreDNS v1.14.6's file plugin reloads on a grown serial, not on mtime); Corefile comment says so; TestZoneSerialGrows. - C-K8, C-K9: the e2e compose and stage comments say how --project-directory and .env resolve. - D-arch:154: postfix-config.sh's "Postfix insists" comment says what TLS_README advises and that smtpd reads the key as root. - B-G7: inflight.go's reservationTTL comment: a dropped client does give ABORT (smtpd_proto() -> mail_reset() -> milter_abort()); the TTL is for smtpd dying or the milter connection breaking. - D-guide:301: the nginx fragment's first-certificate command now reaches certbot (--entrypoint certbot, --standalone, port 80 free), and the renewal loop passes --webroot -w /var/www/certbot - a standalone certificate would otherwise renew standalone while nginx holds port 80 (certbot docs, "Renewing certificates": the issuing plugin is used "unless you specify other plugins or options", and options of a successful renewal are saved). The register's proposal missed that. Documents: every D-arch, D-guide and D-sec claim the register's Answers E found false or partly true, rewritten - by three Sonnet agents, one per document, then reviewed: two proposals were obsolete after this stage's fixes and not used (a bounce's From: line - extract_from = "smtp" now; "Origin: null" - Referrer-Policy is same-origin now); a stand observation the agent had dated from a commit is now tied to the e2e test that checks it; the SQLite writers paragraph describes _txlock=immediate; the log tailer's rotation paragraph describes the minute's grace; the guide's nginx steps were put in working order (links and modes before `up`, no reconfigure step). "Deferred" is kept for Postfix's queue in the guide too. guide.md's CRLF from the agent normalised. Stand (2026-10-15, make e2e, exit 0, the version gate and zone serial test included); go vet and go test ./... pass on the server (24 packages). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
599149bf83 |
build, deploy, inbound, dmarc, backup, e2e: cite the source of what they rely on (assumptions part C)
Comments only, no behaviour changed. Each main.cf/master.cf parameter the
build sets, the wrapper's and the reload hook's reliance on Postfix 3.7.11
(postfix-script, master.5, postconf.5, cleanup_init.c, smtpd_peer.c,
README_FILES), supervisor 4.2.5 (process.py, options.py, events.rst),
logrotate 3.21.0, OpenDKIM's config, Cyrus SASL's sasldb, rspamd 3.11.1
(milter_headers.lua, multimap, the map watcher), Docker Compose, CoreDNS
and certbot now say where that is written. Measurements already there stay
measurements ("date not recorded" where none was); hand-composed fixtures
say so.
Contradictions left for their own commits: C-P31 cleanup's size limit,
C-P42 master outside supervisor's signal, C-V5/V6 startsecs, C-V10,
C-S3/Q3 sasldb written in place, C-Q4/A1 certbot's symlinks, C-R7 rspamd
skips local clients but the signature routine does not, C-P8, C-R4, C-R12,
C-M7, C-E2, C-K8/K9, C-D1.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
|
09d9625f32 |
dmarc-ingest: what is not a report this relay keeps is dropped, not retried
Owner, 2026-10-11: foreign and malformed reports are turned away. The command answered every failure with EX_TEMPFAIL, so Postfix kept a message that could never be stored and offered it again for days, each attempt adding to the parse-failure count. IngestMessage now marks those failures (unreadable as an aggregate report, a domain not sent from here, another domain's report address) with ErrRejected; the command counts one failure, writes the reason where Postfix puts it on the delivery line of the mail log and reports the delivery as done. No bounce: the envelope sender of such a message is whatever its author wrote, and the report addresses are public. A failing store is still EX_TEMPFAIL. go vet ./cmd/... ./internal/dmarc/: ok go test ./cmd/panel/ ./internal/dmarc/ ./internal/web/...: ok Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
b2fbe91de9 |
panel-redesign: security review of 2.0 - sessions belong to a user by id
Stage 3, step 2. Reviewed by Fable (read-only), fixed by Sonnet with a
failing test before each fix, checked by Opus. No Critical or High
finding: route authorization, tenant isolation by id, the /server/
subtree guard, output encoding and the origin check were found sound.
Fixed
- M1: a session was keyed by user name and had no link to the user. A
deleted or renamed user's cookie stayed alive (renewed on every hit,
before the user was looked up) and became a session of whoever was
later created under that name, with that account's role. The
baseline now has sessions.user_id REFERENCES users(id) ON DELETE
CASCADE; the principal is resolved by id and a session is renewed
only after its user was found. RenameSession is gone.
- M2: any user's password change deleted every other user's sessions.
It ends that user's other sessions only.
- L1: a password set on the user form ends that user's sessions (an
administrator editing their own account keeps the current one).
- L2: store, saslpasswd2 and Postfix reload errors were shown verbatim
on the application form, the inbound upstream / recipients forms and
the limit recalculation - pages a domain administrator reaches.
Validation errors (app.ValidationError, inbound.ValidationError,
store.ErrNotAutoRateLimit) keep their text; anything else is logged
and shown as "Could not save ... Check the logs and try again."
- L3: store.ListDMARCReports treated an empty domain list as "all". It
takes a scope now (AllDomains or a list; an empty list is no rows),
and the DMARC hub is open to the domain role as the plan says: the
reports of the assigned domains, without the instance-wide ingest
box. The menu entry was already shown to them and led to a 404.
- I1: /static/ answered with a listing of the embedded assets. A path
that is not a known asset is a 404.
Consequences
- The global DMARC hub lists reports of domains that were deleted
since, too.
- dev/migrate1x (not in the tree) no longer carries sessions over:
everyone signs in again after the transfer.
- docs/schema-migrations.md: the sessions row.
Recorded, not changed - for the owner:
- L4: the report-address select names the followed user and their
address to every administrator of the domain, by default the global
administrator's login name. The plan asks for it.
- L5: a delegated inbound upstream may be any host and port, loopback
and private ranges included - within the trusted-operator model; to
be listed among the accepted risks in security.md.
- L6: Account's profile is saved without the password; becomes a
requirement once password-reset mails to users.email.
- Still echoing an error's text: backup and domain import (global
only), the domain form's validation messages.
Not checked by the review: anything needing the container (e2e, real
Postfix / DNS), CSP behaviour in a live browser.
go test -count=1 ./internal/web/... ./internal/store/... ./internal/app/...
./internal/inbound/... ./internal/dmarc/... ./cmd/panel
ok (all eleven packages; dev/migrate1x also ok)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
|
7ef7f75246 |
release: 1.9.2: Review pass: port-25 milters, DMARC validation, rate-limit fix
Review pass over `1.4.0`…`1.9.1`: port 25 no longer shares the submission milter chain, DMARC reports are validated before they are stored, and the auto rate limit is correct for a non-default level-1 window. Schema migration `0010` (index only). Upgrading is a tag bump. Added: - SQLite migration `0010_send_log_app_login_index.sql` — - DMARC aggregate reports are accepted as **zip** as well as gzip or plain XML - e2e: `checkPort25ConfigCombinations` regenerates the Postfix configuration Changed: - README hero line and a **Positioning** block in `docs/product.md` record the Fixed: - The setup account's password is hashed at bcrypt cost `12` like every other - Login/setup rate limiter: protecting blocked buckets from eviction is now - `postqueue -p` output is bounded to 4 MiB (the timeout alone left the size of - [schema-migrations.md](docs/schema-migrations.md): the schema section was - postqueue reader is now time-bounded: `postqueue -p` runs under a 5-second … (11 more in CHANGELOG) Changed: - [roadmap.md](docs/roadmap.md) now carries everything the 2026-08-19 code - Extracted `internal/configsafe` — the empty-value and forbidden-character - Build, deploy and migration comments no longer cite the archived - Dropped the `parseDomainRateLimitForm` / `parseAppRateLimitForm` pass-through - Extracted shared helpers: `internal/atomicfile` (atomic temp-write+rename) and … (7 more in CHANGELOG) Removed: - Unused legacy asset `docs/assets/icon.png` (superseded by the four brand SVGs - Plan documents for already-shipped features: `domain-stats-auto-ratelimit`, Full detail: CHANGELOG.md (1.9.2). |
||
|
|
20d88e1d9c |
release: 1.7.0: DMARC aggregate report ingest and panel summaries
Optional DMARC aggregate report ingest: accept `rua=` mail on port 25, parse gzip/XML into SQLite, and show summaries in the panel. Added: - Optional DMARC aggregate report ingest (`DMARC_REPORTS_ENABLE`): port 25 - Per-domain **SelfPost hosted** `rua=` mode when ingest is enabled Full detail: CHANGELOG.md (1.7.0). |