A live luxury-resale operation running two applications, a customer storefront and an internal admin panel, off four repositories, two backend codebases forked from a common ancestor, and one shared pair of databases. Static security review of code and configuration, commissioned ahead of a three-programme feature build that would add paid memberships, public profiles and confidential inventory to the same surfaces.
- Surface audited
- 4 repos · 4 backend surfaces · ~1,200 route registrations
- Effort
- ~6 hours, discovery to handoff
- Findings
- 2 critical, 4 high
- Deadline
- None: routine review ahead of the new build
The situation
Two products share one set of databases. The storefront calls its own backend for catalogue, orders and resale submissions, and calls the admin backend for authentication and two-factor, so the admin backend is the identity provider for customers as well as staff. Both backends run two database servers, giving four independently addressable API surfaces reading and writing the same data.
The two backend repositories were forked from a common ancestor. Files with identical paths exist in both with materially different implementations: one controller alone is 1,796 lines on one side and 1,020 on the other. That divergence was documented internally as a maintenance hazard. This review treated it as a security boundary and diffed the pairs for missing checks rather than missing features.
Where the exposure actually is
The constraint is the fork. Two backends, one database, checks that must hold identically on both sides but are maintained separately.
The surprise is that the fix made it worse. At some point the admin backend's auth was consolidated into a hardened shared module, with session invalidation, token refresh and login rate limiting. It is genuinely better code. But to stay compatible with tokens already in circulation, it kept the original development secret as a permanent fallback, and the fallback is not conditional on anything.
The verification routine builds a list of candidate keys, filters out the empty ones, and tries each in turn until one verifies. The development secret is in that list, defined a few lines above it, and is never dropped in any environment. The storefront's second backend reaches the same outcome by a separate route, hardcoding the identical string into an equivalent list.
The practical consequence is that a token claiming administrator status, signed with a string readable in the repository, passes verification on three of the four surfaces. The session-invalidation layer does not help: it compares the token's issue time against a stored revocation point, and a forged token carries a current issue time by construction.
Everything else sits downstream of that. Route guards, ownership checks and the diverged-controller differences all assume the identity layer holds. While this secret is accepted, they are defence in depth around an open door.
What we found
Ordered by severity. The criticals set the sequence below.
- Critical
A committed secret is accepted as a valid signing key on three of four API surfaces
Anyone who can read the source can mint an administrator token. Authentication does not fail. It succeeds.
- Critical
Unauthenticated bulk write into the products collection
No auth middleware, no validation, no ownership check. The caller supplies both the document filters and the values, straight into a bulk write.
- High
Any logged-in customer can read every reseller's price-change history
The route is guarded by the customer middleware rather than the admin one, and the query has no ownership filter at all. Present identically in both repositories.
- High
The public product endpoint returns the entire document with no field projection
Authentication was deliberately commented out on the line above the route. Every internal field ships to anonymous callers. Today that includes resale linkage; after the planned build it would include whatever ties an item to the business that consigned it.
- High
Database TLS is silently disabled whenever the environment variable is unset
The local-development branch is selected by absence rather than by an explicit value, so a deploy that simply forgets the variable connects in cleartext and reports nothing. The same file hardcodes fallback database credentials.
- High
The storefront's admin middleware never checks whether the account is blocked
Its customer middleware queries the account on every request and rejects blocked users. The admin middleware alongside it verifies the token and stops. Revoking a staff account leaves their admin token working until it expires.
Constraints and trade-offs
This was a static review. Production environment variables, live cache exposure and the database role's actual grants could not be verified from source, so three of the discovery questions are answered only as far as the code allows. We can show that the TLS branch fails open when the variable is unset, but not whether it is unset in production. Those need a session against the running environment.
The shared schema was not re-derived migration by migration. Two repositories hold overlapping, diverged migration sets against one database, and establishing which constraints actually exist now requires inspecting the live schema. Reasonable to defer, but it means the question of whether one repository dropped a constraint the other relies on is open rather than cleared.
Two findings from the previous review had already been fixed and were removed from this report rather than carried forward. Reporting them again would have inflated the count.
Sequencing
Remove the legacy secret from both verification lists (half a day)
This is the whole exposure. Everything else is incremental once the identity layer actually holds. It invalidates any token still signed with it, so it needs a deliberate rollout: confirm nothing in circulation depends on it, then drop it and rotate.
Put auth on the bulk-write endpoint, or delete it (under an hour)
Establish whether any client still calls it. If nothing does, removing it is safer than guarding it.
Fix the two authorisation gaps (half a day)
The price-change list needs the admin guard and an ownership filter. The admin middleware needs the same block check its sibling already performs.
Make the TLS branch explicit (an hour)
Select the local path on an affirmative value, never on absence, and fail loudly when the variable is missing rather than quietly dropping TLS.
Project the public product response (half a day)
Return a named field list rather than the whole document. Worth doing before the confidential-inventory build rather than after, since that programme's confidentiality promise depends on this endpoint.
Runtime verification session (half a day)
Closes the three questions this review could not answer from source.
Outcome
The right-hand column is the projected end state once the sequencing is worked through, not a measured result. About two and a half days of work, of which the single highest-value half-day is the first item. Both criticals sit in the identity layer rather than spread across the application, so one focused change closes the majority of the real exposure without a refactor.
| Metric | Before | After |
|---|---|---|
| API surfaces accepting the committed secret | 3 of 4 | 0 of 4 |
| Critical findings open | 2 | 0 |
| High findings open | 4 | 0 |
| Discovery questions answerable from source | 8 of 11 | 11 of 11 |
Build with
confidence.
AI builds the product. Vibecop makes sure it won’t break in production, fail under scale, or expose your users to risk. One audit. Fewer expensive surprises.