A real report, exactly as delivered.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
The payments table declares card_number, card_cvv and card_expiry as plain text, and paymentService.charge inserts those values directly. Storing CVV after authorization is prohibited under PCI DSS under any circumstances, and storing PAN in cleartext pulls the entire database, its backups and anything with DB credentials into card-data scope. The public-read backups bucket in the Terraform makes the blast radius worse.

Reviewed: badapp/db/migrations/0002_add_payments.sql (lines 1-16) — quoted from the scanned source
-- Payments. Storing PAN + CVV is the point: an architecture/compliance finding that no
-- SAST rule will flag, because nothing here is syntactically wrong.
CREATE TABLE IF NOT EXISTS "payments" (
  "id"           uuid PRIMARY KEY DEFAULT gen_random_uuid(),
  "order_id"     uuid NOT NULL REFERENCES "orders"("id"),
  "amount_cents" integer NOT NULL,
  "currency"     varchar(3) NOT NULL DEFAULT 'usd',
  "card_number"  text NOT NULL,
  "card_cvv"     text NOT NULL,
  "card_expiry"  text NOT NULL
);

CREATE UNIQUE INDEX "payments_order_idx" ON "payments" ("order_id");

ALTER TABLE "orders" ADD COLUMN "paid_at" timestamptz;

Suggested remediation:
Stop persisting card data: delegate capture to a payment processor and store only an opaque token plus last four digits and brand. Drop card_cvv immediately and plan a migration to remove card_number.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
aws_s3_bucket.backups sets acl = "public-read". Given the schema stores plaintext card data and password hashes, a world-readable backup bucket exposes the entire customer dataset to anyone who can enumerate or guess the object keys. There is no block-public-access configuration alongside it.

Reviewed: badapp/infra/main.tf (lines 1-14) — quoted from the scanned source
resource "aws_s3_bucket" "backups" {
  bucket = "badapp-backups"
  acl    = "public-read"
}

resource "aws_security_group" "web" {
  ingress {
    from_port   = 0
    to_port     = 65535
    protocol    = "tcp"
    cidr_blocks = ["0.0.0.0/0"]
  }
}

Suggested remediation:
Remove the public-read ACL, set the bucket to private, and add an aws_s3_bucket_public_access_block with all four flags enabled plus server-side encryption.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
app.js mounts adminRoutes at /admin with no auth middleware, and admin.js defines no per-route guard. The routes behind it include a shell-executing /ping and a /refund that writes payment rows. The users table has an is_admin column and the project depends on jsonwebtoken, so an authorization concept exists in the data model but is never enforced in the request path.

Reviewed: badapp/src/app.js (lines 1-15) — quoted from the scanned source
const express = require("express");
const publicRoutes = require("./routes/public");
const userRoutes = require("./routes/users");
const adminRoutes = require("./routes/admin");
const { log } = require("./lib/logger");

const app = express();

app.use("/", publicRoutes);
app.use("/", userRoutes);
// ARCHITECTURE SMELL: admin surface mounted with no authentication middleware at all.
app.use("/admin", adminRoutes);

app.listen(3000, () => log("listening on 3000"));

Suggested remediation:
Introduce an authentication middleware (verify a signed token, load the user, require is_admin) and apply it at the app.use('/admin', ...) mount point before any admin handler is registered.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
findUser, findOrders and findReviews interpolate their arguments into SQL with + and no escaping, and they are fed directly from req.query.id / req.query.userId by the /user and /orders routes. The connection authenticates as the MySQL root user, so a successful injection has full database privileges including reads of the payments table.

Reviewed: badapp/src/db/queries.js (lines 1-13) — quoted from the scanned source
const { connect } = require("./client");

// SQL injection: string concatenation straight into the query.
exports.findUser = (id, cb) =>
  connect().query("SELECT * FROM users WHERE id = '" + id + "'", cb);

exports.findOrders = (userId, cb) =>
  connect().query("SELECT * FROM orders WHERE user_id = '" + userId + "'", cb);

// ARCHITECTURE SMELL: queries a `reviews` table that no migration creates (schema drift).
exports.findReviews = (productId, cb) =>
  connect().query("SELECT * FROM reviews WHERE product_id = '" + productId + "'", cb);

Suggested remediation:
Switch every query to parameter placeholders (mysql's query(sql, [params]) form) and forbid string-built SQL in the data layer via review or a lint rule.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
exec() runs its argument through a shell, and req.query.host is concatenated directly into it. A host value containing shell metacharacters runs additional commands in the container. Combined with the missing authentication on the /admin router, this is an unauthenticated shell.

Reviewed: badapp/src/routes/admin.js (lines 1-19) — quoted from the scanned source
const express = require("express");
const { exec } = require("child_process");
const paymentService = require("../services/paymentService");
const router = express.Router();

// NO AUTH ON ANY OF THESE. The architecture finding is the missing boundary, not the syntax.

// shell command built from user input -> command injection
router.get("/ping", (req, res) =>
  exec("ping -c 1 " + req.query.host, (err, stdout) => res.send(stdout)),
);

router.post("/refund", (req, res) =>
  paymentService.charge(req.query.orderId, { number: "[secret redacted]", cvv: "123", expiry: "12/30" },
    (e) => res.json({ ok: !e })),
);

module.exports = router;

Suggested remediation:
Replace exec with execFile('ping', ['-c','1', host]) and validate host against a strict hostname/IP pattern before use; then put the route behind authentication.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
The /calc handler evaluates req.query.expr with eval() and returns the result. Any caller controls the string that is evaluated in the Node process, which means arbitrary JavaScript execution with the privileges of the app (which, per the Dockerfile, runs as root). This is full remote code execution on the web tier, not an input-validation nit.

Reviewed: badapp/src/routes/users.js (lines 1-16) — quoted from the scanned source
const express = require("express");
const queries = require("../db/queries");
const orderService = require("../services/orderService");
const router = express.Router();

router.get("/user", (req, res) => queries.findUser(req.query.id, (e, rows) => res.json(rows)));

router.get("/orders", (req, res) =>
  orderService.ordersFor(req.query.userId, (e, rows) => res.json(rows)),
);

// eval on user input -> code injection
router.get("/calc", (req, res) => res.send(String(eval(req.query.expr))));

module.exports = router;

Suggested remediation:
Delete the /calc route. If expression evaluation is genuinely a product requirement, replace eval with a sandboxed, non-Turing-complete expression parser and an allowlisted grammar.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
worker/db.py concatenates a path into a shell command with shell=True, and run() calls eval() on its argument. Whatever feeds these functions becomes a code-execution channel into the worker container, which shares the same image and network as the web service.

Reviewed: badapp/worker/db.py (lines 1-32) — quoted from the scanned source
import os
import sqlite3
import subprocess

DSN = os.environ["DATABASE_URL"]


def lookup(user_id):
    conn = sqlite3.connect("app.db")
    # SQL injection: query built by string formatting
    return conn.execute("SELECT * FROM users WHERE id = '%s'" % user_id).fetchall()


def mark_paid(order_id):
    # ARCHITECTURE SMELL: a second, independent writer to `orders` in another language,
    # with no shared schema ownership and no migration of its own.
    conn = sqlite3.connect("app.db")
    conn.execute("UPDATE orders SET status = 'paid' WHERE id = '%s'" % order_id)


def backup(path):
    # command injection: shell=True with interpolated input
    subprocess.call("tar czf backup.tgz " + path, shell=True)


def run(code):
    # arbitrary code execution
    return eval(code)


API_KEY = "[secret redacted]"

Suggested remediation:
Replace subprocess.call("tar czf ... " + path, shell=True) with a list-argument call and shell=False, and delete run(); if dynamic behaviour is needed, dispatch through an explicit map of allowed operations.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
The scan reports testFileCount 0 across 24 files, and the CI workflow runs npm install && npm test as its sole step. With no tests present that step either fails or passes vacuously; either way the pipeline provides no signal, and there is nothing preventing regressions in the payment and query paths.

Reviewed: badapp/.github/workflows/ci.yml (lines 1-9) — quoted from the scanned source
name: ci
on: [push]
jobs:
  build:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      - run: npm install && npm test

Suggested remediation:
Add integration tests for the routes and the query layer (starting with /orders and paymentService), and make CI fail on missing tests rather than treating an empty suite as a pass.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
The migrations use uuid columns, gen_random_uuid(), timestamptz and quoted identifiers — PostgreSQL syntax — while the application uses the npm mysql driver against a mysql:5.7 container with a mysql:// DSN. These migrations cannot create the schema the running code queries, so the declared data model and the deployed one will diverge from day one.

Reviewed: badapp/db/migrations/0001_init.sql (lines 1-27) — quoted from the scanned source
-- Core schema. Deliberately stores card data in plaintext (design finding, not a SAST one).
CREATE TABLE IF NOT EXISTS "users" (
  "id"            uuid PRIMARY KEY DEFAULT gen_random_uuid(),
  "email"         text NOT NULL UNIQUE,
  "password_hash" text NOT NULL,
  "is_admin"      boolean NOT NULL DEFAULT false,
  "created_at"    timestamptz NOT NULL DEFAULT now()
);

CREATE TABLE IF NOT EXISTS "products" (
  "id"          uuid PRIMARY KEY DEFAULT gen_random_uuid(),
  "sku"         varchar(64) NOT NULL UNIQUE,
  "name"        text NOT NULL,
  "price_cents" integer NOT NULL,
  CONSTRAINT "products_price_positive" CHECK ("price_cents" > 0)
);

CREATE TABLE IF NOT EXISTS "orders" (
  "id"         uuid PRIMARY KEY DEFAULT gen_random_uuid(),
  "user_id"    uuid NOT NULL REFERENCES "users"("id"),
  "product_id" uuid NOT NULL REFERENCES "products"("id"),
  "quantity"   integer NOT NULL DEFAULT 1,
  "status"     text NOT NULL DEFAULT 'pending'
);

CREATE INDEX "orders_user_idx" ON "orders" ("user_id");

Suggested remediation:
Pick one engine. If MySQL is the target, rewrite the migrations in MySQL syntax (CHAR(36)/BINARY(16) ids, TIMESTAMP, backtick quoting) and run them in CI against the same image used in compose.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
aws_security_group.web has a single ingress rule covering ports 0-65535 from any source address. That exposes any service that happens to bind on the instance — database, admin ports, debug listeners — rather than just the application port, and removes the network layer as a compensating control for the auth gaps above.

Reviewed: badapp/infra/main.tf (lines 1-14) — quoted from the scanned source
resource "aws_s3_bucket" "backups" {
  bucket = "badapp-backups"
  acl    = "public-read"
}

resource "aws_security_group" "web" {
  ingress {
    from_port   = 0
    to_port     = 65535
    protocol    = "tcp"
    cidr_blocks = ["0.0.0.0/0"]
  }
}

Suggested remediation:
Narrow the ingress rule to the specific application port from the load balancer's security group, and add explicit rules only for ports that must be reachable.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
src/config.js exports awsAccessKeyId, awsSecretAccessKey, apiToken, adminPassword and jwtSecret as literals in a source file, and db/client.js consumes adminPassword as the database password. Whatever the current values are, this design puts credentials in version control and in every image layer, makes rotation a code deploy, and prevents per-environment separation. A source comment asserts the values are decoys; that claim cannot be verified from the code and should not be relied on.

Reviewed: badapp/src/config.js (lines 1-9) — quoted from the scanned source
// Hardcoded credentials — scanner bait, not real.
module.exports = {
  awsAccessKeyId: "[secret redacted]",
  awsSecretAccessKey: "[secret redacted]",
  apiToken: "[secret redacted]",
  adminPassword: "[secret redacted]",
  jwtSecret: "[secret redacted]",
};

Suggested remediation:
Change config.js to read from process.env (the repo already declares names such as AWS_ACCESS_KEY_ID, JWT_SIGNING_SECRET, DATABASE_URL), fail fast when a variable is missing, and rotate every value that has ever been committed.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
db/client.js hardcodes user "root" and passes config.adminPassword as the database password, and docker-compose sets DATABASE_URL to a root DSN. Any SQL injection therefore executes with full DDL and cross-database privileges, and one secret serves two unrelated trust boundaries.

Reviewed: badapp/src/db/client.js (lines 1-12) — quoted from the scanned source
const mysql = require("mysql");
const config = require("../config");

// ARCHITECTURE SMELL: a new connection per call, credentials from a checked-in module.
exports.connect = () =>
  mysql.createConnection({
    host: "db",
    user: "root",
    password: config.adminPassword,
    database: "badapp",
  });

Suggested remediation:
Create a dedicated application database user with DML-only grants on the badapp schema, give it its own secret supplied via the environment, and stop reusing adminPassword for the datastore.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
client.connect() calls mysql.createConnection on each invocation and no caller ends the connection. Under load this means a TCP handshake plus MySQL auth per request, and connections accumulate until the server's max_connections is hit, at which point all queries start failing. The mysql driver ships a pool API that is going unused.

Reviewed: badapp/src/db/client.js (lines 1-12) — quoted from the scanned source
const mysql = require("mysql");
const config = require("../config");

// ARCHITECTURE SMELL: a new connection per call, credentials from a checked-in module.
exports.connect = () =>
  mysql.createConnection({
    host: "db",
    user: "root",
    password: config.adminPassword,
    database: "badapp",
  });

Suggested remediation:
Replace createConnection with a single module-level mysql.createPool and have the query layer borrow and release connections, or call pool.query directly.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
The handler concatenates req.query.name into an HTML response and sends it with Express's default text/html content type, so attacker-supplied markup and script execute in the victim's browser on the application origin. There is no CSP or output-encoding layer anywhere in the app.

Reviewed: badapp/src/routes/public.js (lines 1-10) — quoted from the scanned source
const express = require("express");
const router = express.Router();

router.get("/health", (req, res) => res.json({ ok: true }));

// reflected user input -> XSS
router.get("/hello", (req, res) => res.send("<h1>Hello " + req.query.name + "</h1>"));

module.exports = router;

Suggested remediation:
HTML-escape the value before interpolation (or render via a template engine with auto-escaping) and add a restrictive Content-Security-Policy header.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
Both routes take the identifier straight from req.query and return the rows as JSON with no session, no ownership check and no field filtering. findUser does SELECT *, so password_hash and is_admin are serialised to the caller. This is an insecure direct object reference plus over-exposure of sensitive columns.

Reviewed: badapp/src/routes/users.js (lines 1-16) — quoted from the scanned source
const express = require("express");
const queries = require("../db/queries");
const orderService = require("../services/orderService");
const router = express.Router();

router.get("/user", (req, res) => queries.findUser(req.query.id, (e, rows) => res.json(rows)));

router.get("/orders", (req, res) =>
  orderService.ordersFor(req.query.userId, (e, rows) => res.json(rows)),
);

// eval on user input -> code injection
router.get("/calc", (req, res) => res.send(String(eval(req.query.expr))));

module.exports = router;

Suggested remediation:
Derive the acting user from an authenticated session/token, scope the query to that user (or require an admin role), and project only the columns the client needs instead of SELECT *.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
paymentService.charge builds an INSERT by concatenating orderId and the card number, CVV and expiry into the statement text. Besides the injection risk, string-built SQL containing PAN data tends to end up verbatim in driver error messages and query logs.

Reviewed: badapp/src/services/paymentService.js (lines 1-11) — quoted from the scanned source
const { connect } = require("../db/client");

// ARCHITECTURE SMELL: a service reaching past the query layer into the driver directly,
// AND persisting full card data (see 0002_add_payments.sql).
exports.charge = (orderId, card, cb) =>
  connect().query(
    "INSERT INTO payments (order_id, amount_cents, card_number, card_cvv, card_expiry) VALUES ('" +
      orderId + "', 100, '" + card.number + "', '" + card.cvv + "', '" + card.expiry + "')",
    cb,
  );

Suggested remediation:
Parameterise the insert, and route it through the shared query layer rather than calling connect().query directly from a service.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
lookup() and mark_paid() interpolate user_id and order_id into SQL strings with %s formatting. sqlite3 accepts parameters natively, so this is gratuitous injection exposure in a component that mutates order payment status.

Reviewed: badapp/worker/db.py (lines 1-32) — quoted from the scanned source
import os
import sqlite3
import subprocess

DSN = os.environ["DATABASE_URL"]


def lookup(user_id):
    conn = sqlite3.connect("app.db")
    # SQL injection: query built by string formatting
    return conn.execute("SELECT * FROM users WHERE id = '%s'" % user_id).fetchall()


def mark_paid(order_id):
    # ARCHITECTURE SMELL: a second, independent writer to `orders` in another language,
    # with no shared schema ownership and no migration of its own.
    conn = sqlite3.connect("app.db")
    conn.execute("UPDATE orders SET status = 'paid' WHERE id = '%s'" % order_id)


def backup(path):
    # command injection: shell=True with interpolated input
    subprocess.call("tar czf backup.tgz " + path, shell=True)


def run(code):
    # arbitrary code execution
    return eval(code)


API_KEY = "[secret redacted]"

Suggested remediation:
Use parameterised execute calls, e.g. conn.execute("SELECT * FROM users WHERE id = ?", (user_id,)), throughout worker/db.py.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
worker/db.py reads DATABASE_URL into DSN but then ignores it and connects to sqlite3 "app.db", updating orders.status to 'paid'. Two services claim ownership of the same logical table in two different databases, and the SQLite file is per-container and ephemeral, so payment status written by the worker is invisible to the web tier and lost on restart.

Reviewed: badapp/worker/db.py (lines 1-32) — quoted from the scanned source
import os
import sqlite3
import subprocess

DSN = os.environ["DATABASE_URL"]


def lookup(user_id):
    conn = sqlite3.connect("app.db")
    # SQL injection: query built by string formatting
    return conn.execute("SELECT * FROM users WHERE id = '%s'" % user_id).fetchall()


def mark_paid(order_id):
    # ARCHITECTURE SMELL: a second, independent writer to `orders` in another language,
    # with no shared schema ownership and no migration of its own.
    conn = sqlite3.connect("app.db")
    conn.execute("UPDATE orders SET status = 'paid' WHERE id = '%s'" % order_id)


def backup(path):
    # command injection: shell=True with interpolated input
    subprocess.call("tar czf backup.tgz " + path, shell=True)


def run(code):
    # arbitrary code execution
    return eval(code)


API_KEY = "[secret redacted]"

Suggested remediation:
Make the worker connect using DSN through the same engine as the web service, and give payment-status writes a single owner (either the worker or the app) with a migration that lives in db/migrations.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
The environment inventory includes ADMIN_BYPASS_TOKEN alongside JWT_SIGNING_SECRET, yet no code in the scanned files reads either one and no authentication middleware exists. A named bypass token is a standing backdoor pattern: if any deployed code path honours it, it sidesteps whatever authorization is eventually added.

Suggested remediation:
Confirm whether anything consumes ADMIN_BYPASS_TOKEN; if so, remove that code path and rely on the normal token [secret redacted] flow, and drop the variable from all environments.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
The workflow pins actions/checkout to a major tag and runs npm install, which resolves dependencies afresh and can mutate the lockfile, so CI is not reproducing the versions a deploy would use. The workflow also has no Node version setup step, so the runner's default Node decides the build, which may differ from the node:14 image in the Dockerfile.

Reviewed: badapp/.github/workflows/ci.yml (lines 1-9) — quoted from the scanned source
name: ci
on: [push]
jobs:
  build:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      - run: npm install && npm test

Suggested remediation:
Add actions/setup-node pinned to the same Node major as the image, switch to npm ci, and pin actions to a full commit SHA.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
The db service maps 3306:3306, which makes the database reachable from outside the compose network rather than only from web and worker. The root password is supplied inline in the compose environment block, so it lands in shell history, process listings and docker inspect output.

Reviewed: badapp/docker-compose.yml (lines 1-17) — quoted from the scanned source
services:
  web:
    build: .
    ports:
      - "3000:3000"
    environment:
      - DATABASE_URL=mysql://root:[secret redacted]@db:3306/badapp
  db:
    image: mysql:5.7
    ports:
      - "3306:3306"
    environment:
      - MYSQL_ROOT_PASSWORD=[secret redacted]
  worker:
    build: .
    command: python worker/db.py

Suggested remediation:
Remove the db ports mapping (service-to-service traffic already works over the compose network) and move MYSQL_ROOT_PASSWORD and DATABASE_URL into a secrets mechanism or an untracked env file.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
The Dockerfile does COPY . . with no .dockerignore evidence, then runs CMD as the default root user. That bakes local files — including any .env, git metadata and infrastructure code — into the image layers and gives a process that already has command-injection and eval sinks full root capability inside the container.

Reviewed: badapp/Dockerfile (lines 1-8) — quoted from the scanned source
FROM node:14
WORKDIR /app
COPY . .
RUN npm install
# Runs as root, ships the whole tree including .env — infra findings.
EXPOSE 3000
CMD ["node", "src/app.js"]

Suggested remediation:
Add a .dockerignore covering .env, .git, infra and node_modules, install with a production-only lockfile install, and add a non-root USER before CMD.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
The data layer exposes a query against reviews, but the migration set defines only users, products, orders and payments. Any caller of findReviews will fail at runtime with an unknown-table error, and the mismatch shows migrations are not the authoritative source for the schema the code assumes.

Reviewed: badapp/src/db/queries.js (lines 1-13) — quoted from the scanned source
const { connect } = require("./client");

// SQL injection: string concatenation straight into the query.
exports.findUser = (id, cb) =>
  connect().query("SELECT * FROM users WHERE id = '" + id + "'", cb);

exports.findOrders = (userId, cb) =>
  connect().query("SELECT * FROM orders WHERE user_id = '" + userId + "'", cb);

// ARCHITECTURE SMELL: queries a `reviews` table that no migration creates (schema drift).
exports.findReviews = (productId, cb) =>
  connect().query("SELECT * FROM reviews WHERE product_id = '" + productId + "'", cb);

Suggested remediation:
Either add a migration creating reviews or delete findReviews, and add a CI step that applies migrations to a scratch database and exercises the query layer against it.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
POST /refund passes a literal card object into paymentService.charge, and charge itself inserts amount_cents as the constant 100 regardless of the order. The endpoint is also named refund while performing an insert into payments. Financial logic embedded as literals in a route handler is impossible to test or configure and will silently produce wrong ledger rows.

Reviewed: badapp/src/routes/admin.js (lines 1-19) — quoted from the scanned source
const express = require("express");
const { exec } = require("child_process");
const paymentService = require("../services/paymentService");
const router = express.Router();

// NO AUTH ON ANY OF THESE. The architecture finding is the missing boundary, not the syntax.

// shell command built from user input -> command injection
router.get("/ping", (req, res) =>
  exec("ping -c 1 " + req.query.host, (err, stdout) => res.send(stdout)),
);

router.post("/refund", (req, res) =>
  paymentService.charge(req.query.orderId, { number: "[secret redacted]", cvv: "123", expiry: "12/30" },
    (e) => res.json({ ok: !e })),
);

module.exports = router;

Suggested remediation:
Derive the amount from the order record, pass card or token data in as a parameter rather than a literal, and rename the operation to match what it actually does.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
Handlers such as /user and /orders bind the error as e and never inspect it, sending res.json(rows) with rows undefined on failure, which yields a 200 with an empty body. /refund collapses every failure mode into ok: false with no detail or logging. Debugging a production incident with this pattern requires reproducing it locally.

Reviewed: badapp/src/routes/users.js (lines 1-16) — quoted from the scanned source
const express = require("express");
const queries = require("../db/queries");
const orderService = require("../services/orderService");
const router = express.Router();

router.get("/user", (req, res) => queries.findUser(req.query.id, (e, rows) => res.json(rows)));

router.get("/orders", (req, res) =>
  orderService.ordersFor(req.query.userId, (e, rows) => res.json(rows)),
);

// eval on user input -> code injection
router.get("/calc", (req, res) => res.send(String(eval(req.query.expr))));

module.exports = router;

Suggested remediation:
Add a shared error-handling path: check err in each callback, log it with context, and respond with an appropriate status code via Express's next(err) and a single error middleware.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
There is a db/queries module that owns SQL, but paymentService imports connect from db/client and issues its own INSERT. With two places allowed to write SQL, cross-cutting fixes — parameterisation, pooling, redaction of card fields in logs — have to be applied in more than one location and will drift.

Reviewed: badapp/src/services/paymentService.js (lines 1-11) — quoted from the scanned source
const { connect } = require("../db/client");

// ARCHITECTURE SMELL: a service reaching past the query layer into the driver directly,
// AND persisting full card data (see 0002_add_payments.sql).
exports.charge = (orderId, card, cb) =>
  connect().query(
    "INSERT INTO payments (order_id, amount_cents, card_number, card_cvv, card_expiry) VALUES ('" +
      orderId + "', 100, '" + card.number + "', '" + card.cvv + "', '" + card.expiry + "')",
    cb,
  );

Suggested remediation:
Move the payment insert into db/queries as a named, parameterised function and have paymentService depend only on that module.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
package.json declares jsonwebtoken, handlebars, lodash and minimist, and the Python side declares sqlalchemy, jinja2, pyyaml and requests, yet none of the scanned source imports them — the worker uses raw sqlite3 and the routes build HTML by concatenation. Unused runtime dependencies enlarge the image and the attack surface, and the presence of jsonwebtoken suggests an auth design that was never implemented.

Suggested remediation:
Remove dependencies that nothing imports, and if JWT auth is the intended design, implement it rather than leaving the library declared.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
findUser, findOrders and findReviews all use SELECT *. Beyond leaking password_hash to the /user response, this couples the API payload to the physical schema, so adding a column silently changes what the endpoint returns and what crosses the wire.

Reviewed: badapp/src/db/queries.js (lines 1-13) — quoted from the scanned source
const { connect } = require("./client");

// SQL injection: string concatenation straight into the query.
exports.findUser = (id, cb) =>
  connect().query("SELECT * FROM users WHERE id = '" + id + "'", cb);

exports.findOrders = (userId, cb) =>
  connect().query("SELECT * FROM orders WHERE user_id = '" + userId + "'", cb);

// ARCHITECTURE SMELL: queries a `reviews` table that no migration creates (schema drift).
exports.findReviews = (productId, cb) =>
  connect().query("SELECT * FROM reviews WHERE product_id = '" + productId + "'", cb);

Suggested remediation:
Enumerate the required columns in each query and map them to an explicit response shape in the route layer.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
app.js registers no express.json() or urlencoded middleware, and POST /refund reads orderId from req.query. Request bodies are therefore unavailable to any handler, and mutating operations carry their parameters in the URL where they end up in access logs and browser history.

Reviewed: badapp/src/routes/admin.js (lines 1-19) — quoted from the scanned source
const express = require("express");
const { exec } = require("child_process");
const paymentService = require("../services/paymentService");
const router = express.Router();

// NO AUTH ON ANY OF THESE. The architecture finding is the missing boundary, not the syntax.

// shell command built from user input -> command injection
router.get("/ping", (req, res) =>
  exec("ping -c 1 " + req.query.host, (err, stdout) => res.send(stdout)),
);

router.post("/refund", (req, res) =>
  paymentService.charge(req.query.orderId, { number: "[secret redacted]", cvv: "123", expiry: "12/30" },
    (e) => res.json({ ok: !e })),
);

module.exports = router;

Suggested remediation:
Register express.json() in app.js and change /refund to read its parameters from the request body.

Shown exactly as our checks reported it, with two exceptions: credential values are masked, and internal system names are removed.

Reported:
lib/logger.js emits a timestamp and a concatenated string, and orderService interpolates the caller-supplied userId into it. There is no level, no request correlation id and no structure, so logs cannot be filtered or joined to a request, and newline characters in the input can forge log lines.

Reviewed: badapp/src/services/orderService.js (lines 1-8) — quoted from the scanned source
const queries = require("../db/queries");
const { log } = require("../lib/logger");

exports.ordersFor = (userId, cb) => {
  log("loading orders for " + userId);
  return queries.findOrders(userId, cb);
};

Suggested remediation:
Adopt a structured logger with levels and per-request context, and pass user-supplied values as separate fields rather than concatenating them into the message.