diff --git a/.github/workflows/Comment_command.yml b/.github/workflows/Comment_command.yml index 7ec76b93bae2..65529b65c8a5 100644 --- a/.github/workflows/Comment_command.yml +++ b/.github/workflows/Comment_command.yml @@ -16,35 +16,34 @@ jobs: comment_command: name: Comment command ${{ github.event.issue.number }} - env: - GH_TOKEN: ${{ github.token }} permissions: contents: read runs-on: ubuntu-latest steps: - - name: Check if member - id: member - run: | - gh api \ - -H "Accept: application/vnd.github+json" \ - "/orgs/${GITHUB_REPOSITORY_OWNER}/members/${GITHUB_ACTOR}" || echo "member=false" >> "${GITHUB_OUTPUT}" - - name: Checkout if: - steps.member.outputs.member != 'false' && - github.actor != 'LizardByte-bot' + github.actor != 'LizardByte-bot' && + ( + startsWith(github.event.comment.body, '@LizardByte-bot') || + startsWith(github.event.comment.body, '/question') + ) uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Comment command env: COMMENT_BODY: ${{ github.event.comment.body }} + COMMENT_AUTHOR_ID: ${{ github.event.comment.user.id }} COMMENT_ID: ${{ github.event.comment.id }} ISSUE_BODY: ${{ github.event.issue.body }} + ISSUE_AUTHOR_ID: ${{ github.event.issue.user.id }} # https://regex101.com/r/vHEc61/1 YT_REGEX: '^((?:https?:)?\/\/)?((?:www|m)\.)?((?:youtube(-nocookie)?\.com|youtu.be))(\/(?:[\w\-]+\?v=|embed\/|v\/)?)([\w\-]+)(\S+)?$' # yamllint disable-line rule:line-length if: - steps.member.outputs.member != 'false' && - github.actor != 'LizardByte-bot' + github.actor != 'LizardByte-bot' && + ( + startsWith(github.event.comment.body, '@LizardByte-bot') || + startsWith(github.event.comment.body, '/question') + ) uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: github-token: ${{ secrets.GH_BOT_TOKEN }} diff --git a/.github/workflows/auto-update-db.yml b/.github/workflows/auto-update-db.yml index 5328cd9a2a11..480741fdb136 100644 --- a/.github/workflows/auto-update-db.yml +++ b/.github/workflows/auto-update-db.yml @@ -147,7 +147,7 @@ jobs: echo "\`\`\`" echo "" echo "" - cat ../bot_commands.md + cat ../docs/botCommands.md } >> ../comment.md cat ../comment.md echo "::endgroup::" @@ -172,6 +172,7 @@ jobs: if: always() uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 env: + AUTO_CLOSE: ${{ steps.outputs.outputs.auto_close }} DUPLICATE: ${{ steps.outputs.outputs.duplicate }} EXCEPTION: ${{ steps.outputs.outputs.exception }} with: @@ -204,7 +205,7 @@ jobs: - name: Auto Close if: - (github.event.label.name == 'request-theme') && + (github.event.label.name == 'request-theme' || github.event.label.name == 'approve-theme') && steps.outputs.outputs.auto_close == 'true' env: GH_TOKEN: ${{ secrets.GH_BOT_TOKEN }} @@ -220,6 +221,7 @@ jobs: if: (github.event.label.name == 'approve-theme') && steps.outputs.outputs.exception == 'false' && + steps.outputs.outputs.auto_close == 'false' && steps.labels.outputs.result == 'true' uses: actions-js/push@5a7cbd780d82c0c937b5977586e641b2fd94acc5 # v1.5 with: @@ -235,6 +237,7 @@ jobs: if: (github.event.label.name == 'approve-theme') && steps.outputs.outputs.exception == 'false' && + steps.outputs.outputs.auto_close == 'false' && steps.labels.outputs.result == 'true' env: GH_TOKEN: ${{ secrets.GH_BOT_TOKEN }} diff --git a/auto_approved_users.json b/auto_approved_users.json index 0033051ce26b..004881fc8f89 100644 --- a/auto_approved_users.json +++ b/auto_approved_users.json @@ -1,13 +1,22 @@ [ { + "commands": [ + "*" + ], "user_id": 30657709, "username": "LuckyNoS7evin" }, { + "commands": [ + "*" + ], "user_id": 42013603, "username": "ReenigneArcher" }, { + "commands": [ + "*" + ], "user_id": 88998541, "username": "jjnether" } diff --git a/docs/approverGuide.md b/docs/approverGuide.md index 543e0ae920c3..7db195d26762 100644 --- a/docs/approverGuide.md +++ b/docs/approverGuide.md @@ -57,18 +57,23 @@ The following guide is meant for authorized Themerr approvers to use when approv 6. Watch/listen to the theme on youtube - - This portion comes down to human judgement, but the [Theme Guidelines](../docs/themeGuidelines.md) should be - referenced for precise rules when deciding. - - Typically when examining a theme, we set the computer to about 50% volume then right-click and open the theme in an - incognito window (so google doesn't recommend stuff based on every theme that's reviewed). + - This portion comes down to human judgement, but the + [Theme Approver Guidelines](../docs/themeApproverGuidelines.md) and + [Theme Guidelines](../docs/themeGuidelines.md) should be referenced for precise rules when deciding. + - Typically when examining a theme, we set the computer to about 50% volume. + - Open only the `youtube_theme_url` link from the latest LizardByte-Bot comment table. Do not use the link from the + original issue body, since the bot comment contains the same canonical URL that will be written to the generated + JSON files. + - Prefer opening the theme in a private or incognito browsing session so YouTube does not adjust personal account + recommendations based on reviewed submissions. 7. Approve the theme - First, make sure the latest bot comment matches the latest edit of the issue body (it might take a few minutes, depending on how busy the GitHub activity is in our org at the time). - - To approve the theme, comment `@LizardByte-bot approve`. This will initiate a bot workflow where it will add the - `approve-queue` label. Once the issue comes to the front of the queue, the bot will run the checks again, then it - will add the theme to the database (assuming it passes everything). + - Use the approve command from the [Bot Commands](../docs/botCommands.md) reference. This will initiate a bot + workflow where it will add the `approve-queue` label. Once the issue comes to the front of the queue, the bot will + run the checks again, then it will add the theme to the database (assuming it passes everything). > [!WARNING] > Try to avoid approving any themes when a scheduled update is running. @@ -77,15 +82,12 @@ The following guide is meant for authorized Themerr approvers to use when approv ## Helpful Tools -- If you have a question or suggestion for a theme submission, place your comment, then add the `question` label to mark the issue. +- Bot commands for approvals, checks, edits, and questions are documented in [Bot Commands](../docs/botCommands.md). > [!NOTE] > If a user fails to answer your question after about a week or longer, you can proceed how you think best with the theme. -- A youtube URL in a user submission can be edited by commenting `@LizardByte-bot edit {youtube_url}`. - This will replace the youtube URL and run the workflow to check the new URL. - A useful website for checking regional blocks on a Youtube video can be found [here](https://watannetwork.com/tools/blocked/). -- A bot workflow to check a theme can be manually initiated by first removing, then adding the `request-theme` label. - The `approve-queue` label can be manually added to add an issue to the bot approval queue. > [!NOTE] diff --git a/bot_commands.md b/docs/botCommands.md similarity index 67% rename from bot_commands.md rename to docs/botCommands.md index d4873c1dde31..b037dd137e22 100644 --- a/bot_commands.md +++ b/docs/botCommands.md @@ -6,7 +6,8 @@ A moderator can trigger LizardByte-bot actions by commenting on this issue: - `@LizardByte-bot approve` approve the theme +- `@LizardByte-bot check` re-run the request checks - `@LizardByte-bot edit {youtube_url}` replace the YouTube url in the issue body with the given url - +- `/question {message}` add the question label while asking the submitter for more information diff --git a/docs/themeApproverGuidelines.md b/docs/themeApproverGuidelines.md new file mode 100644 index 000000000000..d7d5f273d2aa --- /dev/null +++ b/docs/themeApproverGuidelines.md @@ -0,0 +1,34 @@ +# Theme Approver Guidelines + +Use these guidelines when making the final approval decision for a theme submission. The public +[Theme Guidelines](../docs/themeGuidelines.md) still apply; this document is for approver judgement when a +submission passes the automated checks. + +## Core Preference + +Prefer themes that clearly represent the submitted movie, show, game, collection, or franchise. + +## Review Source + +Open the theme only from the `youtube_theme_url` row in the latest LizardByte-Bot comment table, not from the original +issue body. The bot comment contains the canonical URL that will be written to the generated JSON files. Prefer using a +private or incognito browsing session so YouTube does not adjust personal account recommendations based on reviewed +submissions. + +## Review Criteria + +- Themes should be representative of the movie, show, game, collection, etc. +- Fan-made themes should be avoided but can be accepted if there are no other alternatives. +- Trailer music should be avoided but can be accepted if there are no other alternatives. +- TV show themes should not be season-based, but can be accepted if there are no other alternatives. +- Collection and franchise themes should be representative of the entire group, not just one item in the group. +- Theme videos should not be an actual scene from the movie, show, or game. +- Themes should ideally not be from the opening sequence of the movie. +- Theme songs should not have extra sound effects other than what is in the song itself. In rare cases, themes with + sound effects can be accepted if there are no other alternatives. +- Theme songs should not contain racial slurs. + +## Exceptions + +If a submission relies on one of the exception cases above, check for better alternatives before approving it. If the +available choices are all imperfect, use the theme that best represents the item and leaves the fewest quality concerns. diff --git a/docs/themeGuidelines.md b/docs/themeGuidelines.md index 998128fe0dca..717eff9f3e85 100644 --- a/docs/themeGuidelines.md +++ b/docs/themeGuidelines.md @@ -44,9 +44,6 @@ the issue to let us know the reasoning behind the change. You can view and search the existing themes at [ThemerrDB](https://app.lizardbyte.dev/ThemerrDB). -## Miscellaneous +## Approval Review -- Theme videos should NOT be an actual scene from the movie or game. -- Themes should be representative of the movie or game. -- Themes should ideally NOT be from the opening sequence of the movie. -- Themes should NOT contain racial slurs. +Approvers use the [Theme Approver Guidelines](../docs/themeApproverGuidelines.md) when reviewing theme submissions. diff --git a/src/github-scripts/comment-command.js b/src/github-scripts/comment-command.js index 5b65b54d3c4b..a1f11342a772 100644 --- a/src/github-scripts/comment-command.js +++ b/src/github-scripts/comment-command.js @@ -2,6 +2,9 @@ * @file Handles issue comment commands for ThemerrDB moderation workflows. */ +const fs = require('node:fs') +const path = require('node:path') + const {queueIssueForApproval} = require('./approval-queue.js') const { delay, @@ -11,6 +14,15 @@ const { repoParams } = require('./github-issue.js') +const ADMIN_REPOSITORY_PERMISSION = 'admin' +const ALL_COMMANDS = '*' +const AUTO_APPROVED_USERS_FILE = 'auto_approved_users.json' +const BOT_COMMAND_PREFIX = '@LizardByte-bot' +const KNOWN_COMMANDS = new Set(['approve', 'check', 'edit', 'question']) +const NORMALIZABLE_PRIMITIVE_TYPES = new Set(['bigint', 'number', 'string']) +const QUESTION_LABEL = 'question' +const REQUEST_THEME_LABEL = 'request-theme' + /** * Normalize a command comment before parsing positional arguments. * @@ -27,6 +39,271 @@ function normalizeComment(commentBody) { return comment } +/** + * Parse a normalized comment into a command and positional arguments. + * + * @param {string} comment Normalized issue comment body. + * @returns {{command: string, args: string[]}} Parsed command details. + */ +function parseCommandComment(comment) { + const args = comment.split(' ') + if (args[0] === BOT_COMMAND_PREFIX) { + return { + command: normalizeCommandName(args[1]), + args + } + } + + if (args[0].startsWith('/')) { + return { + command: normalizeCommandName(args[0].slice(1)), + args + } + } + + return { + command: '', + args: [] + } +} + +/** + * Normalize a GitHub user id for matching. + * + * @param {*} userId GitHub user id. + * @returns {string} Trimmed user id string. + */ +function normalizeUserId(userId) { + if (!NORMALIZABLE_PRIMITIVE_TYPES.has(typeof userId)) { + return '' + } + + return String(userId).trim() +} + +/** + * Normalize a command name for matching. + * + * @param {*} command Bot command name. + * @returns {string} Lowercase command name. + */ +function normalizeCommandName(command) { + if (!NORMALIZABLE_PRIMITIVE_TYPES.has(typeof command)) { + return '' + } + + return String(command).trim().toLowerCase() +} + +/** + * Normalize a configured command list. + * + * @param {object} commands Configured command list. + * @returns {Set} Normalized command names. + */ +function normalizeAllowedCommands(commands) { + if (!Array.isArray(commands)) { + return new Set() + } + + return new Set(commands.map(normalizeCommandName).filter(Boolean)) +} + +/** + * Resolve a trusted users file inside the current workspace. + * + * @param {string} trustedUsersFile Trusted users file path. + * @returns {string} Absolute trusted users file path. + */ +function resolveTrustedUsersFile(trustedUsersFile) { + const baseDir = process.cwd() + const resolvedFile = path.resolve(baseDir, trustedUsersFile) + const relativePath = path.relative(baseDir, resolvedFile) + + if (relativePath.startsWith('..') || path.isAbsolute(relativePath)) { + throw new Error('trusted command users file must be inside the working directory') + } + + return resolvedFile +} + +/** + * Load trusted command users from the configured JSON file. + * + * @param {object} [options] Loader options. + * @param {string} [options.trustedUsersFile] Trusted users file path. + * @returns {Map>} Allowed command names keyed by GitHub user id. + */ +function loadTrustedCommandUsers({trustedUsersFile = process.env.AUTO_APPROVED_USERS_FILE || AUTO_APPROVED_USERS_FILE} = {}) { + let trustedUsers + + try { + const resolvedTrustedUsersFile = resolveTrustedUsersFile(trustedUsersFile) + trustedUsers = JSON.parse(fs.readFileSync(resolvedTrustedUsersFile, 'utf8')) + } catch (error) { + console.log(`trusted command users unavailable: ${error.message}`) + return new Map() + } + + if (!Array.isArray(trustedUsers)) { + return new Map() + } + + const trustedCommandUsers = new Map() + + for (const user of trustedUsers) { + if (!user || typeof user !== 'object' || Array.isArray(user)) { + continue + } + + const userId = normalizeUserId(user.user_id) + const commands = normalizeAllowedCommands(user.commands) + + if (userId && commands.size > 0) { + trustedCommandUsers.set(userId, commands) + } + } + + return trustedCommandUsers +} + +/** + * Determine whether a trusted user configuration allows a command. + * + * @param {object} options Options for checking the command. + * @param {Map>} options.trustedCommandUsers Trusted user command map. + * @param {object} options.userId GitHub user id. + * @param {string} options.command Bot command name. + * @returns {boolean} Whether the command is allowed. + */ +function trustedUserCanRunCommand({trustedCommandUsers, userId, command}) { + const commands = trustedCommandUsers.get(normalizeUserId(userId)) + + return Boolean(commands && (commands.has(ALL_COMMANDS) || commands.has(normalizeCommandName(command)))) +} + +/** + * Determine whether the command commenter is the issue author. + * + * @param {object} options Options for checking authorship. + * @param {string} options.command Bot command name. + * @param {object} options.commentAuthorId Comment author GitHub user id. + * @param {object} options.issueAuthorId Issue author GitHub user id. + * @returns {boolean} Whether the issue author may run the command. + */ +function issueAuthorCanRunCommand({command, commentAuthorId, issueAuthorId}) { + const normalizedCommentAuthorId = normalizeUserId(commentAuthorId) + + return normalizeCommandName(command) === 'edit' && + normalizedCommentAuthorId !== '' && + normalizedCommentAuthorId === normalizeUserId(issueAuthorId) +} + +/** + * Get the repository permission level for a GitHub actor. + * + * @param {object} options Options for checking repository permission. + * @param {object} options.github Authenticated Octokit client from actions/github-script. + * @param {import('./github-issue.js').GitHubScriptContext} options.context The actions/github-script context object. + * @param {string} options.actor GitHub actor login. + * @returns {Promise} Repository permission level. + */ +async function getRepositoryPermission({github, context, actor}) { + if (!actor) { + return '' + } + + try { + const response = await github.rest.repos.getCollaboratorPermissionLevel({ + ...repoParams(context), + username: actor + }) + + return `${response.data.permission}` + } catch (error) { + console.log(`repository permission unavailable for ${actor}: ${error.message}`) + return '' + } +} + +/** + * Determine whether the comment actor is a repository admin. + * + * @param {object} options Options for checking admin permissions. + * @param {object} options.github Authenticated Octokit client from actions/github-script. + * @param {import('./github-issue.js').GitHubScriptContext} options.context The actions/github-script context object. + * @param {string} options.actor GitHub actor login. + * @returns {Promise} Whether the actor has admin permission on the repository. + */ +async function actorIsRepositoryAdmin({github, context, actor}) { + const permission = await getRepositoryPermission({github, context, actor}) + + return permission === ADMIN_REPOSITORY_PERMISSION +} + +/** + * Determine whether the command is authorized for this comment. + * + * @param {object} options Options for checking authorization. + * @param {object} options.github Authenticated Octokit client from actions/github-script. + * @param {import('./github-issue.js').GitHubScriptContext} options.context The actions/github-script context object. + * @param {string} options.command Bot command name. + * @param {string} options.actor GitHub actor login. + * @param {object} options.commentAuthorId Comment author GitHub user id. + * @param {object} options.issueAuthorId Issue author GitHub user id. + * @param {Map>} [options.trustedCommandUsers] Trusted user command map. + * @returns {Promise} Whether the command is authorized. + */ +async function canRunCommand({ + github, + context, + command, + actor, + commentAuthorId, + issueAuthorId, + trustedCommandUsers = loadTrustedCommandUsers() +}) { + if (trustedUserCanRunCommand({trustedCommandUsers, userId: commentAuthorId, command})) { + return true + } + + if (issueAuthorCanRunCommand({command, commentAuthorId, issueAuthorId})) { + return true + } + + return actorIsRepositoryAdmin({github, context, actor}) +} + +/** + * Get the issue comment author id from the event payload or environment. + * + * @param {import('./github-issue.js').GitHubScriptContext} context The actions/github-script context object. + * @returns {string} GitHub user id. + */ +function getCommentAuthorId(context) { + return normalizeUserId(context.payload?.comment?.user?.id ?? process.env.COMMENT_AUTHOR_ID) +} + +/** + * Get the issue author id from the event payload or environment. + * + * @param {import('./github-issue.js').GitHubScriptContext} context The actions/github-script context object. + * @returns {string} GitHub user id. + */ +function getIssueAuthorId(context) { + return normalizeUserId(context.payload?.issue?.user?.id ?? process.env.ISSUE_AUTHOR_ID) +} + +/** + * Get the GitHub actor from the event context or environment. + * + * @param {import('./github-issue.js').GitHubScriptContext} context The actions/github-script context object. + * @returns {string} GitHub actor login. + */ +function getActor(context) { + return context.actor || context.payload?.comment?.user?.login || process.env.GITHUB_ACTOR || '' +} + /** * Update the issue body with a replacement YouTube URL and re-trigger request validation. * @@ -75,6 +352,47 @@ async function editThemeRequest({github, context, issueBody, youtubeRegex, repla } } +/** + * Re-trigger request validation by re-applying the request label. + * + * @param {object} options Options for checking a theme request. + * @param {object} options.github Authenticated Octokit client from actions/github-script. + * @param {import('./github-issue.js').GitHubScriptContext} options.context The actions/github-script context object. + * @returns {Promise} Promise resolved after the check workflow is triggered. + */ +async function checkThemeRequest({github, context}) { + const labels = await github.rest.issues.listLabelsOnIssue(issueParams(context)) + + await removeLabelsByName({ + github, + context, + currentLabels: labelNames(labels), + labelsToRemove: [REQUEST_THEME_LABEL] + }) + + await delay(10000) + + await github.rest.issues.addLabels({ + ...issueParams(context), + labels: [REQUEST_THEME_LABEL] + }) +} + +/** + * Add the question label to an issue. + * + * @param {object} options Options for adding the label. + * @param {object} options.github Authenticated Octokit client from actions/github-script. + * @param {import('./github-issue.js').GitHubScriptContext} options.context The actions/github-script context object. + * @returns {Promise} Promise resolved after the label is added. + */ +async function addQuestionLabel({github, context}) { + await github.rest.issues.addLabels({ + ...issueParams(context), + labels: [QUESTION_LABEL] + }) +} + /** * Add a positive reaction to a handled bot command comment. * @@ -108,22 +426,39 @@ async function run({github, context}) { console.log(`comment: ${comment}`) - if (!comment.startsWith('@LizardByte-bot')) { - console.log('the comment is not a @LizardByte-bot command, exiting') + const {command, args} = parseCommandComment(comment) + + if (!command) { + console.log('the comment is not a bot command, exiting') return } - const args = comment.split(' ') - let commandRan = false + if (!KNOWN_COMMANDS.has(command)) { + console.log('the bot command is unknown, exiting') + return + } - if (args[1] === 'approve') { - console.log('approve command running') - await queueIssueForApproval({github, context}) + const commandAllowed = await canRunCommand({ + github, + context, + command, + actor: getActor(context), + commentAuthorId: getCommentAuthorId(context), + issueAuthorId: getIssueAuthorId(context) + }) - commandRan = true + if (!commandAllowed) { + console.log('the bot command is not authorized for this commenter, exiting') + return } - if (args[1] === 'edit') { + if (command === 'approve') { + console.log('approve command running') + await queueIssueForApproval({github, context}) + } else if (command === 'check') { + console.log('check command running') + await checkThemeRequest({github, context}) + } else if (command === 'edit') { console.log('edit command running') await editThemeRequest({ @@ -133,19 +468,28 @@ async function run({github, context}) { youtubeRegex, replacementUrl: args[2] }) - - commandRan = true + } else { + console.log('question command running') + await addQuestionLabel({github, context}) } - if (commandRan) { - console.log('command ran, adding reaction') - await addCommandReaction({github, context, commentId}) - } + console.log('command ran, adding reaction') + await addCommandReaction({github, context, commentId}) } module.exports = { addCommandReaction, + addQuestionLabel, + actorIsRepositoryAdmin, + canRunCommand, + checkThemeRequest, editThemeRequest, + getActor, + issueAuthorCanRunCommand, + loadTrustedCommandUsers, normalizeComment, + normalizeCommandName, + normalizeUserId, + parseCommandComment, run } diff --git a/src/github-scripts/update-labels.js b/src/github-scripts/update-labels.js index 620301ce9746..e98a39e0595b 100644 --- a/src/github-scripts/update-labels.js +++ b/src/github-scripts/update-labels.js @@ -37,6 +37,7 @@ function setLabelState(currentLabels, labelName, enabled) { */ async function run({github, context}) { const labels = await github.rest.issues.listLabelsOnIssue(issueParams(context)) + const autoClose = process.env.AUTO_CLOSE const exception = process.env.EXCEPTION const duplicate = process.env.DUPLICATE @@ -47,7 +48,7 @@ async function run({github, context}) { const labelAdd = currentLabels.includes(APPROVE_THEME_LABEL) - if (labelAdd && exception === 'true') { + if (labelAdd && (autoClose === 'true' || exception === 'true')) { setLabelState(currentLabels, APPROVE_THEME_LABEL, false) } @@ -58,7 +59,7 @@ async function run({github, context}) { labels: currentLabels }) - if (labelAdd) { + if (labelAdd && autoClose !== 'true' && exception !== 'true') { return 'true' } } diff --git a/src/updater.py b/src/updater.py index ff336b7eaebe..008f8b1e6571 100644 --- a/src/updater.py +++ b/src/updater.py @@ -1248,7 +1248,7 @@ def _build_issue_metadata(item_type: str, json_data: dict) -> dict: return metadata_builders[item_type](json_data) -def _write_issue_metadata_files(item_type: str, json_data: dict) -> None: +def _write_issue_metadata_files(item_type: str, json_data: dict, youtube_url: Optional[str]) -> None: """Write the issue comment and title files for an issue update.""" metadata = _build_issue_metadata(item_type=item_type, json_data=json_data) issue_comment = f""" @@ -1258,6 +1258,7 @@ def _write_issue_metadata_files(item_type: str, json_data: dict) -> None: | year | {metadata['year']} | | summary | {metadata['summary']} | | id | {json_data['id']} | +| youtube_theme_url | {youtube_url or ''} | | poster | {metadata['poster']} | """ with open("comment.md", "a") as comment_f: @@ -1381,7 +1382,11 @@ def process_item_id(item_type: str, pass else: if args.issue_update: - _write_issue_metadata_files(item_type=item_type, json_data=json_data) + _write_issue_metadata_files( + item_type=item_type, + json_data=json_data, + youtube_url=youtube_url, + ) _update_issue_audit_data( og_data=og_data, item_type=item_type, diff --git a/tests/fixtures/trusted-command-users.json b/tests/fixtures/trusted-command-users.json new file mode 100644 index 000000000000..fa18a4bba0a1 --- /dev/null +++ b/tests/fixtures/trusted-command-users.json @@ -0,0 +1,39 @@ +[ + [], + { + "commands": [ + "*" + ], + "username": "missing-user-id" + }, + { + "commands": [ + "*" + ], + "user_id": null, + "username": "null-user-id" + }, + { + "commands": [], + "user_id": 1111, + "username": "missing-commands" + }, + { + "user_id": 4444, + "username": "missing-command-list" + }, + { + "commands": [ + " edit " + ], + "user_id": 2222, + "username": "limited-user" + }, + { + "commands": [ + "*" + ], + "user_id": 3333, + "username": "wildcard-user" + } +] diff --git a/tests/github_scripts.test.js b/tests/github_scripts.test.js index e6b879480480..e3da5fc07d66 100644 --- a/tests/github_scripts.test.js +++ b/tests/github_scripts.test.js @@ -644,6 +644,66 @@ describe('comment command script', () => { .toBe('@LizardByte-bot approve') }) + test('normalizes user ids without object stringification', () => { + expect(commentCommand.normalizeUserId(' 1234 ')).toBe('1234') + expect(commentCommand.normalizeUserId(1234)).toBe('1234') + expect(commentCommand.normalizeUserId({user_id: 1234})).toBe('') + expect(commentCommand.normalizeUserId(null)).toBe('') + expect(commentCommand.normalizeUserId(undefined)).toBe('') + }) + + test('normalizes command names without object stringification', () => { + expect(commentCommand.normalizeCommandName(' CHECK ')).toBe('check') + expect(commentCommand.normalizeCommandName(123)).toBe('123') + expect(commentCommand.normalizeCommandName({command: 'check'})).toBe('') + expect(commentCommand.normalizeCommandName(null)).toBe('') + expect(commentCommand.normalizeCommandName(undefined)).toBe('') + }) + + test('gets the actor from each supported source', () => { + process.env.GITHUB_ACTOR = 'env-user' + + expect(commentCommand.getActor({ + actor: 'context-user', + payload: { + comment: { + user: { + login: 'comment-user' + } + } + } + })).toBe('context-user') + expect(commentCommand.getActor({ + payload: { + comment: { + user: { + login: 'comment-user' + } + } + } + })).toBe('comment-user') + expect(commentCommand.getActor({payload: {}})).toBe('env-user') + + delete process.env.GITHUB_ACTOR + + expect(commentCommand.getActor({payload: {}})).toBe('') + }) + + test('parses bot mention and slash commands', () => { + expect(commentCommand.parseCommandComment('@LizardByte-bot approve')).toEqual({ + command: 'approve', + args: ['@LizardByte-bot', 'approve'] + }) + expect(commentCommand.parseCommandComment('/question Can you fix this theme?')).toEqual({ + command: 'question', + args: ['/question', 'Can', 'you', 'fix', 'this', 'theme?'] + }) + expect(commentCommand.parseCommandComment('plain comment')).toEqual({ + command: '', + args: [] + }) + }) + test('ignores comments that are not bot commands', async () => { process.env.COMMENT_BODY = 'plain comment' process.env.COMMENT_ID = '123' @@ -684,14 +744,131 @@ describe('comment command script', () => { expect(github.rest.reactions.createForIssueComment).not.toHaveBeenCalled() }) - test('queues approval commands and reacts to the comment', async () => { + test('loads trusted command users from configured allowlists', () => { + expect(commentCommand.loadTrustedCommandUsers({ + trustedUsersFile: 'auto_approved_users.json' + }).get('42013603')).toEqual(new Set(['*'])) + + expect(commentCommand.loadTrustedCommandUsers({ + trustedUsersFile: 'tests/fixtures/trusted-command-users.json' + })).toEqual(new Map([ + ['2222', new Set(['edit'])], + ['3333', new Set(['*'])] + ])) + }) + + test('fails closed when trusted command users cannot be loaded', () => { + expect(commentCommand.loadTrustedCommandUsers({ + trustedUsersFile: 'missing-command-users.json' + })).toEqual(new Map()) + expect(commentCommand.loadTrustedCommandUsers({ + trustedUsersFile: 'package.json' + })).toEqual(new Map()) + expect(commentCommand.loadTrustedCommandUsers({ + trustedUsersFile: '../auto_approved_users.json' + })).toEqual(new Map()) + }) + + test('allows configured users to run selected commands', async () => { + const github = {} + const trustedCommandUsers = new Map([ + ['1111', new Set(['approve'])], + ['2222', new Set(['*'])] + ]) + + await expect(commentCommand.canRunCommand({ + github, + context, + command: 'approve', + actor: '', + commentAuthorId: '1111', + issueAuthorId: '9999', + trustedCommandUsers + })).resolves.toBe(true) + await expect(commentCommand.canRunCommand({ + github, + context, + command: 'edit', + actor: '', + commentAuthorId: '1111', + issueAuthorId: '9999', + trustedCommandUsers + })).resolves.toBe(false) + await expect(commentCommand.canRunCommand({ + github, + context, + command: 'approve', + actor: '', + commentAuthorId: '2222', + issueAuthorId: '9999', + trustedCommandUsers + })).resolves.toBe(true) + await expect(commentCommand.canRunCommand({ + github, + context, + command: 'question', + actor: '', + commentAuthorId: '1111', + issueAuthorId: '9999', + trustedCommandUsers + })).resolves.toBe(false) + await expect(commentCommand.canRunCommand({ + github, + context, + command: 'check', + actor: '', + commentAuthorId: '1111', + issueAuthorId: '9999', + trustedCommandUsers + })).resolves.toBe(false) + }) + + test('allows issue authors to run edit commands only', () => { + expect(commentCommand.issueAuthorCanRunCommand({ + command: 'edit', + commentAuthorId: 1234, + issueAuthorId: '1234' + })).toBe(true) + expect(commentCommand.issueAuthorCanRunCommand({ + command: 'approve', + commentAuthorId: '1234', + issueAuthorId: '1234' + })).toBe(false) + expect(commentCommand.issueAuthorCanRunCommand({ + command: 'question', + commentAuthorId: '1234', + issueAuthorId: '1234' + })).toBe(false) + expect(commentCommand.issueAuthorCanRunCommand({ + command: 'check', + commentAuthorId: '1234', + issueAuthorId: '1234' + })).toBe(false) + expect(commentCommand.issueAuthorCanRunCommand({ + command: 'edit', + commentAuthorId: '', + issueAuthorId: '' + })).toBe(false) + }) + + test('allows repository admins to run commands', async () => { process.env.COMMENT_BODY = '@LizardByte-bot approve' process.env.COMMENT_ID = '123' + process.env.COMMENT_AUTHOR_ID = '9999' + process.env.GITHUB_ACTOR = 'repo-admin' + process.env.ISSUE_AUTHOR_ID = '8888' process.env.ISSUE_BODY = 'https://youtu.be/old' process.env.YT_REGEX = String.raw`youtu\.be` const github = { paginate: jest.fn(async () => []), rest: { + repos: { + getCollaboratorPermissionLevel: jest.fn(async () => ({ + data: { + permission: 'admin' + } + })) + }, issues: { listForRepo: jest.fn(), addLabels: jest.fn() @@ -716,6 +893,162 @@ describe('comment command script', () => { }) }) + test('blocks commands from unauthorized commenters', async () => { + process.env.COMMENT_BODY = '@LizardByte-bot approve' + process.env.COMMENT_ID = '123' + process.env.COMMENT_AUTHOR_ID = '9999' + process.env.GITHUB_ACTOR = 'outside-user' + process.env.ISSUE_AUTHOR_ID = '8888' + process.env.ISSUE_BODY = 'https://youtu.be/old' + process.env.YT_REGEX = String.raw`youtu\.be` + const github = { + paginate: jest.fn(async () => []), + rest: { + repos: { + getCollaboratorPermissionLevel: jest.fn(async () => ({ + data: { + permission: 'write' + } + })) + }, + issues: { + listForRepo: jest.fn(), + addLabels: jest.fn() + }, + reactions: { + createForIssueComment: jest.fn() + } + } + } + + await commentCommand.run({github, context}) + + expect(github.rest.issues.addLabels).not.toHaveBeenCalled() + expect(github.rest.reactions.createForIssueComment).not.toHaveBeenCalled() + }) + + test('fails closed when repository admin lookup fails', async () => { + const github = { + rest: { + repos: { + getCollaboratorPermissionLevel: jest.fn(async () => { + throw new Error('not found') + }) + } + } + } + + await expect(commentCommand.actorIsRepositoryAdmin({ + github, + context, + actor: 'outside-user' + })).resolves.toBe(false) + }) + + test('runs commands from trusted wildcard users', async () => { + process.env.COMMENT_BODY = '@LizardByte-bot approve' + process.env.COMMENT_ID = '123' + process.env.COMMENT_AUTHOR_ID = '42013603' + process.env.GITHUB_ACTOR = 'trusted-user' + process.env.ISSUE_AUTHOR_ID = '8888' + process.env.ISSUE_BODY = 'https://youtu.be/old' + process.env.YT_REGEX = String.raw`youtu\.be` + const github = { + paginate: jest.fn(async () => []), + rest: { + issues: { + listForRepo: jest.fn(), + addLabels: jest.fn() + }, + reactions: { + createForIssueComment: jest.fn() + } + } + } + + await commentCommand.run({github, context}) + + expect(github.rest.issues.addLabels).toHaveBeenCalledWith(expect.objectContaining({ + labels: ['approve-queue', 'approve-theme'] + })) + expect(github.rest.reactions.createForIssueComment).toHaveBeenCalled() + }) + + test('runs check commands by re-applying the request label', async () => { + runTimersImmediately() + const removedLabels = [] + process.env.COMMENT_BODY = '@LizardByte-bot check' + process.env.COMMENT_ID = '123' + process.env.COMMENT_AUTHOR_ID = '42013603' + process.env.GITHUB_ACTOR = 'trusted-user' + process.env.ISSUE_AUTHOR_ID = '8888' + process.env.ISSUE_BODY = 'https://youtu.be/old' + process.env.YT_REGEX = String.raw`youtu\.be` + const github = { + rest: { + issues: { + listLabelsOnIssue: jest.fn(async () => labels('request-theme', 'question')), + removeLabel: jest.fn(async params => removedLabels.push(params.name)), + addLabels: jest.fn() + }, + reactions: { + createForIssueComment: jest.fn() + } + } + } + + await commentCommand.run({github, context}) + + expect(removedLabels).toEqual(['request-theme']) + expect(github.rest.issues.addLabels).toHaveBeenCalledWith({ + owner: 'LizardByte', + repo: 'ThemerrDB', + issue_number: 7, + labels: ['request-theme'] + }) + expect(github.rest.reactions.createForIssueComment).toHaveBeenCalledWith({ + owner: 'LizardByte', + repo: 'ThemerrDB', + comment_id: 123, + content: '+1' + }) + }) + + test('runs question commands from trusted wildcard users', async () => { + process.env.COMMENT_BODY = '/question Can you fix this theme?' + process.env.COMMENT_ID = '123' + process.env.COMMENT_AUTHOR_ID = '42013603' + process.env.GITHUB_ACTOR = 'trusted-user' + process.env.ISSUE_AUTHOR_ID = '8888' + process.env.ISSUE_BODY = 'https://youtu.be/old' + process.env.YT_REGEX = String.raw`youtu\.be` + const github = { + rest: { + issues: { + addLabels: jest.fn() + }, + reactions: { + createForIssueComment: jest.fn() + } + } + } + + await commentCommand.run({github, context}) + + expect(github.rest.issues.addLabels).toHaveBeenCalledWith({ + owner: 'LizardByte', + repo: 'ThemerrDB', + issue_number: 7, + labels: ['question'] + }) + expect(github.rest.reactions.createForIssueComment).toHaveBeenCalledWith({ + owner: 'LizardByte', + repo: 'ThemerrDB', + comment_id: 123, + content: '+1' + }) + }) + test('skips edit updates when no YouTube URL is present', async () => { const github = { rest: { @@ -796,6 +1129,8 @@ describe('comment command script', () => { runTimersImmediately() process.env.COMMENT_BODY = '@LizardByte-bot edit https://youtu.be/new' process.env.COMMENT_ID = '123' + process.env.COMMENT_AUTHOR_ID = '1234' + process.env.ISSUE_AUTHOR_ID = '1234' process.env.ISSUE_BODY = 'https://youtu.be/old' process.env.YT_REGEX = String.raw`https:\/\/youtu\.be\/old` const github = { @@ -940,12 +1275,31 @@ describe('update labels script', () => { } } - await expect(updateLabels.run({github, context})).resolves.toBe('true') + await expect(updateLabels.run({github, context})).resolves.toBeUndefined() expect(github.rest.issues.setLabels).toHaveBeenCalledWith(expect.objectContaining({ labels: ['exception'] })) }) + test('removes approval label when auto-close is active', async () => { + process.env.AUTO_CLOSE = 'true' + process.env.EXCEPTION = 'false' + process.env.DUPLICATE = 'true' + const github = { + rest: { + issues: { + listLabelsOnIssue: jest.fn(async () => labels('approve-theme', 'approve-queue')), + setLabels: jest.fn() + } + } + } + + await expect(updateLabels.run({github, context})).resolves.toBeUndefined() + expect(github.rest.issues.setLabels).toHaveBeenCalledWith(expect.objectContaining({ + labels: ['duplicate'] + })) + }) + test('returns undefined when approval is not active', async () => { process.env.EXCEPTION = 'false' process.env.DUPLICATE = 'false' diff --git a/tests/unit/test_issue_updater.py b/tests/unit/test_issue_updater.py index b0a7d64dba56..c775a2b0314f 100644 --- a/tests/unit/test_issue_updater.py +++ b/tests/unit/test_issue_updater.py @@ -404,12 +404,14 @@ def test_write_issue_metadata_files(tmp_path, monkeypatch): updater._write_issue_metadata_files( item_type='game_collection', json_data={'id': 326, 'name': 'James Bond'}, + youtube_url='https://www.youtube.com/watch?v=themevideo', ) assert (tmp_path / 'title.md').read_text(encoding='utf-8') == '[GAME COLLECTION]: James Bond' comment = (tmp_path / 'comment.md').read_text(encoding='utf-8') assert '| title | James Bond |' in comment assert '| id | 326 |' in comment + assert '| youtube_theme_url | https://www.youtube.com/watch?v=themevideo |' in comment def test_update_issue_audit_data_sets_original_submission_fields(tmp_path, monkeypatch, youtube_url): @@ -604,6 +606,17 @@ def test_process_issue_update( data = updater.process_issue_update(database_url=db_url, youtube_url=youtube_url) assert data == db_type + comment = (tmp_path / 'comment.md').read_text(encoding='utf-8') + assert f'| youtube_theme_url | {youtube_url} |' in comment + + generated_theme_urls = [] + for item_file in (tmp_path / 'database').rglob('*.json'): + item_data = json.loads(item_file.read_text(encoding='utf-8')) + if 'youtube_theme_url' in item_data: + generated_theme_urls.append(item_data['youtube_theme_url']) + + assert generated_theme_urls + assert set(generated_theme_urls) == {youtube_url} def test_process_issue_update_invalid_youtube(issue_update_args, submission_invalid_youtube):