Work through the 2026-08-03 review findings

[tomas-kulhanek]

Works through every finding of the 2026-08-03 deep review of v1.0.0. That review passed the code
and the test suite but failed the repository on release safety: the documentation promised stronger
behaviour than the implementation delivers, and nothing warned about the coordination the new
upstream cipher text format demands.

Each finding is its own commit, so any of them can be reverted alone.

Release safety

F1 — coordinating the upgrade to spaze/encryption 3.0. Version 3 writes a marked cipher text
format that earlier versions cannot read. When several deployments share the same data, one of them
upgrading first and writing a single value hands the others data they cannot decrypt. README said
nothing about it. New Upgrading section spelling out the order, and a CHANGELOG.md so the next
upstream format change lands somewhere a reader will find it.

Documentation that promised more than the code delivers

No behaviour changed by these, but code written against the old wording may have rested on a
guarantee that was never there.

F2 — when a misconfigured group actually fails. The intro and the exception table said a
misconfigured group fails while the container is compiled. Only the shape of the configuration and
type errors do; key material is validated when the encryption service is created, because resolving
%env()% at compile time would write the keys into var/cache/. The exception is now documented at
both moments it is thrown at.

F3 — needsReEncryption() is a structural check. The PHPDoc promised a DecryptionException
whenever the value is not cipher text of this group. Upstream only parses the envelope, checks the
marker and compares the key id with the active one — it verifies neither that the key id is
configured nor that the payload authenticates, so a structurally valid value with an unknown key id
reports true. Migration code could have read that as proof of ownership. Contract rewritten and
pinned down by a test in all three implementations, since it lives on the shared Encryptor
interface.

F6 — key id authentication, split by format. README said the key id travels unauthenticated.
Since upstream 3.0 that holds only for anonymous_asymmetric and for values without a marker; the
new symmetric and asymmetric format binds the key id and the marker into what decryption
verifies. Now a table per format and type.

Prefix _. The keygen section read as if a prefix containing _ were invalid configuration.
Verified at runtime that upstream accepts it — the ban is the command's own rule so its output stays
readable in one direction.

Behaviour

F4 — reject an explicitly empty secret_key ⚠️ breaking. An anonymous_asymmetric key carrying
secret_key: '' was treated as no secret key at all, so the whole group was quietly registered
write-only. A config generator emitting an empty value could therefore take away the ability to
decrypt, and nothing would notice until something asked that group for a Decryptor — possibly in a
different deployment than the one that wrote the data. Leave secret_key out to get a write-only
group; an empty value now throws while the container is compiled.

Tooling

F7a — PHP 8.5 in CI. composer.json requires php >=8.4 but every workflow pinned 8.4 alone.
Both the push/PR run and the nightly one are now a matrix over 8.4 and 8.5.

F7b — a deprecation policy that actually holds. phpunit.xml set
SYMFONY_DEPRECATIONS_HELPER=disabled, which read as a deliberate opt-out but was inert.
symfony/phpunit-bridge was never installed — and would not have helped: its bootstrap.php returns
early for PHPUnit >= 10 and DeprecationErrorHandler::register() is reached from nowhere else.
Verified by running a direct deprecation through the bridge with max[direct]=0 and through the
bridge wired as a SymfonyExtension: it passed either way. Replaced with what PHPUnit 12 has
natively, and verified the other way round — a trigger_deprecation() call in src/ now exits 1.

F8 — minimal permissions. ci and nightly declare contents: read; the assign-author workflow
gets exactly the two scopes its API call needs (it calls issues.addAssignees against a pull
request) and no contents access, since it never checks the repository out. No SHA pinning, by
decision.

F5 — stale Dependabot note. The comment still claimed spaze/encryption comes from the
helppc/encryption fork over VCS. Since c33f255 the opposite is true, and leaving the note would
invite someone to bring the forbidden fork dependency back.

F9 — dependency footprint and test hygiene. No production class references
symfony/framework-bundle; moved to require-dev, with symfony/filesystem and
symfony/error-handler declared there explicitly instead of arriving transitively. Verified by
installing with --no-dev in a scratch copy and reflecting every class under src/ — all fifteen
resolve without the framework. The YAML-configured peer group now does a real AAD round trip instead
of asserting the cipher text is not the plain text, the passing integration tests clean the kernel up
in finally like the failing ones already did, and the andstor/file-existence-action steps are
gone — they produced an output nothing read.

F10 — the cryptography rule. AGENTS.md said all cryptography goes through spaze/encryption
while the keygen command calls random_bytes() and the libsodium keypair functions directly. Rule
now says what it means: no primitives, no schemes, all encryption and decryption through upstream,
key generation through the stable PHP libsodium API.

Verification

phpcs, phpstan level 10, rector:check, phpunit (84 tests / 177 assertions),
composer validate --strict, composer audit, check-platform-reqs and actionlint all pass.

A throwaway runtime smoke driver covered all three modes, AAD mismatch, malformed and truncated
input, wrong key and wrong recipient, key rotation, the write-only boundary, and the new F2/F3/F4
behaviour: 33 assertions, 0 failures.

Out of scope

The bundle extends Symfony\Component\HttpKernel\Bundle\AbstractBundle, which inherits from
HttpKernel\Bundle\Bundle implementing HttpKernel\Bundle\BundleInterface — @deprecated since
Symfony 8.1. The replacement is Symfony\Component\DependencyInjection\Kernel\AbstractBundle, and
Kernel::registerBundles() wraps such a bundle in a BundleAdapter, so it works in a normal app;
migrating would let symfony/http-kernel drop out of require entirely. Nothing emits a runtime
deprecation today, so it blocks nothing. Deliberately left for its own change.

🤖 Generated with Claude Code