latest followed the default branch, so every push to main went straight
to the tag the README tells people to pull — including work in progress,
and migrations that run automatically on boot. It now moves only on a v*
tag; main still publishes under its own name for anyone wanting the tip.
The roadmap listed three finished things as open, including the label
refactor and the nested label UI, both of which shipped. A roadmap that
under-reports is worse than none: it is the document a contributor reads
to decide what to pick up. Corrected, with the genuinely open items
described accurately and a note asking that it be kept honest.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
55k lines of PHP with no static analysis, and a dependency tree holding
four OAuth clients, a JWT library and an IMAP parser with nothing
watching it for advisories.
PHPStan at level 5 with a baseline. Level 5 is where the class of defect
this codebase actually has starts showing up — nullable entity getters
flowing into non-nullable parameters — without a fortnight of triage
first. 208 findings are baselined as a debt ledger, not a licence; the
level goes up when the ledger is small enough that raising it is a day's
work.
Two of the 217 were real bugs, both fixed here rather than baselined:
"Test connection" in account settings constructed ConnectionTestResult
with four arguments where six are required, and passed null for a string
parameter. Both call sites are the early returns for an incomplete form
and a blank password — that is, the two cases the button exists to
report, each of which raised an ArgumentCountError instead.
GraphCategorySyncer caught App\Service\Mail\GraphApiException, which does
not exist; the client throws App\Domain\Exception\GraphApiException. The
catch never matched, so the 403 that means "this account needs
reconnecting with MailboxSettings.ReadWrite" propagated as an unhandled
error instead of degrading quietly, which is what the comment inside it
says it is there to do.
One more it found is left alone deliberately, because it needs a decision
rather than a fix: Controller/Admin/User/DefaultController references
App\Form\Admin\UserFormType and User::setName(), neither of which exists,
so that admin page fatals on load.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Neither had any limit. The README explains how to expose plMail to the
internet, so the password form was reachable and guessable at whatever
rate an attacker could manage.
login_throttling covers the password form: five attempts per account per
fifteen minutes, keyed per username so one attacker cannot lock every
other account on the install out of its own login.
The second factor needs its own throttle, because login_throttling stops
at the password form. By the time the code form is reachable the password
has already been accepted — so an attacker holding a stolen or phished
password arrives with the password limiter satisfied and six digits
between them and the mailbox, inside a window otphp widens to about a
minute by design. TwoFactorThrottle refuses on the way in rather than
after verification, and a correct code clears the count so the
legitimate user is not punished for the attacker's attempts.
LoginThrottlingTest asserts the limit is actually reached. An inactive
limiter is indistinguishable from a working one — every request still
renders the ordinary bad-credentials page — and the naive version of this
test, which looked for a 429, passed against no limiter at all. It also
clears the limiter's cache pool in setUp: the counters live in a
filesystem pool with a 15-minute window, so left alone they accumulate
across runs until the per-IP backstop is exhausted and the tests start
passing for the wrong reason.
Also stops the login template rendering APP_DEV_USER_PASSWORD into the
password field's value attribute with no environment guard. It is empty
in the shipped .env so nothing leaked by default, but nothing stopped it
being set either. And the "forgot password?" link was an href="#" that
did nothing: there is no reset flow, recovery is a console command, and
an affordance that lies is worse than none.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The README has always said AGPL-3.0 and the repository invites pull
requests, but there was no LICENSE file and composer.json declared
"proprietary". In that state nobody could legally fork or contribute,
and GitHub could not detect a licence at all.
Adds the AGPL-3.0 text — which the licence itself requires be
distributed with the source — and corrects the composer declaration to
AGPL-3.0-or-later.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Snooze is archive-with-a-timer, expressed in labels like everything else
— Inbox off, Snoozed on — so the "a message carries at least one label"
invariant holds without a second mechanism, and the snoozed pile is
reachable by every path that already works on labels.
The web endpoint was the broken half. POST /status/thread/{id}/snooze
wrote snoozedUntil and nothing else: no labels moved and nothing
propagated, so the conversation stayed in the Inbox both locally and at
the provider while its row vanished from the list — and then the sweep
"woke" a thread that had never left. Both callers now go through
ThreadSnoozeService, so a snooze means the same thing whichever client
set it. ThreadSnoozeEndpointTest asserts on labels rather than on the
column, because the column was always written correctly and a test that
checked it would have passed against the broken implementation.
The UI was missing outright. The row button called an action whose
`until` defaulted to null and which Stimulus calls with the event alone,
so it cleared a snooze instead of setting one; the toolbar hardcoded
"tomorrow 8am" behind a comment asking for a picker. Both now open a
menu. The wake times are computed in the browser, deliberately: the
server has no timezone for the session, so "tomorrow morning" resolved
server-side would mean tomorrow morning wherever the container thinks it
is. "Later today" drops itself once the evening has passed rather than
resolving to a time already gone.
The Snoozed pile also had nowhere to live — no route and no sidebar
entry, so it was unreachable. Both added, gated on there being something
snoozed, on the same reasoning as Archive.
Thread/set is a plMail extension; RFC 8621's Thread is read-only. It
accepts exactly one property and refuses create and destroy outright:
threads come into being by mail arriving, and a client that could conjure
one would be describing something nothing else understands. Unlike the
web endpoint it refuses an unparseable date rather than substituting
"in 1 day" — for an API, a client told its write succeeded would find the
thread back at a time it never chose.
app:mail:wake-snoozed runs every minute, which is the unit people pick a
wake time in, and costs one indexed query when nothing is due.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
RFC 8621 has a client upload bytes to /jmap/upload and then name the
returned blobId in Email.attachments. That half was missing: drafts
created over JMAP silently dropped whatever the client attached.
The bytes are copied into attachment storage rather than referenced in
place. An UploadedBlob is scratch space that PruneBlobsCommand reclaims
on a timer, so a draft pointing at one would lose its files days later
with nothing to say why.
Resolution goes through BlobResolver, which filters by account, so a
blobId belonging to another account or another user resolves to null and
is refused. "Expired", "never existed" and "belongs to someone else" all
give the same answer deliberately — distinguishing them would tell a
caller which blob ids are real. Filenames are basename()d, so a client
sending "../../etc/passwd" cannot steer where anything is written.
Runs after the persisting flush, not before: the storage path is
bucketed by message id and a draft has none until it is persisted. The
web composer forces a save before it accepts an upload for the same
reason.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
EmailPatchApplier read a message's current mailboxIds straight off its
labels, but message_label stores user-scoped label ids while a JMAP
Mailbox id is a per-account binding id — which is what the incoming patch
keys are and what the resolution below expects.
Both are autoincrement ints from different tables, so the two spaces
overlap: a patch removing one mailbox left the others behind as ids from
the wrong space, and the resolution rejected them as "No such Mailbox".
The failure reads like a client sending nonsense rather than like a
translation that was skipped.
Now translated through LabelBindingRepository::bindingIdsByLabelId(),
the same direction EmailMapper already translates on the way out.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
