PDP-1182: Add pull_request_target trigger to copyright-check.yml for org ruleset support#43
Conversation
Enable copyright-check to be used directly via org-level repository rulesets without requiring a per-repo pr-workflow.yaml caller. - Adds pull_request_target trigger (opened, edited, synchronize, reopened) - Retains workflow_call for backward compat with existing per-repo callers - Safe for fork PRs: copyrightcheck.py (from trusted pr-workflows repo) only reads fork files as text — no code execution risk - Worst case from malicious fork .copyrightconfig is check passes (policy bypass only, not security exploit); reviewed by maintainer Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a pull_request_target trigger to the shared copyright validation workflow so it can be enforced directly via org-level repository rulesets, while keeping workflow_call for existing per-repo callers.
Changes:
- Added
pull_request_targettrigger (opened/edited/synchronize/reopened) for ruleset compatibility - Kept
workflow_callfor backward compatibility - Documented the fork-PR safety rationale in workflow comments
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fixes SECCMP-1797 regression: git fetch failed with exit code 128
for private repos because persist-credentials:false removed the token
after checkout, leaving no credentials for subsequent git fetch
- Use gh api /pulls/{n}/files instead — no git credentials needed,
works for public, private, and fork PRs equally
- Use IFS= read -r to correctly handle filenames with spaces
- Deleted files naturally filtered by [ -f target-repo/$f ] check
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Remove pull-requests: write permission (only issues: write needed for issue comment APIs; pull-requests: write was unnecessary scope) - Use refs/pull/N/head for fork PR checkout (more reliable than SHA fetch for pull_request_target; explicit named ref always resolvable) - Use --files-from-stdin to pass files to copyrightcheck.py (eliminates shell word-splitting and argument injection via filenames starting with '--') Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…oint The explicit permissions block restricts the GITHUB_TOKEN to only listed scopes. The 'gh api repos/.../pulls/.../files' call uses the Pull Requests API which requires pull-requests: read. Without it the call can 403 on private repos. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
When a PR only deletes files, '[ -f target-repo/$f ] && echo $f' returns exit code 1 (false && skipped-cmd). With bash pipefail this causes 'Get changed files' to fail before writing skip-validation=true, making all always() steps run with empty VALIDATION_STATUS. Using if/then/fi instead exits 0 when condition is false (no else), so the pipeline succeeds and the empty-file-list path works correctly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…st_target Two bugs fixed: 1. set -e: python3 exits 1 causing bash to exit before ec=$? and status=failed can be written to GITHUB_OUTPUT. Use &&/|| pattern so status is always written before the exit. 2. PR comment 403: org required workflows (pull_request_target via org ruleset) receive a read-only token for the triggering repo — createComment/updateComment always 403. Skip the comment step for pull_request_target and rely on Job Summary instead. workflow_call path still posts PR comments (token has write access to calling repo). Also improves the fallback error message to point to Job Summary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… checkout - Line 5 comment: 'write token' was misleading after the token was scoped down to least privilege. Updated to accurately state that GITHUB_TOKEN is explicitly scoped in the job permissions block. - COPYRIGHT_CHECK_COMMIT_SHA: moved from env: block (GitHub context) to git -C target-repo rev-parse HEAD in the run: script. This guarantees the reported SHA matches what was actually checked out, not the event payload SHA which can theoretically diverge. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…vent paths GitHub Actions has no per-event conditional permissions within a single job. Splitting into two jobs (one per event type) would require artifact sharing for summary.md/validation_output.txt and adds complexity for minimal gain. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
copyright-check.yml action pinning belongs in PR #43 only. Keep each PR to a single workflow file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
SameeraPriyathamTadikonda
force-pushed
the
feat/PDP-1182-copyright-check-org-ruleset-support
branch
from
441a366 to
060d1b0
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Pin actions/checkout@v4, actions/setup-python@v4, and actions/github-script@v7 to immutable commit SHAs to prevent supply chain attacks. - actions/checkout -> 34e114876b0b11c390a56381ad16ebd13914f8d5 - actions/setup-python -> 7f4fc3e22c37d6ff65e88745f38bd3157c663f7c - actions/github-script -> f28e40c7f34bde8b3046d885e986cb6290c5673b Note: reverts accidental regression introduced in prior SHA-pin attempt that inadvertently reverted pull_request_target trigger, refs/pull/N/head checkout ref, pull-requests: read permission, and --files-from-stdin argument safety. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SameeraPriyathamTadikonda
force-pushed
the
feat/PDP-1182-copyright-check-org-ruleset-support
branch
from
060d1b0 to
500c96b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
GAdityaVarma
left a comment
GAdityaVarma
left a comment
There was a problem hiding this comment.
Changes looks good.
GAdityaVarma
deleted the
feat/PDP-1182-copyright-check-org-ruleset-support
branch
Summary
Adds
pull_request_targettrigger tocopyright-check.ymlso it can be invoked directly via org-level repository rulesets, eliminating the need for a per-repopr-workflow.yamlcaller file.What changed
pull_request_targettrigger (types: opened, edited, synchronize, reopened)workflow_calltriggerpull-requests: write→pull-requests: readgithub.rest.issues.*APIs, not the Pull Requests APIrefs/pull/N/head(nothead.sha)head.shacan fail for fork PRs if the SHA is not yet in the base repo object storegit difftogh api .../pulls/.../filesfor changed files--files-from-stdin--or containing spacesCOPYRIGHT_CHECK_COMMIT_SHAderived fromgit -C target-repo rev-parse HEADpull_request_targetcreateCommentwould 403. Job Summary used instead${{ }}expressions moved toenv:blocksrun:blocksactions/checkout@34e114876b...,actions/setup-python@7f4fc3e22c...,actions/github-script@f28e40c7f3...PwnRequest Safety (SECCMP-1797)
pull_request_targettriggerrefs/pull/N/headchecked out into runner workspacecopyrightcheck.pyruns from this trusted repo; fork files are only read as text (never executed). Analogous to a virus scanner reading a suspect file. PwnRequest triad broken at step 3.Why it is safe for fork PRs
copyrightcheck.pyruns from this trusted repo; only reads fork files as text (never executes them)persist-credentials: falseon all checkouts--files-from-stdin), not shell word expansion.copyrightconfigbypass${{ }}injection inrun:env:vars, never inlined into shell commandsJira compliance
pull-requests: write→read;issues: writescoped to comment step only${{ }}moved toenv:varspull_request_target+refs/pull/N/headTest matrix —
marklogic/copyrighttestTested via org ruleset
TestPRworkflowspointing tofeat/PDP-1182-copyright-check-org-ruleset-support. All 8 scenarios verified against commit500c96bon 2026-04-08..javafile has no copyright headerfilesexcludedlist, no copyright header.mdfiles changed (README).copyrightconfigexcludes added fileNext steps
Once merged to
main:TestPRworkflowsto point to@maininstead of the feature branchgithub.com/organizations/marklogic/settings/rulespr-workflow.yamlfiles from the ~30 consumer repos