Skip to content

POST /api/activity/create lets any authenticated user inject a row into any pod's approval queue #1300

Closed

Description

@lilyshen0722

Measured on origin/main (d271b2f7); line numbers below are at the ADR-017 section's stamp 6a262fe8, where the route sits at routes/activity.ts:164.

POST /api/activity/create is gated by auth alone. It destructures type, action, content, podId, target, agentMetadata straight off the request body and calls Activity.create with them. There is no pod-membership or pod-existence check — podId is whatever the caller sends.

The body carries no approval subdoc, and it does not need to. models/Activity.ts:111 declares:

approval: {
  status: { type: String, enum: ['pending', 'approved', 'rejected'], default: 'pending' },

Mongoose materialises that default on every document, so a row created with type: 'approval_needed' satisfies both clauses of the reader:

activitySchema.statics.getPendingApprovals = async function (podIds) {
  return this.find({ podId: { $in: podIds }, type: 'approval_needed', 'approval.status': 'pending', deleted: { $ne: true } })

ActivityService.getPendingApprovals resolves podIds from pods the viewer created or admins — so the row lands in the decision queue of that pod's admins, attributed to the caller as actor, with attacker-controlled content.

Repro shape: authenticate as any user, POST /api/activity/create with { type: 'approval_needed', action: 'approval_needed', content: '<anything>', podId: '<any pod id>' }, then read GET /api/activity/approvals as an admin of that pod.

Why this is filed now. ADR-017 (#1256) stated that seedPodActivities was the only code that ever creates an approval_needed row; that sentence is corrected in 3bcbca61. The designed producer, Activity.createApprovalRequest, still has zero callers — so today the queue is empty and nothing has been injected. #1298 is currently wiring board facts into the same "Needs you" surface, which is why the producer set is worth having right.

Same class as the PATCH /api/integrations/:id finding: a generic create/merge endpoint is a writer for every schema-declared key, and a schema default can supply the half the caller did not send.

Suggested fix: validate podId against the caller's membership, and reject type: 'approval_needed' on this route entirely — approval rows should only come from Activity.createApprovalRequest.

🤖 Generated with Claude Code

Activity

  1. lilyshen0722 commented on Aug 27, 2026

    @lilyshen0722
    ContributorAuthor

    Correction to the blast radius, measured on origin/main — the reader is narrower than this issue claims.

    This issue says the injected row "lands in the decision queue of that pod's admins", and the repro says to read it back "as an admin of that pod". Both are wrong. @sprint-review measured it with a positive control (creator branch 1, admin-member branch 0, control {members: M} 1) and I reproduced it:

    ActivityService.getPendingApprovals (services/activityService.ts:1159) selects pods with

    $or: [
      { createdBy: userId },
      { 'members.userId': userId, 'members.role': 'admin' },
    ]

    but Pod.members is a bare ObjectId array — models/Pod.ts:138, members: [{ type: Schema.Types.ObjectId, ref: 'User' }]. There is no userId or role path beneath it, so the second branch matches nothing, for every pod. An injected approval reaches the pod's creator only.

    Two widenings beyond the correction:

    1. Three sites in activityService.ts query members.userId — :124 (getRecap), :355 (the activity feed), and :1164. The first two carry { members: userId } as a third $or branch beside the dead one, so they resolve correctly; :1164 is the only one missing that branch, which makes it the only site where the dead path is load-bearing.

    2. It cannot be repaired by correcting the path. Pod declares createdBy and members: Types.ObjectId[] and no admin field of any kind. Pod-scoped admin does not exist as data, so "who should receive an approval" is a design decision — creator-only, instance admins via user.role, or a new pod-level role — not a typo fix. Worth settling before anything wires a real producer into this queue (fix(activity): make the recap actionable #1298 is on the same surface).

    None of this changes the write-side finding or the fix in #1302: the route was reachable, the row was creatable, and it did land in a real queue. Only the audience was overstated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions