diff --git a/.claude/commands/test.md b/.claude/commands/test.md new file mode 100644 index 0000000..473c9b1 --- /dev/null +++ b/.claude/commands/test.md @@ -0,0 +1,5 @@ +Run the full test suite with `npm test` and report the results. + +- Show a clear pass/fail summary (total tests, passed, failed). +- If any tests fail, quote the failing test name(s) and the assertion error. +- If all tests pass, confirm the count and that everything is green. diff --git a/.claude/skills/route-convention-check/SKILL.md b/.claude/skills/route-convention-check/SKILL.md new file mode 100644 index 0000000..2c9d4f7 --- /dev/null +++ b/.claude/skills/route-convention-check/SKILL.md @@ -0,0 +1,23 @@ +--- +name: route-convention-check +description: Verify that a newly added or modified route follows project conventions. Use when a route handler is added or changed in routes/. +--- +# Route convention check + +For every route touched in the current change, verify: + +1. **Auth** — is the route mounted before or after `requireToken` in `server.js`? + If it mutates data (POST/PUT/DELETE) and is after the gate, confirm it also + has the correct inline middleware (`requireToken`, `requireAdmin`, or both). + +2. **Input validation** — does the route validate `:id` params with `parseId` + from `lib/parseId.js`? Does it use `req.body ?? {}` before destructuring? + +3. **Error shape** — do all error responses follow `{ "error": "message" }` + with the right status (400 bad input, 404 missing record)? + +4. **Store access** — does the route read/write state only through `db/store.js`, + never holding state directly? + +Report each violation as a short bullet. If everything looks correct, say so in +one line and move on. diff --git a/.gitignore b/.gitignore index 7a07d5b..aef64e4 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,4 @@ npm-debug.log* # Personal, machine-local Claude settings (project-scoped settings ARE committed) .claude/settings.local.json +.claude/memory/ diff --git a/.mcp.json b/.mcp.json new file mode 100644 index 0000000..23bc439 --- /dev/null +++ b/.mcp.json @@ -0,0 +1,20 @@ +{ + "mcpServers": { + "memory": { + "type": "stdio", + "command": "npx", + "args": [ + "-y", + "@modelcontextprotocol/server-memory" + ], + "env": { + "MEMORY_FILE_PATH": "/Users/ton/Code/claude-wire-into-your-stack/.claude/memory/mcp_memory.json" + } + }, +"fetch": { + "type": "stdio", + "command": "uvx", + "args": ["mcp-server-fetch"] + } + } +} \ No newline at end of file diff --git a/CLAUDE.md b/CLAUDE.md index 020aabd..be65861 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -17,3 +17,24 @@ A small Express API used as the working project throughout the Claude Code cours - All data access goes through `db/store.js` — routes never hold state directly - Validate input in the route and return `400` on bad input, `404` when a record is missing - Error responses are JSON in the shape `{ "error": "message" }` + +## GitHub CLI + +Use `/opt/homebrew/bin/gh` (never bare `gh`) for all GitHub CLI operations. + +- Create PR: `/opt/homebrew/bin/gh pr create --title "…" --body "…"` +- List PRs: `/opt/homebrew/bin/gh pr list` +- View PR: `/opt/homebrew/bin/gh pr view []` +- CI status: `/opt/homebrew/bin/gh pr checks []` +- Merge PR: `/opt/homebrew/bin/gh pr merge [] --squash --delete-branch` + +Never push directly to `main` — always create a branch and open a PR. + +## Memory + +Use the `memory` MCP server to persist knowledge across conversations. + +- **Always store** important project context, decisions, user preferences, and architectural notes in the memory graph via `mcp__memory__create_entities` or `mcp__memory__add_observations`. +- A `UserPromptSubmit` hook injects a memory-read reminder each turn — respond to it by calling `mcp__memory__read_graph` when you haven't already done so this session. +- Entity types to use: `Project`, `Decision`, `Convention`, `Person`, `Bug`, `Feature`. +- Keep observations factual and concise; one observation per distinct fact. diff --git a/NOTES.md b/NOTES.md new file mode 100644 index 0000000..e1ce8cd --- /dev/null +++ b/NOTES.md @@ -0,0 +1,35 @@ +# Project 3 Notes + +## 1. Server (MCP) + +I connected two servers in `.mcp.json`: **`fetch`** (via `uvx mcp-server-fetch`) and **`memory`** (via `npx @modelcontextprotocol/server-memory`). + +The `fetch` server is useful here because it lets Claude pull live API references and docs during development — for example, fetching Express or Node.js documentation without leaving the session. No credentials required. The permission rule in `.claude/settings.local.json` explicitly allows `mcp__fetch__fetch` (and `mcp__memory__read_graph`) rather than blanket-allowing every tool the servers expose, keeping the surface area small. + +The `memory` server persists project context across sessions — architectural decisions, conventions, and who did what — so Claude arrives with background knowledge rather than starting cold every conversation. + +## 2. Skill + +The skill I encoded is **`route-convention-check`**, defined in `.claude/skills/route-convention-check/SKILL.md`. + +This project has a clear, repeated pattern every time a route is added or modified: check that auth middleware is applied correctly, that `:id` params are parsed through `lib/parseId.js`, that error responses follow `{ "error": "message" }` with the right status codes, and that data access goes only through `db/store.js`. Without a skill these checks get forgotten under time pressure. + +The description reads: *"Verify that a newly added or modified route follows project conventions. Use when a route handler is added or changed in routes/."* The phrase "route handler is added or changed in routes/" is specific enough to fire on route work and nothing else — it won't trigger on, say, a test refactor or a database change. + +## 3. Command + +I added a **`/test`** command in `.claude/commands/test.md`. + +It runs `npm test` and returns a structured summary: total tests, pass/fail count, and — on failure — the exact test name and assertion error. Running tests is the most frequent development action on this repo (every change needs a green suite before committing), and having a consistent report format means the output is always scannable rather than raw runner noise. + +## 4. Hook + +The hook is set in `.claude/settings.local.json` on the **`UserPromptSubmit`** event. + +It **reacts** (does not prevent) — it fires after the user submits a prompt but before Claude responds, echoing a reminder to call `mcp__memory__read_graph` if it hasn't been called yet this session. This ensures Claude always loads project context from the memory graph before answering, rather than starting from a blank slate. The event choice is deliberate: `UserPromptSubmit` is the earliest point where context injection matters; using `PostToolUse` would be too late in the turn. + +## 5. Headless run + +I ran the memory initialization task headless: seeding the knowledge graph with initial project entities (architecture, conventions, commands) so the memory MCP has baseline context from day one. + +The `--allowedTools` was locked down to just `mcp__memory__create_entities` and `mcp__memory__read_graph` — the minimum needed to read the existing graph and write new entities. No file editing, no bash, no fetch. This mirrors the principle from the course: pre-approve only what the task provably needs, so an autonomous run can't drift into unintended side effects. diff --git a/db/store.js b/db/store.js index 4c92e75..c8eaede 100644 --- a/db/store.js +++ b/db/store.js @@ -1,15 +1,25 @@ // In-memory data store. Every route reads and writes through these helpers, // so swapping in a real database later only touches this one file. +const crypto = require('crypto'); + let users = []; let nextId = 1; +let tokens = []; +let nextTokenId = 1; + function seed() { users = [ { id: 1, name: 'Ada Lovelace', email: 'ada@example.com' }, { id: 2, name: 'Alan Turing', email: 'alan@example.com' }, + { id: 3, name: 'Grace Hopper', email: 'grace@example.com' }, + { id: 4, name: 'Linus Torvalds', email: 'linus@example.com' }, ]; - nextId = 3; + nextId = 5; + + tokens = []; + nextTokenId = 1; } seed(); @@ -36,9 +46,42 @@ function updateUser(id, fields) { return user; } +function deleteUser(id) { + const index = users.findIndex((u) => u.id === id); + if (index !== -1) users.splice(index, 1); +} + +// --- Token helpers --- + +function createToken({ name, role = 'client', value } = {}) { + const token = { + id: nextTokenId, + name: name ?? null, + role, + token: value ?? crypto.randomBytes(32).toString('hex'), + createdAt: new Date().toISOString(), + }; + nextTokenId += 1; + tokens.push(token); + return token; +} + +function getTokenByValue(value) { + return tokens.find((t) => t.token === value); +} + +function revokeToken(id) { + const index = tokens.findIndex((t) => t.id === id); + if (index !== -1) tokens.splice(index, 1); +} + // Reset to the seed data. Used by the tests so each one starts clean. function reset() { seed(); } -module.exports = { listUsers, getUser, createUser, updateUser, reset }; +module.exports = { + listUsers, getUser, createUser, updateUser, deleteUser, + createToken, getTokenByValue, revokeToken, + reset, +}; diff --git a/lib/parseId.js b/lib/parseId.js new file mode 100644 index 0000000..6db16e6 --- /dev/null +++ b/lib/parseId.js @@ -0,0 +1,9 @@ +// Parse a route :id param string into a positive integer. +// Returns null for anything that is not a whole number ≥ 1 +// (NaN, floats, zero, negatives, non-numeric strings). +function parseId(value) { + const id = Number(value); + return Number.isInteger(id) && id >= 1 ? id : null; +} + +module.exports = parseId; diff --git a/middleware/auth.js b/middleware/auth.js new file mode 100644 index 0000000..928cf2b --- /dev/null +++ b/middleware/auth.js @@ -0,0 +1,22 @@ +const store = require('../db/store'); + +// Express middleware that enforces bearer-token authentication. +// Reads the Authorization header, validates the token against the store, +// and either calls next() (valid) or returns 401 (missing / invalid). +function requireToken(req, res, next) { + const authHeader = req.get('authorization'); + if (!authHeader || !authHeader.toLowerCase().startsWith('bearer ')) { + return res.status(401).json({ error: 'Unauthorized' }); + } + + const value = authHeader.slice('bearer '.length).trim(); + const token = store.getTokenByValue(value); + if (!token) { + return res.status(401).json({ error: 'Unauthorized' }); + } + + req.token = token; + return next(); +} + +module.exports = requireToken; diff --git a/middleware/requireAdmin.js b/middleware/requireAdmin.js new file mode 100644 index 0000000..e11c38c --- /dev/null +++ b/middleware/requireAdmin.js @@ -0,0 +1,8 @@ +function requireAdmin(req, res, next) { + if (!req.token || req.token.role !== 'admin') { + return res.status(403).json({ error: 'Forbidden' }); + } + return next(); +} + +module.exports = requireAdmin; diff --git a/package.json b/package.json index b209ca5..a96adfe 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,7 @@ "dev": "node server.js", "start": "node server.js", "test": "node --test", - "lint": "eslint server.js routes db tests" + "lint": "eslint server.js routes db middleware lib tests" }, "dependencies": { "express": "^4.19.2" diff --git a/routes/tokens.js b/routes/tokens.js new file mode 100644 index 0000000..c5a7a8c --- /dev/null +++ b/routes/tokens.js @@ -0,0 +1,28 @@ +const express = require('express'); +const store = require('../db/store'); +const parseId = require('../lib/parseId'); +const requireToken = require('../middleware/auth'); +const requireAdmin = require('../middleware/requireAdmin'); + +const router = express.Router(); + +// POST /tokens — create a new bearer token (open; bootstrapping). +// Always creates a client token — role from the request body is ignored. +router.post('/', (req, res) => { + const { name } = req.body ?? {}; + const token = store.createToken({ name, role: 'client' }); + return res.status(201).json(token); +}); + +// DELETE /tokens/:id — revoke a token by id. Admin only. +// Returns 204 whether or not the token existed (idempotent). +router.delete('/:id', requireToken, requireAdmin, (req, res) => { + const id = parseId(req.params.id); + if (id === null) { + return res.status(400).json({ error: 'Invalid id' }); + } + store.revokeToken(id); + return res.status(204).send(); +}); + +module.exports = router; diff --git a/routes/users.js b/routes/users.js index 8945477..d3a7902 100644 --- a/routes/users.js +++ b/routes/users.js @@ -1,5 +1,6 @@ const express = require('express'); const store = require('../db/store'); +const parseId = require('../lib/parseId'); const router = express.Router(); @@ -10,7 +11,11 @@ router.get('/', (req, res) => { // GET /users/:id — fetch one user, or 404 if it doesn't exist. router.get('/:id', (req, res) => { - const user = store.getUser(Number(req.params.id)); + const id = parseId(req.params.id); + if (id === null) { + return res.status(400).json({ error: 'Invalid id' }); + } + const user = store.getUser(id); if (!user) { return res.status(404).json({ error: 'User not found' }); } @@ -19,7 +24,7 @@ router.get('/:id', (req, res) => { // POST /users — create a user. Requires name and email. router.post('/', (req, res) => { - const { name, email } = req.body; + const { name, email } = req.body ?? {}; if (!name || !email) { return res.status(400).json({ error: 'name and email are required' }); } @@ -29,15 +34,29 @@ router.post('/', (req, res) => { // PUT /users/:id — update an existing user (added in Project 2). router.put('/:id', (req, res) => { - const { name, email } = req.body; + const id = parseId(req.params.id); + if (id === null) { + return res.status(400).json({ error: 'Invalid id' }); + } + const { name, email } = req.body ?? {}; if (name === undefined && email === undefined) { return res.status(400).json({ error: 'name or email is required' }); } - const user = store.updateUser(Number(req.params.id), { name, email }); + const user = store.updateUser(id, { name, email }); if (!user) { return res.status(404).json({ error: 'User not found' }); } return res.json(user); }); +// DELETE /users/:id — remove a user. Returns 204 whether or not the user existed (idempotent). +router.delete('/:id', (req, res) => { + const id = parseId(req.params.id); + if (id === null) { + return res.status(400).json({ error: 'Invalid id' }); + } + store.deleteUser(id); + return res.status(204).send(); +}); + module.exports = router; diff --git a/server.js b/server.js index 2178f6d..4c4d6b7 100644 --- a/server.js +++ b/server.js @@ -1,11 +1,16 @@ const express = require('express'); const usersRouter = require('./routes/users'); const healthRouter = require('./routes/health'); +const tokensRouter = require('./routes/tokens'); +const requireToken = require('./middleware/auth'); +const store = require('./db/store'); const app = express(); app.use(express.json()); app.use('/health', healthRouter); +app.use('/tokens', tokensRouter); +app.use(requireToken); app.use('/users', usersRouter); const PORT = process.env.PORT || 3000; @@ -13,6 +18,11 @@ const PORT = process.env.PORT || 3000; // Only start listening when run directly (e.g. `npm run dev`), so the tests // can import the app without opening a port. if (require.main === module) { + const adminValue = process.env.ADMIN_TOKEN; + const adminToken = store.createToken({ name: 'admin', role: 'admin', value: adminValue }); + if (!adminValue) { + console.log(`\nAdmin token (save this — shown once): ${adminToken.token}\n`); + } app.listen(PORT, () => { console.log(`API listening on http://localhost:${PORT}`); }); diff --git a/tests/tokens.test.js b/tests/tokens.test.js new file mode 100644 index 0000000..4a00eae --- /dev/null +++ b/tests/tokens.test.js @@ -0,0 +1,116 @@ +const test = require('node:test'); +const assert = require('node:assert'); +const request = require('supertest'); +const app = require('../server'); +const store = require('../db/store'); + +let adminToken; +let clientToken; + +test.beforeEach(() => { + store.reset(); + ({ token: adminToken } = store.createToken({ name: 'admin', role: 'admin' })); + ({ token: clientToken } = store.createToken({ name: 'client' })); +}); + +test('POST /tokens creates a client token and returns the secret', async () => { + const res = await request(app) + .post('/tokens') + .send({ name: 'my-key' }); + assert.equal(res.status, 201); + assert.ok(res.body.id); + assert.equal(res.body.name, 'my-key'); + assert.equal(res.body.role, 'client'); + assert.ok(typeof res.body.token === 'string' && res.body.token.length > 0); + assert.ok(res.body.createdAt); +}); + +test('POST /tokens ignores role in body — always creates client token', async () => { + const res = await request(app).post('/tokens').send({ role: 'admin' }); + assert.equal(res.status, 201); + assert.equal(res.body.role, 'client'); +}); + +test('POST /tokens works without a name', async () => { + const res = await request(app).post('/tokens').send({}); + assert.equal(res.status, 201); + assert.ok(res.body.token); +}); + +test('DELETE /tokens/:id revokes a token', async () => { + // Create a client token, confirm it works, then revoke it with the admin token. + const created = await request(app).post('/tokens').send({ name: 'temp' }); + const { id, token } = created.body; + + // Token should authorize /users before revocation. + const before = await request(app) + .get('/users') + .set('Authorization', `Bearer ${token}`); + assert.equal(before.status, 200); + + // Revoke using admin token. + const del = await request(app) + .delete(`/tokens/${id}`) + .set('Authorization', `Bearer ${adminToken}`); + assert.equal(del.status, 204); + + // Token should no longer authorize /users after revocation. + const after = await request(app) + .get('/users') + .set('Authorization', `Bearer ${token}`); + assert.equal(after.status, 401); +}); + +test('DELETE /tokens/:id returns 401 without any token', async () => { + const res = await request(app).delete('/tokens/1'); + assert.equal(res.status, 401); +}); + +test('DELETE /tokens/:id returns 403 for a client token', async () => { + const res = await request(app) + .delete('/tokens/1') + .set('Authorization', `Bearer ${clientToken}`); + assert.equal(res.status, 403); + assert.equal(res.body.error, 'Forbidden'); +}); + +test('DELETE /tokens/:id is idempotent — returns 204 even if already revoked', async () => { + const created = await request(app).post('/tokens').send({ name: 'temp' }); + const { id } = created.body; + + await request(app).delete(`/tokens/${id}`).set('Authorization', `Bearer ${adminToken}`); + const retry = await request(app).delete(`/tokens/${id}`).set('Authorization', `Bearer ${adminToken}`); + assert.equal(retry.status, 204); +}); + +test('DELETE /tokens/:id returns 204 for a never-existing id', async () => { + const res = await request(app) + .delete('/tokens/999') + .set('Authorization', `Bearer ${adminToken}`); + assert.equal(res.status, 204); +}); + +test('DELETE /tokens/:id returns 400 for a non-numeric id', async () => { + const res = await request(app) + .delete('/tokens/abc') + .set('Authorization', `Bearer ${adminToken}`); + assert.equal(res.status, 400); + assert.equal(res.body.error, 'Invalid id'); +}); + +test('DELETE /tokens/:id returns 400 for a fractional id', async () => { + const res = await request(app) + .delete('/tokens/1.5') + .set('Authorization', `Bearer ${adminToken}`); + assert.equal(res.status, 400); + assert.equal(res.body.error, 'Invalid id'); +}); + +test('POST /tokens with a non-JSON body does not crash', async () => { + const res = await request(app) + .post('/tokens') + .type('text') + .send('not json'); + assert.equal(res.status, 201); + assert.ok(res.body.token); +}); diff --git a/tests/users.test.js b/tests/users.test.js index d0d8f3d..c8c7a21 100644 --- a/tests/users.test.js +++ b/tests/users.test.js @@ -4,23 +4,39 @@ const request = require('supertest'); const app = require('../server'); const store = require('../db/store'); -test.beforeEach(() => store.reset()); +let token; -test('GET /users returns the seeded list', async () => { +test.beforeEach(() => { + store.reset(); + // Mint a fresh token for each test so the users routes can be reached. + ({ token } = store.createToken({ name: 'test' })); +}); + +test('GET /users returns 401 without a token', async () => { const res = await request(app).get('/users'); + assert.equal(res.status, 401); +}); + +test('GET /users returns the seeded list', async () => { + const res = await request(app) + .get('/users') + .set('Authorization', `Bearer ${token}`); assert.equal(res.status, 200); assert.ok(Array.isArray(res.body)); - assert.equal(res.body.length, 2); + assert.equal(res.body.length, 4); }); test('GET /users/:id returns 404 for a missing user', async () => { - const res = await request(app).get('/users/999'); + const res = await request(app) + .get('/users/999') + .set('Authorization', `Bearer ${token}`); assert.equal(res.status, 404); }); test('POST /users creates a user', async () => { const res = await request(app) .post('/users') + .set('Authorization', `Bearer ${token}`) .send({ name: 'Grace Hopper', email: 'grace@example.com' }); assert.equal(res.status, 201); assert.equal(res.body.name, 'Grace Hopper'); @@ -28,12 +44,64 @@ test('POST /users creates a user', async () => { }); test('PUT /users/:id updates an existing user', async () => { - const res = await request(app).put('/users/1').send({ name: 'Ada L.' }); + const res = await request(app) + .put('/users/1') + .set('Authorization', `Bearer ${token}`) + .send({ name: 'Ada L.' }); assert.equal(res.status, 200); assert.equal(res.body.name, 'Ada L.'); }); test('PUT /users/:id returns 404 for a missing user', async () => { - const res = await request(app).put('/users/999').send({ name: 'Nobody' }); + const res = await request(app) + .put('/users/999') + .set('Authorization', `Bearer ${token}`) + .send({ name: 'Nobody' }); assert.equal(res.status, 404); }); + +test('GET /users/:id returns 400 for a non-numeric id', async () => { + const res = await request(app) + .get('/users/abc') + .set('Authorization', `Bearer ${token}`); + assert.equal(res.status, 400); + assert.equal(res.body.error, 'Invalid id'); +}); + +test('PUT /users/:id returns 400 for a non-numeric id', async () => { + const res = await request(app) + .put('/users/abc') + .set('Authorization', `Bearer ${token}`) + .send({ name: 'X' }); + assert.equal(res.status, 400); + assert.equal(res.body.error, 'Invalid id'); +}); + +test('DELETE /users/:id removes the user and returns 204', async () => { + const res = await request(app) + .delete('/users/1') + .set('Authorization', `Bearer ${token}`); + assert.equal(res.status, 204); + + const check = await request(app) + .get('/users/1') + .set('Authorization', `Bearer ${token}`); + assert.equal(check.status, 404); +}); + +test('DELETE /users/:id returns 204 even when the user does not exist (idempotent)', async () => { + const res = await request(app) + .delete('/users/999') + .set('Authorization', `Bearer ${token}`); + assert.equal(res.status, 204); +}); + +test('DELETE /users/:id returns 400 for a non-numeric id', async () => { + const res = await request(app) + .delete('/users/abc') + .set('Authorization', `Bearer ${token}`); + assert.equal(res.status, 400); + assert.equal(res.body.error, 'Invalid id'); +}); + +