github-actions[bot] commented on code in PR #66959:
URL: https://github.com/apache/doris/pull/66959#discussion_r3820062580
##########
.github/workflows/code-review-sync-result.yml:
##########
@@ -7,10 +7,123 @@ on:
types: [created, edited]
permissions:
+ contents: read
pull-requests: read
statuses: write
jobs:
+ accept-skill-review:
+ name: Accept local skill review PASS
+ runs-on: ubuntu-latest
+ if: >
+ github.event_name == 'issue_comment' &&
+ github.event.issue.pull_request != null &&
+ contains(github.event.comment.body, '<!-- doris-repo-review:v1:begin
-->')
+ steps:
+ - name: Checkout trusted validation script
+ uses: actions/checkout@v4
+ with:
+ ref: ${{ github.event.repository.default_branch }}
+ persist-credentials: false
+
+ - name: Validate local review comment
+ id: validation
+ env:
+ GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ REPO: ${{ github.repository }}
+ PR_NUMBER: ${{ github.event.issue.number }}
+ COMMENT_AUTHOR: ${{ github.event.comment.user.login }}
Review Comment:
[major] Authorize the actor who supplied an edited receipt, not only the
comment's original author. On an `edited` event, `comment.user` remains the
original author while GitHub records the editor/sender separately; an installed
GitHub App with Issues/Pull requests write permission can PATCH another user's
comment and trigger this workflow. Such an app can turn maintainer M's comment
into a valid receipt naming M, after which this job checks M's collaborator
permission and never checks the app. Since the supported producer self-edits,
require the event actor/editor to equal the original author and declared
reviewer, verify that actor's live permission, and preserve that editor
provenance during later live reconciliation. Add a negative edited-event test
where the author is trusted but the sender is not.
##########
.github/workflows/code-review-sync-result.yml:
##########
@@ -7,10 +7,123 @@ on:
types: [created, edited]
permissions:
+ contents: read
pull-requests: read
statuses: write
jobs:
+ accept-skill-review:
+ name: Accept local skill review PASS
+ runs-on: ubuntu-latest
+ if: >
+ github.event_name == 'issue_comment' &&
+ github.event.issue.pull_request != null &&
+ contains(github.event.comment.body, '<!-- doris-repo-review:v1:begin
-->')
+ steps:
+ - name: Checkout trusted validation script
Review Comment:
[major] Move authorization before this checkout and limit retrieval to the
trusted validator. The job condition accepts any public PR comment containing
the marker, so an unaffiliated account can make the runner materialize a
one-commit but non-sparse default tree before lines 50-64 reject it. At the
authoritative base that tree has 40,724 entries / 707.2 MiB of raw blobs, while
the validator is only 8.8 KiB; repeated tiny comments therefore amplify into
substantial runner, disk, and network work. First require the event
actor/editor to equal the original author and verify that actor's live
permission without a checkout; `author_association` is only a coarse allocation
filter. Then sparse-fetch only the validator at a pinned trusted workflow SHA.
##########
.github/workflows/code-review-sync-result.yml:
##########
@@ -7,10 +7,123 @@ on:
types: [created, edited]
permissions:
+ contents: read
pull-requests: read
statuses: write
jobs:
+ accept-skill-review:
+ name: Accept local skill review PASS
+ runs-on: ubuntu-latest
+ if: >
+ github.event_name == 'issue_comment' &&
+ github.event.issue.pull_request != null &&
+ contains(github.event.comment.body, '<!-- doris-repo-review:v1:begin
-->')
+ steps:
+ - name: Checkout trusted validation script
+ uses: actions/checkout@v4
+ with:
+ ref: ${{ github.event.repository.default_branch }}
+ persist-credentials: false
+
+ - name: Validate local review comment
+ id: validation
+ env:
+ GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ REPO: ${{ github.repository }}
+ PR_NUMBER: ${{ github.event.issue.number }}
+ COMMENT_AUTHOR: ${{ github.event.comment.user.login }}
+ run: |
+ comment_file="$RUNNER_TEMP/doris-repo-review-comment.md"
+ jq -r '.comment.body' "$GITHUB_EVENT_PATH" > "$comment_file"
+
+ pr_info="$(gh api "repos/${REPO}/pulls/${PR_NUMBER}")"
+ head_sha="$(jq -r '.head.sha' <<<"$pr_info")"
+ live_base_sha="$(jq -r '.base.sha' <<<"$pr_info")"
+ pr_state="$(jq -r '.state' <<<"$pr_info")"
+ if [ "$pr_state" != "open" ]; then
+ echo "Review comment ignored: pull request is ${pr_state}."
+ echo "valid=false" >> "$GITHUB_OUTPUT"
+ exit 0
+ fi
+
+ if ! permission_info="$(gh api
"repos/${REPO}/collaborators/${COMMENT_AUTHOR}/permission")"; then
+ echo "Review comment ignored: cannot verify ${COMMENT_AUTHOR}'s
repository permission."
+ echo "valid=false" >> "$GITHUB_OUTPUT"
+ exit 0
+ fi
+ if ! comment_author_permission="$(jq -er '.permission | strings'
<<<"$permission_info")"; then
+ echo "Review comment ignored: cannot read ${COMMENT_AUTHOR}'s
repository permission."
+ echo "valid=false" >> "$GITHUB_OUTPUT"
+ exit 0
+ fi
+ if [[ "$comment_author_permission" != "write" &&
"$comment_author_permission" != "admin" ]]; then
+ echo "Review comment ignored: ${COMMENT_AUTHOR} does not have
write permission."
+ echo "valid=false" >> "$GITHUB_OUTPUT"
+ exit 0
+ fi
+
+ if ! reviewed_base_sha="$(python3
.github/scripts/validate_review_pass_comment.py \
+ extract-base --comment-file "$comment_file")"; then
+ echo "Review comment ignored: cannot read its reviewed base."
+ echo "valid=false" >> "$GITHUB_OUTPUT"
+ exit 0
+ fi
+ if ! base_compare="$(gh api \
+
"repos/${REPO}/compare/${reviewed_base_sha}...${live_base_sha}")"; then
+ echo "Review comment ignored: cannot compare its base with the
current PR base."
+ echo "valid=false" >> "$GITHUB_OUTPUT"
+ exit 0
+ fi
+ base_compare_status="$(jq -r '.status' <<<"$base_compare")"
+ if [ "$reviewed_base_sha" = "$live_base_sha" ]; then
+ reviewed_base_committed_at=""
+ live_base_committed_at=""
+ else
+ if ! reviewed_base_committed_at="$(gh api \
+ "repos/${REPO}/commits/${reviewed_base_sha}" --jq
'.commit.committer.date')" || \
+ ! live_base_committed_at="$(gh api \
+ "repos/${REPO}/commits/${live_base_sha}" --jq
'.commit.committer.date')"; then
+ echo "Review comment ignored: cannot resolve base commit times."
+ echo "valid=false" >> "$GITHUB_OUTPUT"
+ exit 0
+ fi
+ fi
+
+ if validation_output="$(python3
.github/scripts/validate_review_pass_comment.py validate \
+ --comment-file "$comment_file" \
+ --repository "$REPO" \
+ --pr-number "$PR_NUMBER" \
+ --head-sha "$head_sha" \
+ --live-base-sha "$live_base_sha" \
+ --base-compare-status "$base_compare_status" \
+ --reviewed-base-committed-at "$reviewed_base_committed_at" \
+ --live-base-committed-at "$live_base_committed_at" \
+ --comment-author "$COMMENT_AUTHOR" \
+ --comment-author-permission "$comment_author_permission" 2>&1)";
then
+ echo "$validation_output"
+ echo "valid=true" >> "$GITHUB_OUTPUT"
+ echo "head_sha=$head_sha" >> "$GITHUB_OUTPUT"
+ else
+ echo "$validation_output"
+ echo "valid=false" >> "$GITHUB_OUTPUT"
Review Comment:
[major] Reconcile live, provenance-preserving sources before retaining or
publishing success. After a valid comment posts `code-review: success`, this
`valid=false` path leaves that success as the latest status; removing the
marker skips the job, and deletion is not subscribed to. Because validation
reads the event snapshot and runs are not serialized, an older create run or
manual rerun can also finish after an edit/delete and restore success from
stale content. Please handle the full comment lifecycle, refetch/enumerate
current receipts with their editor provenance, and make stale runs idempotent
with a final generation/current-state check. One PR/head/base-bound arbiter
must OR the independent automated-review, authorized-skip, and live-receipt
sources so invalidating one receipt does not clobber another legitimate
success; authoritative-read failures should remain retryable/unknown, not reuse
the aggregate status as evidence.
##########
.github/workflows/code-review-sync-result.yml:
##########
@@ -7,10 +7,123 @@ on:
types: [created, edited]
permissions:
+ contents: read
pull-requests: read
statuses: write
jobs:
+ accept-skill-review:
+ name: Accept local skill review PASS
+ runs-on: ubuntu-latest
+ if: >
+ github.event_name == 'issue_comment' &&
+ github.event.issue.pull_request != null &&
+ contains(github.event.comment.body, '<!-- doris-repo-review:v1:begin
-->')
+ steps:
+ - name: Checkout trusted validation script
+ uses: actions/checkout@v4
+ with:
+ ref: ${{ github.event.repository.default_branch }}
+ persist-credentials: false
+
+ - name: Validate local review comment
+ id: validation
+ env:
+ GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ REPO: ${{ github.repository }}
+ PR_NUMBER: ${{ github.event.issue.number }}
+ COMMENT_AUTHOR: ${{ github.event.comment.user.login }}
+ run: |
+ comment_file="$RUNNER_TEMP/doris-repo-review-comment.md"
+ jq -r '.comment.body' "$GITHUB_EVENT_PATH" > "$comment_file"
+
+ pr_info="$(gh api "repos/${REPO}/pulls/${PR_NUMBER}")"
+ head_sha="$(jq -r '.head.sha' <<<"$pr_info")"
+ live_base_sha="$(jq -r '.base.sha' <<<"$pr_info")"
+ pr_state="$(jq -r '.state' <<<"$pr_info")"
+ if [ "$pr_state" != "open" ]; then
+ echo "Review comment ignored: pull request is ${pr_state}."
+ echo "valid=false" >> "$GITHUB_OUTPUT"
+ exit 0
+ fi
+
+ if ! permission_info="$(gh api
"repos/${REPO}/collaborators/${COMMENT_AUTHOR}/permission")"; then
+ echo "Review comment ignored: cannot verify ${COMMENT_AUTHOR}'s
repository permission."
+ echo "valid=false" >> "$GITHUB_OUTPUT"
+ exit 0
+ fi
+ if ! comment_author_permission="$(jq -er '.permission | strings'
<<<"$permission_info")"; then
+ echo "Review comment ignored: cannot read ${COMMENT_AUTHOR}'s
repository permission."
+ echo "valid=false" >> "$GITHUB_OUTPUT"
+ exit 0
+ fi
+ if [[ "$comment_author_permission" != "write" &&
"$comment_author_permission" != "admin" ]]; then
+ echo "Review comment ignored: ${COMMENT_AUTHOR} does not have
write permission."
+ echo "valid=false" >> "$GITHUB_OUTPUT"
+ exit 0
+ fi
+
+ if ! reviewed_base_sha="$(python3
.github/scripts/validate_review_pass_comment.py \
+ extract-base --comment-file "$comment_file")"; then
+ echo "Review comment ignored: cannot read its reviewed base."
+ echo "valid=false" >> "$GITHUB_OUTPUT"
+ exit 0
+ fi
+ if ! base_compare="$(gh api \
+
"repos/${REPO}/compare/${reviewed_base_sha}...${live_base_sha}")"; then
+ echo "Review comment ignored: cannot compare its base with the
current PR base."
+ echo "valid=false" >> "$GITHUB_OUTPUT"
+ exit 0
+ fi
+ base_compare_status="$(jq -r '.status' <<<"$base_compare")"
+ if [ "$reviewed_base_sha" = "$live_base_sha" ]; then
+ reviewed_base_committed_at=""
+ live_base_committed_at=""
+ else
+ if ! reviewed_base_committed_at="$(gh api \
+ "repos/${REPO}/commits/${reviewed_base_sha}" --jq
'.commit.committer.date')" || \
+ ! live_base_committed_at="$(gh api \
+ "repos/${REPO}/commits/${live_base_sha}" --jq
'.commit.committer.date')"; then
+ echo "Review comment ignored: cannot resolve base commit times."
+ echo "valid=false" >> "$GITHUB_OUTPUT"
+ exit 0
+ fi
+ fi
+
+ if validation_output="$(python3
.github/scripts/validate_review_pass_comment.py validate \
+ --comment-file "$comment_file" \
+ --repository "$REPO" \
+ --pr-number "$PR_NUMBER" \
+ --head-sha "$head_sha" \
+ --live-base-sha "$live_base_sha" \
+ --base-compare-status "$base_compare_status" \
+ --reviewed-base-committed-at "$reviewed_base_committed_at" \
+ --live-base-committed-at "$live_base_committed_at" \
+ --comment-author "$COMMENT_AUTHOR" \
+ --comment-author-permission "$comment_author_permission" 2>&1)";
then
+ echo "$validation_output"
+ echo "valid=true" >> "$GITHUB_OUTPUT"
+ echo "head_sha=$head_sha" >> "$GITHUB_OUTPUT"
+ else
+ echo "$validation_output"
+ echo "valid=false" >> "$GITHUB_OUTPUT"
+ fi
+
+ - name: Mark code review as success
+ if: steps.validation.outputs.valid == 'true'
+ env:
+ GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ REPO: ${{ github.repository }}
+ HEAD_SHA: ${{ steps.validation.outputs.head_sha }}
+ COMMENT_URL: ${{ github.event.comment.html_url }}
+ run: |
+ gh api "repos/${REPO}/statuses/${HEAD_SHA}" \
Review Comment:
[major] Publish this decision on a PR/base-specific evaluated object rather
than this reusable head context. This endpoint stores only `HEAD_SHA` plus the
generic `code-review` context, so a valid receipt for PR A also satisfies PR B
when both use the same head commit—even if B targets a different base and has a
different diff. This occurs in Doris history (for example, #66211 and #66212
were concurrently open on the same SHA against different bases), and
`sync-status` compounds it by copying any success using only that SHA. Checking
all currently open PRs is only a conservative interim guard: a later PR can
reuse H after A closes and immediately inherit H's persistent success, while
one H status cannot represent divergent decisions for A and B. Please gate the
PR/base-specific merge commit or another PR-scoped check, and have
`sync-status` recompute from that scoped source instead of the SHA-wide
aggregate.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
