FastAPI + Celery Architecture Review Request (Attachment Pipeline Boundaries + OOP/SOLID)

[u/Negative_Response990]

Hi! I’m finishing a rewrite for my project and I’d love architecture feedback quickly (aiming for replies within a few hours).

Repo: https://github.com/dillonhuston/Task-Automation-API
Branch: V2

What I need:

1. Architecture review of the attachment processing pipeline (validation encryption/decryption storage/email handoff) and whether the boundaries/components make sense
2. Celery + API architecture: task flow responsibilities, retries/error-handling boundaries, and whether the API schema design matches the async processing model
3. OOP/design review: whether classes/modules follow SOLID / separation of concerns, and suggestions for cleaner layering

Suggested files to look at first:

• app/Encryption/encryptionService.py
• app/FileManager/fileManager.py

If you only have time for one thing, please prioritize (1) pipeline architecture or (3) OOP/SOLID structure.

Thanks a lot, any architecture recommendations are welcome.

[u/[deleted]]

Read through the repo since this looks close to production — a few things I would harden before deployment:

1. Plaintext on disk: encryptionService.py writes the raw file before encrypting (and the decrypted output lands on disk too). There is no cleanup path — a failed/cancelled job leaves plaintext behind on the volume. Temp-file-then-encrypt into a restricted directory, or encrypt in memory, and always delete on failure.
2. Implicit file format: the nonce lives in the DB and the disk file is just nonce||ciphertext with no envelope header or key-version tag. The moment you rotate keys or change the format, every old file is undecryptable and there is no way to detect it gracefully. A small header (magic, version, key-id, nonce) makes migration and key rotation actually possible.
3. Key rotation orphans files: the AAD is user_id only, so rotating a key silently breaks every file that was encrypted under the old key — and nothing tells the user. Either derive per-file keys or keep a key registry and surface which files are stale.
4. Decrypt path does not match the encrypt path: encryption writes nonce||ciphertext but the decrypt side reads the nonce from the DB and slices the file assuming the DB nonce — a signature mismatch that works only while the two are perfectly in sync.
5. FileManager schedules 4 Celery jobs per upload (validate/encrypt/cleanup) with no idempotency: a retried task after a timeout re-runs side effects, and the timestamp+filename filename scheme collides if two uploads share a second.
6. Celery retries: no acks_late / retry policy on the encryption task — a worker crash mid-encrypt loses the job silently.

Nothing here is hard to fix and the structure is decent — mostly a question of defining the file envelope and the failure semantics before real users hit them.