feat: Todo MVP — REST API + frontend

[algorithm-conduction]

Summary

Implements the Todo MVP as described in the OpenSpec change 'spec'. A Node.js/Express REST API backed by an in-memory store is created alongside a vanilla HTML/JS single-page frontend. All four required CRUD endpoints are covered with supertest integration tests; every test passes.

Spec Reference

https://github.com/algorithm-conduction/todo-app/blob/hydra/spec/openspec/design.md

Changes

• package.json — Node.js project manifest; declares express, supertest, vitest; sets "type": "module" for ESM
• src/store.js — In-memory todo store with list, create, update, remove, and reset helpers; EUPL-1.2 header
• src/server.js — Express app exposing GET/POST /api/todos and PATCH/DELETE /api/todos/:id; serves static frontend; EUPL-1.2 header
• public/index.html — Single-page frontend: add, toggle-complete, and delete todos via fetch API calls; EUPL-1.2 header
• test/api.test.js — Vitest + supertest integration tests covering all four CRUD scenarios from specs/api.md; EUPL-1.2 header

Test Coverage

• test/api.test.js — POST creates todo (201, id/title/completed:false); POST rejects missing title (400); GET returns array with existing todos (200); GET returns empty array; PATCH marks todo completed (200); PATCH returns 404 for unknown id; DELETE removes todo (204) and absent from list; DELETE returns 404 for unknown id

[juanclaude-conduction]

[WARNING] No lint script defined in package.json
File: package.json
Issue: No lint script is configured. ESLint or another linter is absent, making it impossible to enforce consistent code style automatically. The Hydra pipeline requires npm run lint to be executable per the code-review spec.
Fix: Add ESLint (npm install --save-dev eslint) and define "lint": "eslint src test" in the scripts block of package.json.

[juanclaude-conduction]

[WARNING] No package-lock.json committed — npm ci fails
File: package.json
Issue: package-lock.json is absent from the repository. Running npm ci (the reproducible-install command used in CI pipelines) fails with EUSAGE. Without a lockfile, sub-dependency versions are not pinned and different environments may install different trees, making builds non-reproducible.
Fix: Run npm install locally and commit the generated package-lock.json. Consider adding package-lock.json explicitly to .gitignore negation (i.e., ensure it is not gitignored) so it stays tracked.

[clydebarcode-conduction]

[WARNING] Mass Assignment — PATCH endpoint passes unfiltered request body to Object.assign
Tool: Manual code review (Semgrep p/security-audit)
Rule: CWE-915 Improperly Controlled Modification of Object Prototype
File: src/server.js:36-37 → src/store.js:17-21
Issue: The PATCH /api/todos/:id handler extracts the entire req.body as { completed } via destructuring at the call site, but the store.update() function accepts a generic fields object and calls Object.assign(todo, fields) unconditionally. Any caller that passes a wider req.body — or a future refactor that removes the destructuring — will merge arbitrary client-supplied properties onto a stored todo object. A client can already send { "completed": true, "id": 0, "title": "overwritten" } and the destructuring assignment const { completed } = req.body only reads completed, but then passes { completed } (safe as written). However, the store.update API accepts any fields object — there is no allow-list inside the store itself, making it a latent mass-assignment sink. Additionally, Object.assign with a source that contains the string key __proto__ (as produced by older versions of body-parser) can lead to prototype pollution affecting the entire process.
Fix: Restrict store.update() to an explicit allow-list of mutable fields: only copy completed (and any other intentionally mutable fields) rather than forwarding the whole fields bag. For example:

export function update(id, { completed }) {
  const todo = todos.find((t) => t.id === id);
  if (!todo) return null;
  if (completed !== undefined) todo.completed = Boolean(completed);
  return todo;
}

This eliminates both the mass-assignment risk and any prototype-pollution surface.

[juanclaude-conduction]

[WARNING] Dead import — createRequire imported but never used
File: src/server.js:5
Issue: import { createRequire } from 'module'; is present on line 5 but createRequire is never referenced anywhere in the file. This appears to be copy-paste residue. Dead imports add noise, confuse readers, and will trigger linter errors once ESLint is added.
Fix: Remove line 5 entirely.

[clydebarcode-conduction]

[WARNING] Missing HTTP Security Headers
Tool: Manual code review
Rule: CWE-693 Protection Mechanism Failure / OWASP A05:2021 Security Misconfiguration
File: src/server.js (no middleware registered)
Issue: The Express application sets no security-related HTTP response headers. Without these headers, browsers apply no extra protections:

• X-Content-Type-Options: nosniff — prevents MIME-sniffing attacks that could cause the browser to interpret a response as a different content type.
• X-Frame-Options: DENY — prevents the app from being embedded in an iframe (clickjacking).
• Content-Security-Policy — without a CSP, any injected script (e.g. via a stored XSS) executes freely. Although the current frontend uses textContent (safe), the absence of a CSP removes a key defence-in-depth layer.
• Referrer-Policy — controls how much referrer information is sent with requests.
  Fix: Add the helmet middleware (one line) which sets all of the above with sensible defaults:

import helmet from helmet;
app.use(helmet());

Or set the individual headers manually if you prefer to avoid the dependency.

[juanclaude-conduction]

[SUGGESTION] list() returns the internal array reference — callers can mutate store state
File: src/store.js:8
Issue: list() returns the raw todos array. Any caller that mutates the returned array (e.g. store.list().push(x) or store.list().splice(0)) bypasses create()/remove() and corrupts internal state silently. While unlikely to matter in the current single-server MVP, it is a latent defensive-programming gap.
Fix: Return a shallow copy: return [...todos];

[juanclaude-conduction]

[SUGGESTION] PATCH endpoint does not validate that completed is a boolean
File: src/server.js:36
Issue: const { completed } = req.body is passed directly to store.update without a type check. A client can send { "completed": "yes" } or { "completed": 1 }, storing a non-boolean in the todo and potentially breaking consumers that strict-check === true.
Fix: Add a guard before calling store.update:

if (typeof completed !== 'boolean') {
  return res.status(400).json({ error: 'completed must be a boolean' });
}

[clydebarcode-conduction]

[SUGGESTION] No input length validation on title
Tool: Manual code review
Rule: CWE-400 Uncontrolled Resource Consumption
File: src/server.js:26
Issue: The POST handler validates that title is present and is a string, but places no upper bound on its length. An unauthenticated caller can submit arbitrarily large strings (e.g. several megabytes) which are accepted, stored in-memory, and then reflected back in every GET /api/todos response. Because the store is in-memory and the server is single-process, this provides a low-effort path to memory exhaustion. The default express.json() body size limit is 100 kB, which is a partial mitigation, but an attacker can still submit thousands of 99 kB titles.
Fix: Enforce a maximum title length in the validation block:

if (!title || typeof title !== 'string' || title.length > 255) {
  return res.status(400).json({ error: 'title is required and must be ≤ 255 characters' });
}

Also consider reducing the JSON body size limit: express.json({ limit: '4kb' }).

[juanclaude-conduction]

[SUGGESTION] Frontend fetch calls swallow errors silently
File: public/index.html
Issue: loadTodos, toggleTodo, deleteTodo, and the form submit handler all use await fetch(...) with no try/catch or .catch() handler. Network failures or non-2xx responses (e.g., a 500) produce no visible feedback; the UI simply does not update and the user has no way to know something went wrong.
Fix: Wrap async fetch calls in try/catch and surface errors to the user, e.g. with a simple alert(err.message) or a visible error banner.

[clydebarcode-conduction]

[SUGGESTION] No rate limiting on any API endpoint
Tool: Manual code review
Rule: CWE-307 Improper Restriction of Excessive Authentication Attempts / OWASP A04:2021
File: src/server.js (all routes)
Issue: All four REST endpoints (GET, POST, PATCH, DELETE) are unprotected from request flooding. Because there is no rate limiter and the store is in-memory, a rapid sequence of POST /api/todos calls can fill server memory without constraint.
Fix: Add a basic rate limiter for the mutation endpoints using express-rate-limit:

import rateLimit from 'express-rate-limit';
const limiter = rateLimit({ windowMs: 60_000, max: 60 });
app.use('/api', limiter);

This is optional for an MVP but should be in place before any public exposure.

[juanclaude-conduction]

[SUGGESTION] Deprecated dependencies — supertest@6.3.4 / superagent@8.1.2
File: package.json
Issue: supertest@6.3.4 pulls in the deprecated superagent@8.1.2. npm install reports 4 moderate severity vulnerabilities. Both packages themselves recommend upgrading.
Fix: Upgrade to supertest@^7.1.3 in devDependencies (npm install --save-dev supertest@^7.1.3).

[clydebarcode-conduction]

Hydra Security Review — Scanner Summary

Tool	Version	Findings
Semgrep (p/security-audit, p/secrets, p/owasp-top-ten)	1.135.0	0
Gitleaks	—	0
Trivy	not run	—

No automated findings were raised. The findings in this review are from manual code inspection of the PR diff.

No false positives to suppress — all scanner results were empty.

[clydebarcode-conduction]

{ "pass": true, "blocking": [] }

Hydra Security Verdict — Clyde Barcode, Conduction B.V.

No CRITICAL findings. Two WARNINGs and two SUGGESTIONs were raised from manual review; none are blocking for an MVP merge but the mass-assignment sink in store.update and the absent security headers should be addressed before any production exposure.

Severity	Count	Finding
CRITICAL	0	—
WARNING	2	Mass assignment in store.update (CWE-915); Missing HTTP security headers (CWE-693)
SUGGESTION	2	No title length limit (CWE-400); No rate limiting (CWE-307)

[juanclaude-conduction]

Hydra Code Review — Juan Claude van Damme

Summary

Severity	Count
CRITICAL	0
WARNING	3
SUGGESTION	3

Tests: 8/8 passed (vitest run) ✅
Lint: No npm run lint script defined — could not run ⚠️

Verdict

{ "pass": false, "blocking": ["No lint script defined in package.json", "No package-lock.json committed — npm ci fails", "Dead import: createRequire imported but never used in src/server.js:5"] }

No CRITICAL issues; all spec scenarios are implemented and all tests pass. The three WARNING items must be resolved before merge: the missing lint setup (a Hydra pipeline requirement), the missing lockfile (reproducibility), and the unused import (will block linting once ESLint is added).