Fix: Mass assignment of role in unauthenticated POST /api/Users lets anyone register an admin account #695
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| name: 'PR Compliance Check' | |
| on: | |
| pull_request_target: | |
| types: [opened, edited, reopened] | |
| permissions: | |
| pull-requests: write | |
| issues: write | |
| jobs: | |
| check-compliance: | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: "Initialize PR Information" | |
| uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd #v8.0.0 | |
| id: init | |
| with: | |
| script: | | |
| const pr = context.payload.pull_request; | |
| const author = pr.user.login; | |
| // Fetch files changed in PR | |
| const { data: filesData } = await github.rest.pulls.listFiles({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| pull_number: pr.number | |
| }); | |
| const files = filesData.map(f => ({ | |
| filename: f.filename, | |
| status: f.status, | |
| additions: f.additions, | |
| deletions: f.deletions | |
| })); | |
| // Fetch commits for DCO check | |
| const { data: commitsData } = await github.rest.pulls.listCommits({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| pull_number: pr.number | |
| }); | |
| const commits = commitsData.map(c => ({ | |
| commit: { | |
| message: c.commit.message | |
| } | |
| })); | |
| const filteredPr = { | |
| number: pr.number, | |
| body: pr.body, | |
| title: pr.title, | |
| base: { ref: pr.base.ref }, | |
| user: { login: pr.user.login } | |
| }; | |
| core.setOutput('pr', JSON.stringify(filteredPr)); | |
| core.setOutput('author', author); | |
| core.setOutput('files', JSON.stringify(files)); | |
| core.setOutput('commits', JSON.stringify(commits)); | |
| console.log(`Initialized PR information for #${pr.number} by ${author}.`); | |
| - name: "Check Org Membership" | |
| uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd #v8.0.0 | |
| id: membership | |
| with: | |
| script: | | |
| const author = process.env.AUTHOR; | |
| try { | |
| await github.rest.orgs.checkPublicMembershipForUser({ | |
| org: 'juice-shop', | |
| username: author | |
| }); | |
| console.log(`${author} is a juice-shop org member, skipping checks.`); | |
| core.setOutput('is_member', 'true'); | |
| } catch (e) { | |
| console.log(`${author} is not a juice-shop org member, running checks.`); | |
| core.setOutput('is_member', 'false'); | |
| } | |
| env: | |
| AUTHOR: ${{ steps.init.outputs.author }} | |
| - name: "Check Target Branch" | |
| if: steps.membership.outputs.is_member == 'false' | |
| uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd #v8.0.0 | |
| id: branch | |
| with: | |
| script: | | |
| const pr = JSON.parse(process.env.PR_JSON); | |
| const wrongBranch = pr.base.ref === 'master'; | |
| core.setOutput('violation', wrongBranch ? 'true' : 'false'); | |
| console.log(`Target branch check: ${wrongBranch ? '❌ FAILED' : '✅ PASSED'}`); | |
| env: | |
| PR_JSON: ${{ steps.init.outputs.pr }} | |
| - name: "Check AI Tool Disclosure" | |
| if: steps.membership.outputs.is_member == 'false' | |
| uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd #v8.0.0 | |
| id: ai | |
| with: | |
| script: | | |
| const pr = JSON.parse(process.env.PR_JSON); | |
| const prBody = pr.body || ''; | |
| const aiNoContent = prBody.includes('[x] My contribution does not include any AI-generated content'); | |
| const aiWithContent = prBody.includes('[x] My contribution includes AI-generated content, as disclosed below'); | |
| const aiMissing = !aiNoContent && !aiWithContent; | |
| core.setOutput('violation', aiMissing ? 'true' : 'false'); | |
| console.log(`AI disclosure check: ${aiMissing ? '❌ FAILED' : '✅ PASSED'}`); | |
| env: | |
| PR_JSON: ${{ steps.init.outputs.pr }} | |
| - name: "Check Affirmation" | |
| if: steps.membership.outputs.is_member == 'false' | |
| uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd #v8.0.0 | |
| id: affirmation | |
| with: | |
| script: | | |
| const pr = JSON.parse(process.env.PR_JSON); | |
| const prBody = pr.body || ''; | |
| const affirmationMissing = !prBody.includes('[x] My code follows the [CONTRIBUTING.md]'); | |
| core.setOutput('violation', affirmationMissing ? 'true' : 'false'); | |
| console.log(`Affirmation check: ${affirmationMissing ? '❌ FAILED' : '✅ PASSED'}`); | |
| env: | |
| PR_JSON: ${{ steps.init.outputs.pr }} | |
| - name: "Check DCO Sign-off" | |
| if: steps.membership.outputs.is_member == 'false' | |
| uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd #v8.0.0 | |
| id: dco | |
| with: | |
| script: | | |
| const commits = JSON.parse(process.env.COMMITS_JSON); | |
| const dcoMissing = commits.some(c => !c.commit.message.includes('Signed-off-by:')); | |
| core.setOutput('violation', dcoMissing ? 'true' : 'false'); | |
| console.log(`DCO check: ${dcoMissing ? '❌ FAILED' : '✅ PASSED'}`); | |
| env: | |
| COMMITS_JSON: ${{ steps.init.outputs.commits }} | |
| - name: "Check I18N Modification" | |
| if: steps.membership.outputs.is_member == 'false' | |
| uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd #v8.0.0 | |
| id: i18n | |
| with: | |
| script: | | |
| const author = process.env.AUTHOR; | |
| const files = JSON.parse(process.env.FILES_JSON); | |
| const i18nModified = author !== 'bkimminich' && files.some(f => (f.filename.startsWith('frontend/src/assets/i18n/') || f.filename.startsWith('data/static/i18n/')) && !f.filename.endsWith('en.json')); | |
| core.setOutput('violation', i18nModified ? 'true' : 'false'); | |
| console.log(`I18N modification check: ${i18nModified ? '❌ FAILED' : '✅ PASSED'}`); | |
| env: | |
| AUTHOR: ${{ steps.init.outputs.author }} | |
| FILES_JSON: ${{ steps.init.outputs.files }} | |
| - name: "Perform Spam Detection" | |
| if: steps.membership.outputs.is_member == 'false' | |
| uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd #v8.0.0 | |
| id: spam | |
| with: | |
| script: | | |
| const pr = JSON.parse(process.env.PR_JSON); | |
| const files = JSON.parse(process.env.FILES_JSON); | |
| const commits = JSON.parse(process.env.COMMITS_JSON); | |
| const body = (pr.body || '').toLowerCase(); | |
| const title = (pr.title || '').toLowerCase(); | |
| const prBody = pr.body || ''; | |
| let spamScore = 0; | |
| const sastDastTools = /semgrep|snyk|sonar|fortify|checkmarx|veracode|trivy|grype|zap|burp/i; | |
| if (sastDastTools.test(title)) { | |
| spamScore += 25; | |
| console.log(`+25 Score (SAST/DAST tool name in title: "${title}") - Current: ${spamScore}`); | |
| } | |
| if (sastDastTools.test(body)) { | |
| spamScore += 25; | |
| console.log(`+25 Score (SAST/DAST tool name in body) - Current: ${spamScore}`); | |
| } | |
| if (commits.some(c => sastDastTools.test(c.commit.message))) { | |
| spamScore += 25; | |
| console.log(`+25 Score (SAST/DAST tool name in commit messages) - Current: ${spamScore}`); | |
| } | |
| if (files.some(f => sastDastTools.test(f.filename))) { | |
| spamScore += 25; | |
| console.log(`+25 Score (SAST/DAST tool name in filenames) - Current: ${spamScore}`); | |
| } | |
| if (title.length <= 5 || /^(update|fix|patch|docs|hi|hello|test|bug fix|typo|lab|homework|fixes|problem)$/i.test(title)) { | |
| spamScore += 20; | |
| console.log(`+20 Score (Low quality title: "${title}") - Current: ${spamScore}`); | |
| } | |
| if (body.length <= 5 || /^(fixed|update|hi|hello|done|check|fix)$/i.test(body) || body === '') { | |
| spamScore += 20; | |
| console.log(`+20 Score (Low quality body: "${body}") - Current: ${spamScore}`); | |
| } | |
| if (body.includes('<!-- ✍️-->') || /### description\s+a clear and concise summary/i.test(body)) { | |
| spamScore += 40; | |
| console.log(`+40 Score (Default template instructions not removed) - Current: ${spamScore}`); | |
| } | |
| if (files.length === 1 && files[0].filename === 'lib/insecurity.ts') { | |
| spamScore += 50; | |
| console.log(`+50 Score (Common fake change to lib/insecurity.ts detected) - Current: ${spamScore}`); | |
| } | |
| const maintainerFiles = [ | |
| 'HALL_OF_FAME.md', 'LICENSE', 'SECURITY.md', 'CODE_OF_CONDUCT.md', 'CONTRIBUTING.md', 'crowdin.yaml', | |
| 'AGENTS.md', 'CODEOWNERS', 'config.schema.yml', 'ctf.key', 'app.json' | |
| ]; | |
| const maintainerFileCount = files.filter(f => | |
| maintainerFiles.includes(f.filename) || | |
| f.filename.startsWith('.claude/') || | |
| f.filename.startsWith('.codeium/') || | |
| f.filename.startsWith('.continue/') || | |
| f.filename.startsWith('.cursor/') || | |
| f.filename.startsWith('.dependabot/') || | |
| f.filename.startsWith('.github/') || | |
| f.filename.startsWith('.gitlab/') || | |
| f.filename.startsWith('.zap/') | |
| ).length; | |
| if (maintainerFileCount > 0) { | |
| spamScore += 40; | |
| console.log(`+40 Score (Modified maintainer-only files) - Current: ${spamScore}`); | |
| if (maintainerFileCount > 1) { | |
| spamScore += 20; | |
| console.log(`+20 Score (Multiple maintainer-only files modified) - Current: ${spamScore}`); | |
| } | |
| } | |
| if (files.some(f => { | |
| if (f.status !== 'added') return false; | |
| const isYaml = f.filename.endsWith('.yml') || f.filename.endsWith('.yaml'); | |
| const isDotFile = f.filename.split('/').pop().startsWith('.'); | |
| const isMdOrLog = f.filename.endsWith('.md') || f.filename.endsWith('.log'); | |
| const isRoot = !f.filename.includes('/'); | |
| const isInDotFolder = f.filename.startsWith('.') || f.filename.includes('/.'); | |
| if ((isYaml || isDotFile) && (isRoot || isInDotFolder)) return true; | |
| if (isMdOrLog) return true; | |
| return false; | |
| })) { | |
| spamScore += 20; | |
| console.log(`+20 Score (Suspicious new file: .md, .log, dotfile or YAML in root/dotfolder) - Current: ${spamScore}`); | |
| } | |
| if (files.length === 1 && files[0].filename === 'README.md' && (files[0].additions + files[0].deletions) < 5) { | |
| spamScore += 20; | |
| console.log(`+20 Score (Trivial README.md change) - Current: ${spamScore}`); | |
| } | |
| if (files.length <= 2 && maintainerFileCount === 0) { | |
| const totalChanges = files.reduce((sum, f) => sum + f.additions + f.deletions, 0); | |
| if (totalChanges < 5) { | |
| spamScore += 30; | |
| console.log(`+30 Score (Low amount of total changes) - Current: ${spamScore}`); | |
| } | |
| } | |
| if (process.env.BRANCH_VIOLATION === 'true') { | |
| spamScore += 15; | |
| console.log(`+15 Score (Wrong target branch) - Current: ${spamScore}`); | |
| } | |
| if (process.env.AI_VIOLATION === 'true') { | |
| spamScore += 15; | |
| console.log(`+15 Score (Missing AI Tool Disclosure) - Current: ${spamScore}`); | |
| } | |
| if (process.env.AFFIRMATION_VIOLATION === 'true') { | |
| spamScore += 15; | |
| console.log(`+15 Score (Missing Affirmation) - Current: ${spamScore}`); | |
| } | |
| if (process.env.DCO_VIOLATION === 'true') { | |
| spamScore += 15; | |
| console.log(`+15 Score (Missing DCO Sign-off) - Current: ${spamScore}`); | |
| } | |
| if (process.env.I18N_VIOLATION === 'true') { | |
| spamScore += 15; | |
| console.log(`+15 Score (Direct I18N Modification) - Current: ${spamScore}`); | |
| } | |
| if (/(?:closes|fixes|resolves)\s+#\d+/i.test(prBody)) { | |
| spamScore -= 50; | |
| console.log(`-50 Score (References an issue) - Current: ${spamScore}`); | |
| } | |
| if (/#\d+/.test(title)) { | |
| spamScore -= 30; | |
| console.log(`-30 Score (References an issue in title) - Current: ${spamScore}`); | |
| } | |
| const spamRating = Math.max(0, Math.min(100, spamScore)); | |
| core.setOutput('spamRating', spamRating); | |
| console.log(`===================================`); | |
| console.log(`Final Spam Score: ${spamRating}/100`); | |
| env: | |
| PR_JSON: ${{ steps.init.outputs.pr }} | |
| FILES_JSON: ${{ steps.init.outputs.files }} | |
| COMMITS_JSON: ${{ steps.init.outputs.commits }} | |
| BRANCH_VIOLATION: ${{ steps.branch.outputs.violation }} | |
| AI_VIOLATION: ${{ steps.ai.outputs.violation }} | |
| AFFIRMATION_VIOLATION: ${{ steps.affirmation.outputs.violation }} | |
| DCO_VIOLATION: ${{ steps.dco.outputs.violation }} | |
| I18N_VIOLATION: ${{ steps.i18n.outputs.violation }} | |
| - name: "Finalize Compliance Check" | |
| if: always() && steps.membership.outputs.is_member == 'false' | |
| uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd #v8.0.0 | |
| id: check | |
| with: | |
| script: | | |
| const pr = JSON.parse(process.env.PR_JSON); | |
| const author = process.env.AUTHOR; | |
| const wrongBranch = process.env.BRANCH_VIOLATION === 'true'; | |
| const aiMissing = process.env.AI_VIOLATION === 'true'; | |
| const affirmationMissing = process.env.AFFIRMATION_VIOLATION === 'true'; | |
| const dcoMissing = process.env.DCO_VIOLATION === 'true'; | |
| const i18nModified = process.env.I18N_VIOLATION === 'true'; | |
| const spamRating = parseInt(process.env.SPAM_RATING); | |
| const isSpam = spamRating >= 75; | |
| const violations = []; | |
| const checkEmojis = [ | |
| !wrongBranch ? '✅' : '❌', | |
| !aiMissing ? '✅' : '❌', | |
| !affirmationMissing ? '✅' : '❌', | |
| !dcoMissing ? '✅' : '❌', | |
| !i18nModified ? '✅' : '❌' | |
| ].join(''); | |
| const spamPrefix = isSpam ? '🚨 ' : (spamRating >= 40 ? '⚠️ ' : '✅ '); | |
| core.setOutput('results', `${checkEmojis}${spamRating > 0 ? ` (${spamPrefix}Spam Score: ${spamRating}/100)` : ''} - ${author}`); | |
| if (isSpam) { | |
| violations.push( | |
| '🚨 **Potential Spam Detected:** _This PR has been identified as potential spam (Rating: ' + spamRating + '/100) based on recurring indicators._ ' + | |
| 'We sincerely appreciate your interest in OWASP Juice Shop, but we must focus our limited reviewer ' + | |
| 'resources on meaningful contributions that significantly improve the project. Thank you for your understanding. ' + | |
| 'See our [spam handling guide](https://pwning.owasp-juice.shop/companion-guide/latest/part3/contribution.html#_handling_of_spam_prs) for more details.' | |
| ); | |
| } | |
| if (wrongBranch) { | |
| violations.push( | |
| '🎯 **Wrong target branch:** This PR targets the `master` branch. ' + | |
| 'Per our [contributing guidelines](https://github.com/juice-shop/juice-shop/blob/develop/CONTRIBUTING.md), ' + | |
| 'all PRs must be based on the `develop` branch. Please re-open this PR against `develop`.' | |
| ); | |
| } | |
| if (aiMissing) { | |
| violations.push( | |
| '🤖 **Missing AI Tool Disclosure:** This PR is missing the required AI Tool Disclosure. ' + | |
| 'Please check one of the AI disclosure boxes in the PR template.' | |
| ); | |
| } | |
| if (affirmationMissing) { | |
| violations.push( | |
| '🧾 **Missing Affirmation:** This PR is missing the required affirmation that your code follows the [CONTRIBUTING.md](https://github.com/juice-shop/juice-shop/blob/develop/CONTRIBUTING.md) guidelines.' | |
| ); | |
| } | |
| if (dcoMissing) { | |
| violations.push( | |
| '✍️ **Missing DCO Sign-off:** All commits in this PR must be signed off to indicate your agreement with the [Developer Certificate of Origin](https://developercertificate.org/). Please use `git commit -s` for all your commits.' | |
| ); | |
| } | |
| if (i18nModified) { | |
| violations.push( | |
| '🌐 **Direct I18N Modification:** Translations must be contributed via [Crowdin](https://crowdin.com/project/owasp-juice-shop) and not via GitHub PRs. Please see our [translation guidelines](https://pwning.owasp-juice.shop/companion-guide/latest/part3/translation.html) for more details.' | |
| ); | |
| } | |
| if (violations.length === 0) { | |
| console.log('All compliance checks passed.'); | |
| return; | |
| } | |
| const header = 'Unfortunately, this PR does not meet our contributing guidelines and has been closed:'; | |
| const violationsList = violations.map(v => '- ' + v).join('\n'); | |
| let comment = ''; | |
| if (isSpam) { | |
| comment = [header, '', violationsList].join('\n'); | |
| } else { | |
| comment = [ | |
| 'Hi @' + author + ', thank you for your contribution! :raised_hands:', | |
| '', | |
| header, | |
| '', | |
| violationsList, | |
| '', | |
| 'Please address the above and open a new PR. If you have questions, check our [contributing guidelines](https://github.com/juice-shop/juice-shop/blob/develop/CONTRIBUTING.md) ' | |
| ].join('\n'); | |
| } | |
| const labels = ['invalid']; | |
| if (isSpam) labels.push('spam'); | |
| try { | |
| await github.rest.issues.addLabels({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: pr.number, | |
| labels: labels | |
| }); | |
| } catch (e) { | |
| console.log(`Failed to add labels to PR #${pr.number}: ${e.message}`); | |
| } | |
| try { | |
| await github.rest.issues.createComment({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: pr.number, | |
| body: comment | |
| }); | |
| } catch (e) { | |
| console.log(`Failed to create comment on PR #${pr.number}: ${e.message}`); | |
| } | |
| try { | |
| await github.rest.pulls.update({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| pull_number: pr.number, | |
| state: 'closed' | |
| }); | |
| } catch (e) { | |
| console.log(`Failed to close PR #${pr.number}: ${e.message}`); | |
| } | |
| console.log(`PR #${pr.number} closed due to ${violations.length} compliance violation(s). Applied labels: ${labels.join(', ')}`); | |
| env: | |
| PR_JSON: ${{ steps.init.outputs.pr }} | |
| AUTHOR: ${{ steps.init.outputs.author }} | |
| BRANCH_VIOLATION: ${{ steps.branch.outputs.violation }} | |
| AI_VIOLATION: ${{ steps.ai.outputs.violation }} | |
| AFFIRMATION_VIOLATION: ${{ steps.affirmation.outputs.violation }} | |
| DCO_VIOLATION: ${{ steps.dco.outputs.violation }} | |
| I18N_VIOLATION: ${{ steps.i18n.outputs.violation }} | |
| SPAM_RATING: ${{ steps.spam.outputs.spamRating }} | |
| - name: "Block Spammer" | |
| if: always() && steps.membership.outputs.is_member == 'false' && steps.spam.outputs.spamRating >= 75 | |
| uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd #v8.0.0 | |
| env: | |
| AUTHOR: ${{ steps.init.outputs.author }} | |
| ORG_ADMIN_TOKEN: ${{ secrets.ORG_ADMIN_TOKEN }} | |
| with: | |
| script: | | |
| const author = process.env.AUTHOR; | |
| const orgAdminToken = process.env.ORG_ADMIN_TOKEN; | |
| if (orgAdminToken) { | |
| try { | |
| const response = await fetch(`https://api.github.com/orgs/juice-shop/blocks/${encodeURIComponent(author)}`, { | |
| method: 'PUT', | |
| headers: { | |
| 'Authorization': `Bearer ${orgAdminToken}`, | |
| 'Accept': 'application/vnd.github+json', | |
| 'User-Agent': 'actions/github-script' | |
| } | |
| }); | |
| if (response.ok) { | |
| console.log(`User ${author} blocked due to spam.`); | |
| } else { | |
| const errorText = await response.text(); | |
| console.log(`Failed to block user ${author}: ${response.status} ${errorText}`); | |
| } | |
| } catch (e) { | |
| console.log(`Failed to block user ${author}: ${e.message}`); | |
| } | |
| } else { | |
| console.log('ORG_ADMIN_TOKEN secret not configured, skipping user block.'); | |
| } | |
| - name: "Slack Notification" | |
| if: always() && steps.check.outputs.results | |
| uses: Gamesight/slack-workflow-status@68bf00d0dbdbcb206c278399aa1ef6c14f74347a #v1.3.0 | |
| with: | |
| repo_token: ${{ secrets.GITHUB_TOKEN }} | |
| slack_webhook_url: ${{ secrets.SLACK_WEBHOOK_URL }} | |
| name: "PR Compliance: ${{ steps.check.outputs.results }}" |