-
Notifications
You must be signed in to change notification settings - Fork 0
chore: add repository synchronization gates #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: qa/agent-triggerdotdev-trigger-dev/pr-08-4859/base
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| name: Dispatch RepoOps sync | ||
|
|
||
| on: | ||
| push: | ||
| branches: [main] | ||
| workflow_dispatch: | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| concurrency: | ||
| group: dispatch-repo-ops-sync | ||
| cancel-in-progress: false | ||
|
|
||
| jobs: | ||
| dispatch: | ||
| if: vars.REPO_OPS_SYNC_ENABLED == 'true' | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Create Dispatcher App token | ||
| id: app-token | ||
| uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 | ||
| with: | ||
| app-id: ${{ vars.REPO_OPS_DISPATCHER_APP_ID }} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shipwright · HIGH The 'dispatch' job in the dispatch workflow uses 'permission-contents: write' for the GitHub App token, but the workflow-level permissions are 'contents: read'. Impact: The 'dispatch' job in the dispatch workflow uses 'permission-contents: write' for the GitHub App token, but the workflow-level permissions are 'contents: read'. The App token is created with write access to the private mono repository, and the workflow dispatches a 'repository_dispatch' event to that repository. If the 'MONO_REPOSITORY' secret is ever compromised or the regex validation is bypassed, an attacker coul… Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright. |
||
| private-key: ${{ secrets.REPO_OPS_DISPATCHER_APP_PRIVATE_KEY }} | ||
| repositories: ${{ secrets.REPO_OPS_MONO_REPOSITORY }} | ||
| permission-contents: write | ||
|
|
||
| - name: Dispatch the private sync worker | ||
| env: | ||
| GH_TOKEN: ${{ steps.app-token.outputs.token }} | ||
| PUBLIC_SHA: ${{ github.sha }} | ||
| MONO_REPOSITORY: ${{ secrets.REPO_OPS_MONO_REPOSITORY }} | ||
| run: | | ||
| set -euo pipefail | ||
| [[ "$MONO_REPOSITORY" =~ ^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$ ]] | ||
| [[ "$PUBLIC_SHA" =~ ^[0-9a-f]{40}$ ]] | ||
| gh api "repos/${MONO_REPOSITORY}/dispatches" \ | ||
| --method POST \ | ||
| --field event_type=repo-ops-public-push \ | ||
| --field "client_payload[public_sha]=$PUBLIC_SHA" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| name: repo-ops-sync-gate | ||
|
|
||
| on: | ||
| pull_request: | ||
| types: [opened, edited, reopened, synchronize] | ||
| merge_group: | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: | ||
| gate: | ||
| if: github.event_name == 'pull_request' || vars.REPO_OPS_SYNC_ENABLED == 'true' | ||
| timeout-minutes: 15 | ||
| runs-on: ubuntu-latest | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shipwright · LOW The merge_group gate job condition is 'github.event_name == 'pull_request' || vars.REPO_OPS_SYNC_ENABLED == 'true''. Impact: The merge_group gate job condition is 'github.event_name == 'pull_request' || vars.REPO_OPS_SYNC_ENABLED == 'true''. For merge_group events, the first condition is false, so the job only runs when the repository variable is enabled. However, the subsequent steps that create the app token and wait for pending mono changes are gated only on 'github.event_name == 'merge_group''. If the variable is disabled, the job is… Suggested fix: Fix the review finding before release. |
||
| steps: | ||
| - name: Reject reserved RepoOps trailers | ||
| if: github.event_name == 'pull_request' | ||
| env: | ||
| PR_TITLE: ${{ github.event.pull_request.title }} | ||
| PR_BODY: ${{ github.event.pull_request.body }} | ||
| run: | | ||
| python3 - <<'PY' | ||
| import os | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shipwright · HIGH The 'Reject reserved RepoOps trailers' step only checks the PR title and body for 'OSS-RevId:' or 'Mono-RevId:' trailers. Impact: The 'Reject reserved RepoOps trailers' step only checks the PR title and body for 'OSS-RevId:' or 'Mono-RevId:' trailers. It does not check commit messages, which are a common place for trailers. An attacker could add a reserved trailer to a commit message and potentially confuse the sync tooling. The regex is also case-sensitive and anchored to line start, so 'oss-revid:' or leading whitespace would bypass the chec… Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright. |
||
| import re | ||
| import sys | ||
|
|
||
| text = f"{os.environ.get('PR_TITLE', '')}\n{os.environ.get('PR_BODY', '')}" | ||
| match = re.search(r"(?m)^(?:OSS-RevId|Mono-RevId):", text) | ||
| if match: | ||
| print(f"BLOCKED: PR title/body contains reserved RepoOps trailer {match.group(0)!r}", file=sys.stderr) | ||
| raise SystemExit(1) | ||
| print("ok: no reserved RepoOps trailers") | ||
| PY | ||
|
|
||
| - name: Pull request check context | ||
| if: github.event_name == 'pull_request' | ||
| run: echo 'State is checked again against the live tips by merge queue.' | ||
|
|
||
| - name: Create read-only Dispatcher App token | ||
| if: github.event_name == 'merge_group' | ||
| id: app-token | ||
| uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 | ||
| with: | ||
| app-id: ${{ vars.REPO_OPS_DISPATCHER_APP_ID }} | ||
| private-key: ${{ secrets.REPO_OPS_DISPATCHER_APP_PRIVATE_KEY }} | ||
| repositories: ${{ secrets.REPO_OPS_MONO_REPOSITORY }} | ||
| permission-contents: read | ||
|
|
||
| - name: Wait for pending mono changes | ||
| if: github.event_name == 'merge_group' | ||
| env: | ||
| APP_TOKEN: ${{ steps.app-token.outputs.token }} | ||
| PUBLIC_TOKEN: ${{ github.token }} | ||
| MONO_BASELINE: ${{ vars.REPO_OPS_MONO_BASELINE }} | ||
| PUBLIC_BASELINE: ${{ vars.REPO_OPS_PUBLIC_BASELINE }} | ||
| MONO_REPOSITORY: ${{ secrets.REPO_OPS_MONO_REPOSITORY }} | ||
| PUBLIC_REPOSITORY: ${{ github.repository }} | ||
| run: | | ||
| set -euo pipefail | ||
| [[ "$MONO_REPOSITORY" =~ ^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$ ]] | ||
| [[ "$PUBLIC_REPOSITORY" =~ ^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$ ]] | ||
| git clone --filter=blob:none --no-tags \ | ||
| "https://x-access-token:${PUBLIC_TOKEN}@github.com/${PUBLIC_REPOSITORY}.git" public | ||
| git clone --filter=blob:none --no-tags \ | ||
| "https://x-access-token:${APP_TOKEN}@github.com/${MONO_REPOSITORY}.git" mono | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shipwright · CRITICAL The workflow embeds the GitHub token directly into the clone URL: 'https://x-access-token:${PUBLIC_TOKEN}@github.com/...'. Impact: The workflow embeds the GitHub token directly into the clone URL: 'https://x-access-token:${PUBLIC_TOKEN}@github.com/...'. GitHub Actions automatically redacts registered secrets in logs, but 'PUBLIC_TOKEN' is 'github.token', which is not a registered secret and may not be redacted. If the clone command fails or is echoed by 'set -x'/debug logging, the token can leak into workflow logs. Use 'git -c http.extraheader=… Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright. |
||
|
|
||
| for _ in $(seq 1 60); do | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shipwright · CRITICAL The same token-in-URL pattern is used for the private mono repository with the GitHub App token: 'https://x-access-token:${APP_TOKEN}@github.com/${MONO_REPOSITORY}.git'. Impact: The same token-in-URL pattern is used for the private mono repository with the GitHub App token: 'https://x-access-token:${APP_TOKEN}@github.com/${MONO_REPOSITORY}.git'. The App token has 'contents: read' on the private mono repository. If this URL leaks in logs, an attacker could read the entire private mono repository, which likely contains proprietary or unreleased code. This is a high-impact secret exposure risk. Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright. |
||
| git -C public fetch --no-tags origin \ | ||
| +refs/heads/main:refs/remotes/origin/main | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shipwright · CRITICAL The wait loop runs 'mono/tooling/plan-repo-ops-outbound.sh' and then parses 'native_count' from its output file. Impact: The wait loop runs 'mono/tooling/plan-repo-ops-outbound.sh' and then parses 'native_count' from its output file. If the script fails, 'set -euo pipefail' will abort the step, but the temporary output file is never cleaned up because 'rm -f "$output"' is skipped on failure. More importantly, if the script produces no 'native_count=' line, 'grep' returns non-zero and the step fails immediately rather than retrying, wh… Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright. |
||
| git -C mono fetch --no-tags origin \ | ||
| +refs/heads/main:refs/remotes/origin/main | ||
| output="$(mktemp)" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shipwright · HIGH The 'plan-repo-ops-outbound.sh' script is invoked with positional arguments 'mono public "$(git -C mono rev-parse origin/main)" "$MONO_BASELINE" "$PUBLIC_BASELINE" "$output"', but Impact: The 'plan-repo-ops-outbound.sh' script is invoked with positional arguments 'mono public "$(git -C mono rev-parse origin/main)" "$MONO_BASELINE" "$PUBLIC_BASELINE" "$output"', but there is no documentation or comment explaining what these arguments mean. A new hire would have to read the script in the private mono repository to understand the contract. The variable names 'MONO_BASELINE' and 'PUBLIC_BASELINE' are opa… Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright. |
||
| mono/tooling/plan-repo-ops-outbound.sh \ | ||
| mono public "$(git -C mono rev-parse origin/main)" \ | ||
| "$MONO_BASELINE" "$PUBLIC_BASELINE" "$output" | ||
| native_count="$(grep -E '^native_count=' "$output" | cut -d= -f2)" | ||
| rm -f "$output" | ||
|
|
||
| if (( native_count == 0 )); then | ||
| echo 'No unsynced native mono oss/ commits.' | ||
| exit 0 | ||
| fi | ||
| echo "Waiting for $native_count native mono oss/ commit(s)." | ||
| sleep 10 | ||
| done | ||
| echo 'Timed out waiting for mono-to-public sync.' >&2 | ||
| exit 1 | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shipwright · HIGH
The 'dispatch' workflow has a 'concurrency' group with 'cancel-in-progress: false', which means multiple pushes to main will queue up dispatches.
Impact: The 'dispatch' workflow has a 'concurrency' group with 'cancel-in-progress: false', which means multiple pushes to main will queue up dispatches. If the private sync worker is slow, this could create a backlog of dispatches. There is no comment explaining why 'cancel-in-progress' is false, which is unusual for a dispatch workflow where only the latest SHA matters.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.