Skip to content

fix: verify notification account ownership#5203

Merged
JhaSourav07 merged 5 commits into
JhaSourav07:mainfrom
Krishnx21:fix/5193-notification-ownership
Jun 12, 2026
Merged

fix: verify notification account ownership#5203
JhaSourav07 merged 5 commits into
JhaSourav07:mainfrom
Krishnx21:fix/5193-notification-ownership

Conversation

@Krishnx21

Copy link
Copy Markdown
Contributor

Summary

Fixes a critical account takeover vulnerability in the notification settings functionality where notification records could be created, updated, or deleted using only a GitHub username without any authentication or ownership verification.

This PR introduces proper authentication and ownership validation to ensure that only the legitimate owner of a GitHub account can manage their notification settings.

Related Issue

Closes #5193

Changes Made

  • Added authentication checks before processing notification settings requests.
  • Implemented ownership verification to ensure users can only manage their own notification records.
  • Restricted update operations to authenticated and authorized users.
  • Restricted delete operations to authenticated and authorized users.
  • Added validation to prevent unauthorized modification of existing notification settings.
  • Added validation to prevent unauthorized deletion of notification settings.
  • Improved security handling and error responses for unauthorized access attempts.

Security Impact

Before

  • Any user could modify notification settings by supplying another user's GitHub username.
  • Any user could change notification email addresses and preferences belonging to another account.
  • Any user could delete notification settings belonging to another account.
  • No ownership verification was performed.

After

  • Notification settings can only be managed by the verified owner of the associated GitHub account.
  • Unauthorized update attempts are blocked.
  • Unauthorized delete attempts are blocked.
  • Ownership verification is enforced before any modification or deletion operation.

Testing

Tested Scenarios

  • ✅ Authenticated user can manage their own notification settings.
  • ✅ Unauthorized user cannot modify another user's notification settings.
  • ✅ Unauthorized user cannot delete another user's notification settings.
  • ✅ Valid requests continue to function as expected.
  • ✅ Proper error responses are returned for unauthorized access attempts.

Type of Change

  • Bug fix
  • Security fix

Checklist

  • Code follows project guidelines.
  • Existing functionality remains unaffected.
  • Security validation added.
  • Tested locally.
  • Related issue linked.

GSSoC 2026

@vercel

vercel Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

@Krishnx21 is attempting to deploy a commit to the jhasourav07's projects Team on Vercel.

A member of the Team first needs to authorize it.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 991acaf56e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread lib/github-owner-verification.ts

@Aamod-Dev Aamod-Dev left a comment

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.

Thanks for the contribution. I went through the changes and the overall approach looks good. This fix addresses the issue effectively and prevents the edge case from occurring in the future. Looks good to merge.

@Aamod-Dev Aamod-Dev added GSSoC 2026 mentor:Aamod007 level:intermediate Moderate complexity tasks quality:clean PR follows clean coding practices, proper formatting, documentation, and maintainability standards. labels Jun 11, 2026

@Aamod-Dev Aamod-Dev left a comment

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.

Thanks for the contribution. I've taken a look at the changes and everything seems to align with what we need here.

Merging this looks safe. Approved!

@github-actions

Copy link
Copy Markdown
Contributor

🚨 Hey @Krishnx21, the CI Pipeline is failing on this PR and it has been marked as status:blocked.

Please fix the issues before this can be reviewed. Here's how:

1. Run checks locally before pushing:

npm run format:check   # Check Prettier formatting
npm run lint           # Run ESLint
npm run typecheck      # TypeScript type check
npm run test           # Run unit tests (Vitest)
npm run build          # Verify production build passes

2. Auto-fix common issues:

npm run format         # Auto-fix formatting with Prettier
npm run lint -- --fix  # Auto-fix lint errors where possible

3. Check the full failure log here:
👉 View CI Run

Once you push a fix and the CI passes, the status:blocked label will be removed automatically. 💪

@github-actions github-actions Bot added the status:blocked This PR is blocked due to a failing CI check. label Jun 12, 2026
@Krishnx21

Copy link
Copy Markdown
Contributor Author

@Aamod007 can you check this one ?

@github-actions github-actions Bot removed the status:blocked This PR is blocked due to a failing CI check. label Jun 12, 2026
@JhaSourav07 JhaSourav07 added the gssoc:approved PR has been reviewed and accepted for valid contribution points label Jun 12, 2026
@JhaSourav07 JhaSourav07 merged commit 6d5722f into JhaSourav07:main Jun 12, 2026
5 of 6 checks passed
@github-actions github-actions Bot added this to the GSSoC 2026 milestone Jun 12, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🎉 Congratulations @Krishnx21! Your PR has been successfully merged. 🚀

Thank you for contributing to CommitPulse. Your work helps us build a better tool for the community.

⚠️ Important for GSSoC Contributors:
You are strictly advised to join our Discord Server as it is mandatory for all GSSoC participants. All important announcements, point claims, and community discussions happen there.

Keep building! 💻✨

@JhaSourav07 JhaSourav07 added gssoc:approved PR has been reviewed and accepted for valid contribution points and removed gssoc:approved PR has been reviewed and accepted for valid contribution points labels Jun 17, 2026
@github-actions github-actions Bot added the type:bug Something isn't working as expected label Jun 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gssoc:approved PR has been reviewed and accepted for valid contribution points gssoc:needs-rebase GSSoC 2026 level:intermediate Moderate complexity tasks mentor:Aamod007 quality:clean PR follows clean coding practices, proper formatting, documentation, and maintainability standards. type:bug Something isn't working as expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Notification Account Takeover via Unauthenticated Username-Based Updates

3 participants