Skip to content

Fix/follow log analytics poisoning#337

Merged
Harxhit merged 2 commits into
Dev-Card:mainfrom
Ridanshi:fix/follow-log-analytics-poisoning
May 27, 2026
Merged

Fix/follow log analytics poisoning#337
Harxhit merged 2 commits into
Dev-Card:mainfrom
Ridanshi:fix/follow-log-analytics-poisoning

Conversation

@Ridanshi
Copy link
Copy Markdown
Contributor

Closes #301

Summary

Fixes an analytics integrity flaw where authenticated users could submit arbitrary status and layer values to:

POST /:platform/:targetUsername/log

The endpoint previously persisted attacker-controlled values directly into followLog without validation.

These fields are later consumed by analytics and follower-state logic, allowing fabricated follow activity and poisoned engagement metrics.


Problem

The route accepted free-form values for:

  • status
  • layer

and wrote them directly into persistence:

await app.prisma.followLog.create({
  data: {
    status: body.status,
    layer: body.layer,
  }
});

@Harxhit Harxhit self-requested a review May 26, 2026 07:16
@Harxhit Harxhit added the gssoc:approved Required label for every approved PR. Gives the base +50 points and enables contribution tracking. label May 26, 2026
@Harxhit
Copy link
Copy Markdown
Collaborator

Harxhit commented May 26, 2026

@Ridanshi Could please self review your PR I can see changes not valid for issue.

@Ridanshi
Copy link
Copy Markdown
Contributor Author

Self-review addressed and branch updated.

Changes are now scoped strictly to issue #301:

  • Restored the original API response contract in follow.ts ({ status: 'success', logId }) and removed the unintended logged response change.
  • Kept the shared followLogSchema enum validation as the core security fix.
  • Reduced the regression suite from 22 → 15 focused tests to avoid unnecessary churn.
  • Preserved an explicit documented rejection test for legacy layer: 'webview'.
  • Removed unnecessary lifecycle/factory complexity from tests.

Verification:

  • follow.test.ts → 15/15 tests passing
  • No unrelated refactors or formatting-only changes
  • Validation logic remains unchanged and enforced before DB writes

Branch is updated and ready for re-review.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the changes are related to Follow.ts why this file?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the changes are related to Follow.ts why this file?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the changes are related to Follow.ts why this file?

Ridanshi added 2 commits May 26, 2026 22:56
… logs

POST /:platform/:targetUsername/log accepted free-form `status` and
`layer` values and wrote them directly to followLog without validation.
Both fields feed analytics counters (totalFollows) and the
follower-state dashboard via `status: 'success'` queries, so an
authenticated user could fabricate successful follow events, inflate
engagement metrics, and manipulate the dashboard.

Fix:
- Add `followLogSchema` (Zod) in validations/follow.validation.ts with
  strict enum allowlists:
    status  → 'success' | 'failed' | 'pending'
    layer   → 'foreground' | 'background'
- Validate request body with safeParse before any database write;
  invalid payloads return 400 without touching followLog.create()
- Remove unsafe free-form defaults ('success' / 'webview') that
  silently accepted omitted fields
- Response body on validation failure contains only { error } —
  no Zod internals, paths, or stack traces are exposed

Layer 1 (API follow) writes status/layer internally and is unaffected.

Tests: 22 cases covering all valid enum combinations, all rejection
paths, DB-not-called guarantee on failure, correct payload written to
DB, and opaque error responses.

Closes Dev-Card#301
@Ridanshi Ridanshi force-pushed the fix/follow-log-analytics-poisoning branch from 0988d25 to 4f14f98 Compare May 26, 2026 17:27
@Ridanshi
Copy link
Copy Markdown
Contributor Author

Cleaned the branch and removed the unrelated event changes from PR #337.

The PR now contains only the follow log validation changes related to issue #301:

  • src/routes/follow.ts
  • src/validations/follow.validation.ts
  • src/__tests__/follow.test.ts

I had accidentally branched from the events work earlier, that unrelated commit has now been removed and the branch was safely force-pushed with --force-with-lease.

Latest commit:
4f14f98

Copy link
Copy Markdown
Collaborator

@Harxhit Harxhit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes are spefically to follow route eslint proofs and tests proofs are provided.

@Harxhit Harxhit merged commit 8421d23 into Dev-Card:main May 27, 2026
1 check failed
@Harxhit Harxhit added level:advanced Complex contribution involving deeper technical work. (+55 pts) quality:clean PR is well-structured, readable, and follows good practices. (×1.2 multiplier) type:performance Performance optimization (+15 pts) type:refactor Code refactoring/cleanup (+10 pts) type:bug Bug fixes (+10 pts) labels May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gssoc:approved Required label for every approved PR. Gives the base +50 points and enables contribution tracking. level:advanced Complex contribution involving deeper technical work. (+55 pts) quality:clean PR is well-structured, readable, and follows good practices. (×1.2 multiplier) type:bug Bug fixes (+10 pts) type:performance Performance optimization (+15 pts) type:refactor Code refactoring/cleanup (+10 pts)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Follow analytics can be poisoned through unvalidated followLog status and layer values

2 participants