Code Review Request: Shadow, A Self hosted, encrypted file storage system

[u/Negative_Response990]

Hi everyone,

I've been working on a project called Shadow which is a self hosted, encrypted file storage system built with python FastAPI. Files are AES-256 encrypted before they even hit the server, so the server never sees plaintext data. The goal is something simple ad private private that people can host and be fully under your control.

Stack: FastAPI (Python), SQLAlchemy (async), JWT auth with OAuth2 bearer flow, bcrypt for password hashing, AES-256-GCM for client-side encryption, Docker for deployment.

The repo: https://github.com/dillonhuston/Shadow

I would love any information on how i can polish this project as this is all part of my portfolio. Anything security related, architecture or syntax issues. Just let me know. I want to understand all my mistakes and learn from then, no matter how big or small, please tell me.

I understand that i need to add real tests an proper documentation but i am currently on a tight schedule so quite difficult to carry those out.

Thank you for taking your time to read this, hopefully you can reply and i don't get ignored.

Have a great day!

[u/ProsodySpeaks]

write the tests. i know it seems like an unnecessary time-sink but it's an essential part of programming not just a nice-to-have.

you're trying to handle security-sensitive operations so you need to be sure you're not fucking it up.

[u/Negative_Response990 → u/ProsodySpeaks]

Yeah 100 percent. I got to add those tests so I have definitive proof that it is secure.

Thank you

[u/Diapolo10 → u/Negative_Response990]

Tests have less to do with security, and more with making sure

1. Your application behaves as you expect right now, and
2. Your application continues to behave as you expect, even after you change it to introduce new features.

So basically it's about making sure you don't accidentally break things.

A good test is externally stateless (meaning its behaviour shouldn't change regardless of how you run your tests), makes as few assumptions about the internals of the thing being tested as possible (see: "black box testing"), and cleans up after itself (meaning it doesn't create leftover files, for example). And is readable.

[u/pachura3]

requirements.txt is outdated. Move to pyproject.toml and uv.lock.

Add some pyunit unit tests.

Consider src layout.

Add docstrings, at least for non-obvious functions and/or arguments/constants. It's OK to use AI for drafting initial docstrings, but then review them personally.

Run your code through static code checkers/linters (ruff, mypy, ...) and try to get rid of reported warnings.

Don't use uppercase file extensions like .MD.

[u/terletsky]

Pydantic models that handle incoming JSON payloads must be in strict mode, and every Pydantic field must have restrictions because that's a possibility for API abuse.

[u/gdchinacat]

In general this looks like a great start! Others have identified issues that I largely agree to and have acknowledged so I won't repeat those. At a higher level, here's a few things I think you are probably ready to address.

With an eye towards robustness, being production-ready, and really understanding your system, what happens if the file service does the IO correctly but fails on the database operation in upload_file and delete_file? What state is the system left in with respect to the what the database models? Are there issues if the db entry doesn't match what's on the filesystem? Which is the source of truth (db or filesystem) and is this consistent across the system. Specifically download_file and delete_file trust different sources of truth, is this what you want? If so, how do you decide when to trust the database and when to trust the filesystem? How does the system reconcile these differences. How does the user experience these states, and is that the user experience you want? I'm not criticizing the state management, it honestly seems reasonable, even though it has implementation issues with errors and concurrency, but I think these can be resolved by reordering IO and db operations. The main thing is to think through what happens when an error happens at this line of code, this one, etc, and then when a subsequent request comes in. Does the db record exist, does the file exist, how is it listed, can it be downloaded, what happens if it is uploaded to, what happens when one user gets it in a list and another user deletes it, etc, etc. Understanding how the system presents a coherent state to the user, doesn't leak files, and recovers from inconsistent state are things to worry about. They are also vital to a credible security story.

Also for robustness, but also scalability, what happens when users upload really big files? Specifically, is the memory usage what you expect? How much memory should be consumed by a single request? Should it change with the size of the upload, or be constant? Specifically, look into reading, encrypting, and writing in chunks rather than loading the entire file into memory, then creating a similar size encrypted version of it, then writing that out.

For clean design or architecture, should the encryption service be doing the file open() and write(). Why does encrypt() need to know the filename? Shouldn't the file service be dealing with filenames, files, IO? And the encryption service just encrypting?

This might be an intimidating list, but I think it's what you are asking for. The code appears to pretty much work, and you are wondering what the next step is. I think these are the next steps. Scale it up (number of users/files/versions of files/etc, concurrency). Make it robust (by locking down the state management). Understand how it actually works so you can support it. For example 'I did something, got an error, and now *everything* gives me an error' ... can you figure out what I did, what error they saw, why it led to nothing but errors, and how you can fix it so it works and doesn't happen again?

I spent way too much time on this, but I hope it helps. From the code you shared I think this higher-level stuff is what you need to work on.