1. 🗺️ Scope and architecture
Goal of the PR
Users can currently add and complete tasks but not change their text. This PR adds an Edit button next to each task. Clicking it asks for a new description, saves it, and updates the page without a reload.
What changed
Relevant code outside the diff: app.js, where sessions and requireAuth are set up, and store.get().
Tech stack: Node.js with Express and express-session, PostgreSQL through pg, plain JavaScript in the browser.
Roles: the task owner (logged in), other logged-in users, anonymous visitors.
The story: what happens when a user clicks Edit
Each step says what happens; the code it happens in is attached to the step.
🖥️ Browser / client
🌐 Network
⚙️ Backend
🗄️ Database
1
Clicks Edit and types a new descriptionBrowser · public/tasks.js
2
Sends the update request, with the session cookie attachedBrowser · public/tasks.js → Backend · app.js
HTTP requestPUT /api/tasks/42 HTTP/1.1Host: todo.example.comCookie: connect.sid=s%3AkF9x...Content-Type: application/json {"description": "Buy oat milk"}3
Refuses the request unless the session identifies a userBackend · app.js
app.js5function requireAuth(req, res, next) {6 if (!req.session.userId) return res.sendStatus(401)7 next()8}11app.use('/api/tasks', requireAuth, tasksRouter)🔗 app.js#L5-L114
Hands off to the task update handlerBackend · routes/tasks.js
5
Asks for the task named in the URLBackend · store/taskStore.js
store/taskStore.js11async function get(id) {12 const { rows } = await db.query(13 'SELECT * FROM tasks WHERE id = $1', [id])14 return rows[0]15}🔗 store/taskStore.js#L11-L156
Reads that task rowPostgreSQL
7
Compares the task's owner with the logged-in userBackend · routes/tasks.js
routes/tasks.js32 const task = await store.get(req.params.id)33 if (task?.owner_id !== req.session.userId) {34 return res.sendStatus(404)35 }🔗 routes/tasks.js#L32-L358
Passes the new description to the storeBackend · store/taskStore.js
store/taskStore.js19 const { rows } = await db.query(20 `UPDATE tasks SET description = $121 WHERE id = $2 RETURNING *`,22 [description, id])🔗 store/taskStore.js#L19-L229
Writes the new descriptionPostgreSQL
10
Returns the updated rowBackend · routes/tasks.js
11
Returns the updated task as JSONBackend · routes/tasks.js → Browser · public/tasks.js
HTTP responseHTTP/1.1 200 OKContent-Type: application/json {"id": 42, "owner_id": 7, "description": "Buy oat milk"}12
Replaces the text shown on the pageBrowser · public/tasks.js
public/tasks.js33 document.getElementById(`desc-${id}`).textContent = task.description🔗 public/tasks.js#L33Assumptions and out of scope
- No CORS middleware is configured, so the API accepts only same-origin browser requests.
- Creating, completing and deleting tasks are unchanged and out of scope.
2. 🚪 Entry points
| # | Entry point | Authentication | Action performed | Sensitivity | Location |
|---|
| EP1 | PUT /api/tasks/:id | Session cookie | Rewrites one task's description | Medium — private user content, single record, reversible | routes/tasks.js#L31 |
EP1 · PUT /api/tasks/:id
| |
|---|
| Action performed | Replaces the description text of one existing task |
| Inputs | id (URL path, integer) · description (JSON body, string, no length limit) |
| Outputs | The updated task: id, owner_id, description |
| Data sensitivity | Task text is private user content. No PII, credentials or payment data. owner_id is returned, which discloses an internal user ID to its owner only |
| Action sensitivity | Write to a single record, reversible by the owner. Not financial, not destructive, no side effects outside the row |
| Authentication | Session cookie, enforced by requireAuth on the router mount |
| Authorization model | Per-record ownership: tasks.owner_id must equal the session user. This app has no tenants, organizations, groups, roles, permissions or feature flags — ownership is the only dimension |
| Location | routes/tasks.js#L31-L39 |
Who should be able to do this
- ✅ The task's owner, in an authenticated session started by them.
- ❌ Any other logged-in user, whether or not they know the task ID.
- ❌ Anonymous callers.
- ❌ Another origin acting through the victim's browser and cookie, even with the owner logged in.
- ❌ Nobody can change
owner_id through this route; ownership transfer is not a feature of this app.
The Edit button and prompt() in the browser collect the description, but the server can't trust them: any client can call EP1 directly.
5. 🔍 Mitigation analysis
T1 · Missing authentication
✅ Mitigated
Category: 🔓 Business logic · Entry point: EP1
Justification
All /api/tasks routes are mounted behind requireAuth, which returns 401 when there's no session, so the new route can't be reached anonymously.
app.js (unchanged by this PR)5function requireAuth(req, res, next) {6 if (!req.session.userId) {7 return res.sendStatus(401)8 }9 next()10}11app.use('/api/tasks', requireAuth, tasksRouter)
🔗 app.js#L5-L11
Detailed analysis, repro steps and evidence
Trace
EP1 → app.js L11 mounts tasksRouter behind requireAuth → L6 checks req.session.userId → the handler at routes/tasks.js L31 only runs after next().
Checked
requireAuth runs before the router, because middleware runs in the order it's registered.- No other code mounts
tasksRouter or defines /api/tasks routes without requireAuth.
Repro (confirms the control)
curl -i -X PUT https://todo.example.com/api/tasks/42 \ -H 'Content-Type: application/json' \ -d '{"description": "no session"}'
Evidence
HTTP/1.1 401 Unauthorized
T2 · Missing authorization (IDOR)
✅ Mitigated
Category: 🔓 Business logic · Entry point: EP1
Justification
The handler loads the task and returns 404 unless its owner_id matches the logged-in user, before calling store.update().
routes/tasks.js31router.put('/:id', async (req, res) => {32 const task = await store.get(req.params.id)33 if (task?.owner_id !== req.session.userId) {34 return res.sendStatus(404)35 }36 const updated = await store.update(37 task.id, req.body.description)38 res.json(updated)39})
🔗 routes/tasks.js#L31-L39
Detailed analysis, repro steps and evidence
Trace
EP1 → store.get(req.params.id) → ownership check at L33 → store.update() at L36 uses the loaded task.id, not the raw URL value.
Checked
- A missing task and someone else's task both return
404, so the response doesn't reveal which task IDs exist. task?.owner_id handles a missing task without throwing.owner_id and session.userId are both numbers, so the strict !== comparison behaves as intended.
Repro (confirms the control)
- Log in as
bob (user 8). - Send a request for task 42, which belongs to
alice (user 7):
curl -i -X PUT https://todo.example.com/api/tasks/42 \ -H 'Cookie: connect.sid=<bob session>' \ -H 'Content-Type: application/json' \ -d '{"description": "edited by bob"}'
Evidence
HTTP/1.1 404 Not Found
Task 42's description is unchanged in the database.
T3 · Missing CSRF protection
⚠️ Partially mitigatedSeverity: Low
Category: 🔓 Business logic · Entry point: EP1
Justification
There's no CSRF token and the session cookie has no SameSite attribute. Cross-site requests are blocked today only because a PUT with a JSON body triggers a CORS preflight that the server doesn't approve. That protection disappears if the route ever accepts POST or form bodies, or if CORS is enabled with credentials.
app.js (unchanged by this PR)4app.use(session({ secret: SECRET }))
🔗 app.js#L4
💥 Exploitability
Not exploitable as the code stands. An attacker would need the victim to be logged in and to visit a page they control, and would need the route to accept a request the browser sends cross-site — today it doesn't, because a PUT with a JSON body is preflighted and the preflight is refused. No resource ID is needed to try, since the attacker can guess task IDs, and no account on the app is required.
🛠️ Suggested fix
Set the cookie attributes so the browser never sends the session cookie cross-site, rather than relying on the preflight.
- app.use(session({ secret: SECRET }))+ app.use(session({+ secret: SECRET,+ cookie: { httpOnly: true, secure: true, sameSite: 'lax' },+ }))
secure: true requires HTTPS. Behind a proxy, also set app.set('trust proxy', 1).- For defense in depth, add a CSRF token check on state-changing routes.
- Verify: log in and confirm
Set-Cookie includes SameSite=Lax; Secure; HttpOnly.
Detailed analysis, repro steps and evidence
Trace
EP1 authenticates with the connect.sid cookie alone → the browser attaches that cookie to any request to the site it's allowed to send, including requests started by another site.
Checked
- No CSRF middleware or token check anywhere in the request path.
- Session cookie options are the defaults: no
SameSite, no Secure. - No CORS middleware, so preflight requests don't get
Access-Control-Allow-* headers. - Browsers always preflight a cross-site
PUT with Content-Type: application/json.
How to verify
from a page on another origin, call the route with the victim's session and observe whether the request is sent at all. Not executed in this review.
Evidence
none gathered; this finding is from code alone.
T4 · Cross-site scripting (XSS)
✅ Mitigated
Category: 💉 Source to sink · Sink: SK2
Justification
The description from the response is written with .textContent, so the browser shows it as text and never parses it as HTML.
public/tasks.js32 const task = await res.json()33 document.getElementById(`desc-${id}`).textContent = task.description
🔗 public/tasks.js#L32-L33
Detailed analysis, repro steps and evidence
Trace
description (EP1 body) → stored by SK1 → returned in the 200 response → written at L33 with .textContent.
Checked
- The new code has no
innerHTML, outerHTML, insertAdjacentHTML or document.write. - The element's ID is built from
id, the numeric task ID passed from the button, not from user text.
Repro (confirms the control)
- Edit a task and enter
<img src=x onerror=alert(1)> as the description. - Watch the task list.
Evidence
the task shows the literal text <img src=x onerror=alert(1)>, and no alert appears.
T5 · SQL injection
✅ Mitigated
Category: 💉 Source to sink · Sink: SK1
Justification
description and id are passed to PostgreSQL as parameters ($1, $2), never concatenated into the SQL text.
store/taskStore.js18async function update(id, description) {19 const { rows } = await db.query(20 `UPDATE tasks SET description = $121 WHERE id = $2 RETURNING *`,22 [description, id])23 return rows[0]24}
🔗 store/taskStore.js#L18-L24
Detailed analysis, repro steps and evidence
Trace
req.body.description (EP1) → store.update(task.id, description) → db.query with a fixed SQL string and a parameter array.
Checked
- The SQL text is a constant; the template literal has no
${…} placeholders. id comes from the task already loaded from the database (T2), not straight from the URL.
Repro (confirms the control)
curl -i -X PUT https://todo.example.com/api/tasks/42 \ -H 'Cookie: connect.sid=<alice session>' \ -H 'Content-Type: application/json' \ -d "{\"description\": \"x', owner_id = 1 --\"}"
Evidence
HTTP/1.1 200 OK {"id": 42, "owner_id": 7, "description": "x', owner_id = 1 --"}
The payload is stored as plain text, and owner_id is unchanged.