The first fix in this review made the review itself possible. Program.cs was checking a role claim called "role"; the code that actually issues tokens had been issuing ClaimTypes.Role the whole time. The two strings never matched, so every admin-only endpoint had been rejecting real admins with a 403 — including, most likely, whoever was trying to get into the admin surfaces to review them in the first place. The fix was one line. Finding it wasn't the point of the exercise, but it's a fair summary of the exercise's mood.
The thing that was there from the start
The second commit of this entire project, back in March, set a default JWT signing key directly in docker-compose.yml: a literal string, MockEvalio-SuperSecretKey-AtLeast32Chars!!. It was never meant to be permanent — the kind of placeholder every project has in its second commit, before there's a real secrets story. Today, checking four files against that string, it was still there. appsettings.json, Program.cs, IdentityService.cs, the same docker-compose.yml — all still willing to fall back to it if a real key wasn't configured.
The fix removes the fallback rather than replacing it. The key is now named KnownLeakedDevKey — leaked isn't a hedge, it's the accurate word for a string that's been sitting in git history for five months — and it's only permitted in Development or Testing. In any other environment, if the configured key is missing or equal to that known value, the application now refuses to start. docker-compose.yml's default changed too: no more fallback value, just the environment variable or nothing, with a comment explaining why nothing is the safer default. Production, as far as the deploy configuration shows, was already supplying its own key by this point — so the fix closes a door that appears not to have been open in production, but was open everywhere else for as long as the project has existed.
The other four in the same commit, briefly
A resume cache keyed only by resume ID and job description ID skipped the ownership check on a cache hit — anyone who knew another user's resume ID could read their cached score. Three interview-generation paths trusted a client-supplied job description ID with no check that the caller actually owned it; one of them returned another user's job-description-derived content directly in the response. And two of the highest-cost AI flows had never been wired into the rate limiter the rest of the product uses, meaning nothing capped their spend. All three now route ownership checks through the same service the rest of the codebase already used for this.
A bug the tests found while being written
Seventeen minutes after the first fix, while writing a test to confirm a real key wouldn't trigger the new startup guard, a second bug surfaced. appsettings.json had Jwt:Key set to an empty string rather than left absent, and ?? only falls through on null — not on empty string. The fallback that was supposed to catch a missing key had never actually been reachable in that config. Fixed by checking for blank explicitly instead of relying on null-coalescing to catch it. It's a small thing next to the rest of the day, but it's the clearest example in this whole log of a fix generating its own second fix, just by being tested properly.
An hour later, the structural pass
Just under an hour after that, the bare "you're an admin or you're not" check across thirteen admin controllers was replaced with named, per-action permissions — a real answer to "which admins can do what," where before there was only one tier. Two more of the same ownership gap from earlier in the day turned up and got closed the same way. Audit logging, which had quietly been skipped in three controllers, got wired in, and a new endpoint made that trail readable for the first time. The same commit added the project's first written description of its own bounded contexts — a document about how the codebase is organized, landing on the same day as a review of what was and wasn't protected inside it.
What I don't know
I don't know what specifically prompted looking at this on this particular day rather than any other — nothing in the repository points to an external trigger, and I'm not going to imply one that isn't there. And I don't know whether this is everything. A review finds what it's scoped to look for; it doesn't come with a guarantee that nothing else is out there. What I can say is narrower and, I think, more honest: eight real gaps, each one fixed with a test proven to fail without the fix, and one of them a secret that had been sitting in the project since its second commit, waiting for someone to go looking.
Top comments (0)