DEV Community

Elwin Ernst
Elwin Ernst

Posted on

Harden for the reader: three red flags an AppSec engineer hits on `git clone`

#softwareengineering #cybersecurity #github #security

There's a gap between software that works and software that survives being read by a security engineer. A stack can pass every test, boot cleanly, and demo flawlessly while quietly carrying the exact patterns a reviewer is trained to grep for in the first five minutes. For a security product especially, the reader is the user — the person deciding whether to trust you is going to open the repo, not just click the buttons.

So I did a pass over the suite with one question: if an AppSec engineer cloned this, what would make them wince? Three things did. None broke anything at runtime. All three are the kind of finding that turns "interesting" into "no thanks" in a hiring or procurement read. Here's each red flag and how it was closed.

Full code: ElwinErnst/sytadel-suite (auth-api, billing-api, vault-api, and the compose at the root).

Red flag 1 — synchronize: true

TypeORM's synchronize auto-derives the database schema from your entities on every boot. It's wonderful for the first week and a liability forever after: no migration history, no review of schema changes, and one renamed column away from data loss. Seeing it on in a repo says "nobody owns the schema." Worse, the default here was on:

// before — auth-api
synchronize: String(process.env.DB_SYNC ?? 'true') === 'true',
Enter fullscreen mode Exit fullscreen mode

The fix is to make migrations the source of truth. DB_SYNC now defaults to false, and the app applies migrations on boot:

// after
synchronize: String(process.env.DB_SYNC ?? 'false') === 'true',
migrations: [join(__dirname, 'database', 'migrations', '*.js')],
migrationsRun: true,
Enter fullscreen mode Exit fullscreen mode

Generating the baseline honestly matters: I ran migration:generate against an empty throwaway Postgres so the diff was the entire current schema (12 tables for auth, 4 for billing), then actually applied it to confirm it stands up. The data-source.ts used for the CLI globs every *.entity.ts — the old one listed a stale subset of four entities, which would have generated an incomplete baseline and silently missed tables.

Bonus find while I was in there. The DB module logged its own config on boot:

console.log('DB CONFIG =>', db);   // ← db includes the password. deleted.
Enter fullscreen mode Exit fullscreen mode

A credential in stdout is a credential in your log aggregator, your CI output, and anywhere those get shipped. Gone.

Red flag 2 — change-me-* secrets hardcoded in compose

Eighteen occurrences of six distinct secrets, all reading like this:

AUTH_JWT_ACCESS_SECRET: change-me-access-secret
Enter fullscreen mode Exit fullscreen mode

Two problems in one line. First, secrets living in a committed file with no way to override them — you can't inject real values without editing the repo. Second, they're shared: the same JWT secret is used by auth to sign and by zerotrust + billing to verify, copy-pasted across three service blocks, which means they can drift out of sync during an edit and break cross-service auth in a way that's miserable to debug.

The fix externalizes each logical secret to a single interpolation variable, reused everywhere it's shared:

# auth signs, zerotrust + billing verify — all reference the ONE variable,
# so they can't drift. Real value comes from a gitignored root .env;
# local/CI fall back to a clearly dev-only default.
AUTH_JWT_ACCESS_SECRET: ${JWT_ACCESS_SECRET:-dev-insecure-access-secret}
ZT_JWT_HS256_SECRET:    ${JWT_ACCESS_SECRET:-dev-insecure-access-secret}
BILLING_JWT_ACCESS_SECRET: ${JWT_ACCESS_SECRET:-dev-insecure-access-secret}
Enter fullscreen mode Exit fullscreen mode

The dev-insecure-* default keeps docker compose up and CI working with zero setup — but the name shouts that it's not for production. For production, a separate overlay removes the fallbacks entirely and fails closed:

# docker-compose.prod.yml — no defaults; missing secret ⇒ compose refuses to start
AUTH_JWT_ACCESS_SECRET: ${JWT_ACCESS_SECRET:?set JWT_ACCESS_SECRET in .env for production}
Enter fullscreen mode Exit fullscreen mode 
docker compose -f docker-compose.yml -f docker-compose.prod.yml up -d
# error: required variable JWT_ACCESS_SECRET is missing a value
Enter fullscreen mode Exit fullscreen mode

Because auth-api references all six shared secrets, requiring them in that one service block gates the entire stack at docker compose config time — nothing starts on placeholder secrets. Now the reader sees: secrets are injectable, shared ones can't drift, and prod can't accidentally boot insecure.

Red flag 3 — anti-replay in an in-memory Map

The internal service-to-service calls (and vault's Zero Trust requests) are HMAC-signed with a nonce, and replays are rejected by remembering nonces for a short window. The remembering was a per-process Map:

private readonly replayCache = new Map<string, number>();
Enter fullscreen mode Exit fullscreen mode

Correct for exactly one instance. Restart, and the memory is gone; scale to two instances, and a replay hitting the other instance sails through. For a Zero Trust product, "replay protection that only works single-process" is precisely the gap a reviewer probes.

Moved it to a replay_nonces table with an atomic check-and-record — a unique key plus insert-on-conflict, which is both correct across instances and free of the check-then-set race the Map had:

const result = await this.repo
  .createQueryBuilder()
  .insert().into(ReplayNonce)
  .values({ key, expiresAt })
  .orIgnore()                 // ON CONFLICT DO NOTHING
  .returning('key')
  .execute();
const inserted = result.raw as unknown[];
return Array.isArray(inserted) && inserted.length > 0;   // false ⇒ replay
Enter fullscreen mode Exit fullscreen mode

The guard verifies the signature first and only then records the nonce — no polluting the store with unverified requests — and a per-minute job prunes expired rows. I chose Postgres over Redis deliberately: every service already runs Postgres, so a new stateful dependency for low-volume internal calls would be its own thing to justify to that same reviewer. Right-size the infra to the access pattern.

The through-line

None of these changed a feature. They changed what the code says about itself to someone qualified to read it. That framing — harden for the reader, not just the runtime — is a useful lens because it aligns with the actual threat model of a security product: the people evaluating you will read the source, and "it works" is table stakes, not the bar.

A few honest gotchas from doing it:

  • Docker died mid-migration. With no daemon to generate against, I hand-wrote one migration mirroring the generated style and let CI validate it on a fresh DB — the smoke boots the whole stack, applies migrations, and gates on a real login, so a broken migration fails there.
  • npm install in a yarn repo. Reflex cost me: it created a bogus package-lock.json and rewrote every URL in yarn.lock. Soft-reset, put the dep in package.json only, let the Dockerfile's non-frozen yarn install reconcile it. Use the repo's package manager, always.
  • Init-script ordering + grants (vault). Vault owns its schema through numbered SQL init scripts, not migrations. The new table had to be numbered before the runtime-role grant script so it's covered by GRANT … ON ALL TABLES, and deliberately left out of the append-only revokes since the app prunes it.

Every change above shipped the same way — branch → PR → the integration smoke on a fresh stack → merge — because the point of hardening you can't demonstrate is nothing. If a reviewer can't see it holds, it may as well not.

Source

ElwinErnst/sytadel-suite: migrations in auth-api/billing-api src/database/migrations/, secret interpolation in docker-compose.yml + docker-compose.prod.yml, and the persistent replay store in each service's common/replay/ (vault via infra/postgres/init/058_replay_nonces.sql). Related: the MCP server and agentic HITL access.

Top comments (0)