Skip to content

fix(cloud): authenticate task requests before payload validation (#1134) - #1132

Closed
groupthinking with Copilot wants to merge 6 commits into
mainfrom
copilot/fix-payload-parameter-issue
Closed

fix(cloud): authenticate task requests before payload validation (#1134)#1132
groupthinking with Copilot wants to merge 6 commits into
mainfrom
copilot/fix-payload-parameter-issue

Conversation

Copilot AI commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Canonical issue

Closes #1134

Parent program: #898

Outcome

/api/v3/process-video-task now authenticates the Cloud Tasks caller before parsing the body. Unauthorized malformed calls return 403; authorized malformed calls retain strict 422 Pydantic validation; authorized valid calls preserve the existing handler behavior.

Scope

Included exactly two files:

  • src/youtube_extension/backend/cloud_api_endpoints.py
  • tests/unit/test_cloud_routes.py

The handler now parses CloudTaskPayload only after the existing X-CloudTasks-TaskName gate and translates Pydantic ValidationError to 422. The focused tests cover malformed unauthorized and malformed authorized requests.

Excluded: Cloud Tasks queue construction, payload format changes, authentication expansion, production configuration, and unrelated refactors.

Risk and rollback

Risk is low and isolated to request validation and OpenAPI documentation. Roll back by reverting the PR's two-file diff. No production mutation is authorized.

Exact-head verification

Current head: 42939e0012ed2d3bab18de445655c98da0ad6980

  • Focused test: PYTHONPATH=src pytest tests/unit/test_cloud_routes.py -k process_video_task --no-cov -q
  • CI 30638001041
  • Coverage 30638000686
  • CodeQL 30637999823
  • Security Scan 30638003330
  • Secret Scan 30637999959
  • Dependency Review 30638000847
  • E2E 30637999965 is repository-skipped
  • Zero unresolved review threads
  • Copilot and CodeRabbit independently reviewed exact head 42939e0012ed2d3bab18de445655c98da0ad6980; no blocking defects remain
  • Exact-head Vercel preview was provider-canceled for connector-authored commit provenance; production is unchanged
  • Keep draft until the rebound metadata/truth gate is terminal and independently verified

Execution receipt

  • Agent login: groupthinking
  • Run ID: eventrelay-daily-control-20260731-1317z
  • Canonical branch/PR: copilot/fix-payload-parameter-issue / fix(cloud): authenticate task requests before payload validation (#1134) #1132
  • Claimed: 2026-07-31T13:22:24Z
  • Latest heartbeat: 2026-07-31T14:24:12.502Z
  • Exact head: 42939e0012ed2d3bab18de445655c98da0ad6980
  • Continuation note: Copilot restored the OpenAPI request-body contract on the canonical branch. The blocker watch then fixed CodeRabbit's invalid UTF-8 serialization finding by omitting raw Pydantic input and strengthening the focused regression on the same branch.

@vercel

vercel Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
v0-uvai Canceled Canceled Jul 31, 2026 2:17pm

@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown

🔍 PR Validation

✅ Current validation passed.

@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 42939e0.
Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

Scanned Files

None

@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown

Agent Completion Truth Gate: BLOCKED

Reasons: evidence_collection_failed, missing_agent_result, missing_copilot_current_head_review, required_checks_failed, draft_pr, focused_tests_failed

Machine-readable verdict
{
  "details": {
    "collection_errors": [
      "missing_intent_snapshot"
    ],
    "focused_test_failures": [
      "tests/unit/test_cloud_routes.py"
    ],
    "identity_projection": {
      "agent_login": "groupthinking",
      "issue_number": 1134,
      "run_id": "eventrelay-daily-control-20260731-1317z"
    }
  },
  "reasons": [
    "evidence_collection_failed",
    "missing_agent_result",
    "missing_copilot_current_head_review",
    "required_checks_failed",
    "draft_pr",
    "focused_tests_failed"
  ],
  "verdict": "blocked"
}

Workflow evidence

Copilot AI changed the title [WIP] Investigate and fix payload parameter issue in process_video_task_handler Prioritize Cloud Tasks auth before v3 task payload validation Jul 31, 2026
Copilot AI requested a review from groupthinking July 31, 2026 08:50

Copy link
Copy Markdown
Owner

Blocker-watch containment receipt

PR #1132 is a new, self-contained two-file implementation at exact head 5746ba2387498b53b347dde630a1efe41323781e. The behavior change is plausible and its required code/security workflows and Vercel Preview are green, but it cannot advance because:

  • No canonical issue is linked. The PR body leaves “Canonical issue” blank and the lock manifest uses issue_number: 0.
  • The completion payload is invalid. Truth-gate run 30617800139 rejects issue.number, policy.agent_login, and policy.run_id.
  • No independent current-head review evidence exists.
  • Repository search found no existing focused issue or competing PR that owns this exact 403-before-422 repair.

Containment completed: applied needs-triage; PR remains draft and mergeable; branch and exact code head are preserved. No code, deployment, credential, merge, or branch-history mutation was performed. The next control pass must establish a focused child issue and declare this existing PR/branch canonical before remediation continues.

@groupthinking groupthinking changed the title Prioritize Cloud Tasks auth before v3 task payload validation fix(cloud): authenticate task requests before payload validation (#1134) Jul 31, 2026

Copy link
Copy Markdown
Owner

@copilot review

Please review exact head 5746ba2387498b53b347dde630a1efe41323781e against focused issue #1134. The PR remains draft; do not merge.

Copilot AI commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

@copilot review

Please review exact head 5746ba2387498b53b347dde630a1efe41323781e against focused issue #1134. The PR remains draft; do not merge.

Reviewed head 5746ba2 against #1134.

Acceptance criteria met at that head:

  • The X-CloudTasks-TaskName gate runs before CloudTaskPayload parsing, so unauthorized malformed calls short-circuit to 403.
  • Authorized malformed payloads raise 422 via the ValidationError translation.
  • Authorized valid payloads are unchanged; the 422 raise sits outside the broad except Exception block, so it is not masked as 500.

One defect found and fixed: for an authorized request with a non-JSON body, ValidationError.errors() returns an input value of type bytes, which FastAPI cannot serialize — the response failed with a TypeError instead of 422. The detail is now passed through jsonable_encoder, with a regression test covering a raw non-JSON body.

Fixed in dd0f5c4. tests/unit/test_cloud_routes.py: 92 passed. PR left in draft.

Copy link
Copy Markdown
Owner

@copilot review
@coderabbitai review

Please independently review exact head 33a267d5905265dcc80a5b25c3e2de1c053509d8 against focused issue #1134, including the new jsonable_encoder path and raw non-JSON regression test. The PR remains draft; do not merge or change production.

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== Repository state and requested commit =='
git rev-parse HEAD
git status --short
git show --no-ext-diff --format=fuller --no-renames --stat 33a267d5905265dcc80a5b25c3e2de1c053509d8
git diff --no-ext-diff --no-renames 33a267d5905265dcc80a5b25c3e2de1c053509d8^ 33a267d5905265dcc80a5b25c3e2de1c053509d8 -- \
  src/youtube_extension/backend/cloud_api_endpoints.py \
  tests/unit/test_cloud_routes.py

printf '%s\n' '== Source outlines =='
ast-grep outline src/youtube_extension/backend/cloud_api_endpoints.py --items all --view expanded
ast-grep outline tests/unit/test_cloud_routes.py --items all --view expanded

printf '%s\n' '== Relevant implementation and test references =='
rg -n -C 8 --glob 'cloud_api_endpoints.py' \
  'process_video_task|CloudTaskPayload|ValidationError|jsonable_encoder|X-CloudTasks-TaskName' \
  src/youtube_extension/backend
rg -n -C 8 --glob 'test_cloud_routes.py' \
  'process_video_task|CloudTaskPayload|ValidationError|jsonable_encoder|X-CloudTasks-TaskName|non.?JSON|malformed' \
  tests/unit

Length of output: 13163


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== Locate cloud route tests =='
fd -HI -t f '.*cloud.*route.*\.py$|.*route.*cloud.*\.py$|test_.*cloud.*\.py$' . \
  -E .git -E node_modules -E dist -E build | sort
rg -n -C 4 \
  'test_process_video_task_with_header_invalid_json_returns_422|process_video_task_handler|/api/v3/process-video-task|X-CloudTasks-TaskName' \
  . -g '*.py' -g '!**/.git/**' -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**'

printf '%s\n' '== Handler and route registration context =='
sed -n '145,235p' src/youtube_extension/backend/cloud_api_endpoints.py
sed -n '420,475p' src/youtube_extension/backend/cloud_api_endpoints.py

printf '%s\n' '== Full PR range relative to merge base =='
base="$(git merge-base HEAD HEAD^)"
printf 'parent=%s\n' "$base"
git diff --no-ext-diff --no-renames --stat "$base" HEAD
git diff --no-ext-diff --no-renames "$base" HEAD -- \
  src/youtube_extension/backend/cloud_api_endpoints.py

printf '%s\n' '== Dependency declarations relevant to FastAPI and Pydantic =='
rg -n -i -C 2 'fastapi|pydantic' \
  pyproject.toml requirements.txt requirements*.txt setup.cfg setup.py 2>/dev/null || true

printf '%s\n' '== Check whether the committed test path exists in this commit tree =='
git ls-tree -r --name-only HEAD | rg '(^|/)test_cloud_routes\.py$|(^|/)tests/unit/'

Length of output: 15856


🏁 Script executed:

#!/bin/bash
set -euo pipefail

test_path='tests/unit/test_cloud_routes.py'
parent='5746ba2387498b53b347dde630a1efe41323781e'
head='33a267d5905265dcc80a5b25c3e2de1c053509d8'

printf '%s\n' '== Exact-head focused test section =='
git show "${head}:${test_path}" | nl -ba | sed -n '1,115p'
git show "${head}:${test_path}" | nl -ba | sed -n '540,690p'

printf '%s\n' '== Focused test diff against parent =='
git diff --no-ext-diff --no-renames "$parent" "$head" -- "$test_path"

printf '%s\n' '== All exact-head task-route test names and assertions =='
git show "${head}:${test_path}" | rg -n -C 6 \
  'def test_.*process_video_task|/api/v3/process-video-task|X-CloudTasks-TaskName|status_code|response\.json'

printf '%s\n' '== Parent task-route test section =='
git show "${parent}:${test_path}" | nl -ba | sed -n '555,665p'

printf '%s\n' '== Test dependencies and application fixture imports =='
git show "${head}:${test_path}" | rg -n -C 5 \
  'TestClient|FastAPI|setup_cloud_api_endpoints|_build_app|monkeypatch|AsyncMock'

Length of output: 1408


🏁 Script executed:

#!/bin/bash
set -euo pipefail

test_path='tests/unit/test_cloud_routes.py'
parent='5746ba2387498b53b347dde630a1efe41323781e'
head='33a267d5905265dcc80a5b25c3e2de1c053509d8'

number_lines() { awk '{ printf "%6d\t%s\n", NR, $0 }'; }

printf '%s\n' '== Exact-head focused test section =='
git show "${head}:${test_path}" | sed -n '1,115p' | number_lines
git show "${head}:${test_path}" | sed -n '540,690p' | awk '{ printf "%6d\t%s\n", NR + 539, $0 }'

printf '%s\n' '== Focused test diff against parent =='
git diff --no-ext-diff --no-renames "$parent" "$head" -- "$test_path"

printf '%s\n' '== All exact-head task-route test names and assertions =='
git show "${head}:${test_path}" | rg -n -C 6 \
  'def test_.*process_video_task|/api/v3/process-video-task|X-CloudTasks-TaskName|status_code|response\.json'

printf '%s\n' '== Parent task-route test section =='
git show "${parent}:${test_path}" | sed -n '555,665p' | awk '{ printf "%6d\t%s\n", NR + 554, $0 }'

printf '%s\n' '== Test dependencies and application fixture imports =='
git show "${head}:${test_path}" | rg -n -C 5 \
  'TestClient|FastAPI|setup_cloud_api_endpoints|_build_app|monkeypatch|AsyncMock'

Length of output: 113649


🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='33a267d5905265dcc80a5b25c3e2de1c053509d8'
parent='5746ba2387498b53b347dde630a1efe41323781e'

printf '%s\n' '== Direct parent implementation change =='
git show --no-ext-diff --format=fuller --no-renames --stat "$parent"
git show --no-ext-diff --format= --no-renames "$parent" -- \
  src/youtube_extension/backend/cloud_api_endpoints.py \
  tests/unit/test_cloud_routes.py

printf '%s\n' '== Commit ancestry and available refs =='
git log --oneline --decorate -8 "$head"
git branch -a --contains "$head" || true
git remote -v
git for-each-ref --format='%(refname:short) %(objectname)' refs/heads refs/remotes | sed -n '1,160p'

printf '%s\n' '== GitHub PR `#1132` metadata and changed-file list =='
gh pr view 1132 --repo groupthinking/EventRelay \
  --json number,state,isDraft,headRefName,headRefOid,baseRefName,baseRefOid,title,body,commits,files \
  --jq '{number,state,isDraft,headRefName,headRefOid,baseRefName,baseRefOid,title,files:[.files[]|{path,additions,deletions}],commits:[.commits[]|{oid,messageHeadline}]}'

Length of output: 6254


@groupthinking, I found one blocking defect in the exact head 33a267d5905265dcc80a5b25c3e2de1c053509d8.

Blocker — invalid UTF-8 request bytes can still produce a 500 response.
src/youtube_extension/backend/cloud_api_endpoints.py, Line 175 passes exc.errors() to jsonable_encoder. A Pydantic JSON parsing error includes the raw request body in its input field. FastAPI's default bytes encoder uses strict UTF-8 decoding. Therefore, a request body such as b"\xff" can raise UnicodeDecodeError while the 422 response is serialized.

The new regression test at tests/unit/test_cloud_routes.py, Lines 639-647 uses b"not-json". Those bytes are valid UTF-8, so the test does not cover this failure mode.

Use exc.errors(include_input=False) before serialization, or define a byte encoder that cannot fail. Add a regression test that sends invalid UTF-8 bytes with X-CloudTasks-TaskName and verifies a 422 response.

The validation-order change itself is correct. The handler checks X-CloudTasks-TaskName before it reads or validates the request body. The existing tests cover the 403 and 422 ordering requirements.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@groupthinking, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 45 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 822d98b5-7288-4312-aff6-bac90a75b6a0

📥 Commits

Reviewing files that changed from the base of the PR and between abd9332 and 42939e0.

⛔ Files ignored due to path filters (1)
  • tests/unit/test_cloud_routes.py is excluded by !tests/**
📒 Files selected for processing (1)
  • src/youtube_extension/backend/cloud_api_endpoints.py
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Fix failing CI checks
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch copilot/fix-payload-parameter-issue
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch copilot/fix-payload-parameter-issue

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

@copilot review
@coderabbitai review
...

Reviewed head 33a267d against #1134.

Correct at that head:

  • The X-CloudTasks-TaskName gate precedes CloudTaskPayload.model_validate_json, so unauthorized malformed calls short-circuit to 403; verified by test_process_video_task_no_header_malformed_payload_still_403.
  • Authorized malformed payloads raise 422, and the raise sits above the broad except Exception block, so it is not masked as 500.
  • jsonable_encoder(exc.errors()) correctly handles the non-JSON case, where the input value is bytes and the raw errors() list is not serializable. The regression test asserts both 422 and detail[0]["type"] == "json_invalid", so it fails if the encoder is removed.

One regression found and fixed: moving the body out of the handler signature dropped the endpoint's requestBody from the generated OpenAPI schema (verified: app.openapi() returned requestBody: None for /api/v3/process-video-task). The route now declares openapi_extra with CloudTaskPayload.model_json_schema(), restoring the documented schema, with a test asserting the body is required and lists video_id/video_url.

Fixed in 53f560d. tests/unit/test_cloud_routes.py: 93 passed; CodeQL: 0 alerts. PR left in draft; no production change.

Out-of-scope observation (not changed here, since #1134 declares a two-file scope): /api/v1/process-video-task in src/youtube_extension/backend/api/v1/router.py still declares payload: dict[str, Any] in its signature, so an unauthenticated call with a malformed body is rejected by FastAPI with 422 before its own 403 header check runs — the same ordering defect this PR repairs for v3.

Copy link
Copy Markdown
Owner

@copilot review
@coderabbitai review

Please independently review exact head 42939e0012ed2d3bab18de445655c98da0ad6980 against focused issue #1134. Verify the OpenAPI request-body restoration and the invalid UTF-8 422 path using exc.errors(include_input=False). The PR remains draft; do not merge or change production.

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='42939e0012ed2d3bab18de445655c98da0ad6980'
source_file='src/youtube_extension/backend/cloud_api_endpoints.py'
test_file='tests/unit/test_cloud_routes.py'

printf '%s\n' '== Requested commit and checked-out state =='
git rev-parse HEAD
git cat-file -t "$head"
git status --short
git show --no-ext-diff --format=fuller --no-renames --stat "$head"

printf '%s\n' '== PR metadata =='
gh pr view 1132 --repo groupthinking/EventRelay \
  --json number,state,isDraft,headRefName,headRefOid,baseRefOid,title,body,files \
  --jq '{number,state,isDraft,headRefName,headRefOid,baseRefOid,title,files:[.files[]|{path,additions,deletions}]}'

printf '%s\n' '== Full requested-head change range =='
base="$(git merge-base "$head" "$(gh pr view 1132 --repo groupthinking/EventRelay --json baseRefOid --jq .baseRefOid)")"
printf 'merge_base=%s\n' "$base"
git diff --no-ext-diff --no-renames --stat "$base" "$head"
git diff --no-ext-diff --no-renames "$base" "$head" -- "$source_file" "$test_file"

printf '%s\n' '== Source structure =='
ast-grep outline "$source_file" --items all --view expanded
ast-grep outline "$test_file" --items all --view expanded

printf '%s\n' '== Route implementation and schema-relevant declarations =='
rg -n -C 12 \
  'process_video_task|CloudTaskPayload|ValidationError|include_input|X-CloudTasks-TaskName|Request|Body|response_model' \
  "$source_file"

printf '%s\n' '== Focused task-route tests =='
rg -n -C 10 \
  'def test_.*process_video_task|/api/v3/process-video-task|X-CloudTasks-TaskName|invalid UTF|invalid_utf|\\xff|include_input|openapi|requestBody' \
  "$test_file"

printf '%s\n' '== Exact source and test ranges =='
nl -ba "$source_file" | sed -n '130,245p'
nl -ba "$source_file" | sed -n '410,490p'
nl -ba "$test_file" | sed -n '530,710p'

Length of output: 17851


🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='42939e0012ed2d3bab18de445655c98da0ad6980'
source_file='src/youtube_extension/backend/cloud_api_endpoints.py'
test_file='tests/unit/test_cloud_routes.py'
base='abd93326b1a58cb45aaf1c120d3737174b74a201'

printf '%s\n' '== Locate exact committed test path =='
git ls-tree -r --name-only "$head" | rg '(^|/)(test_cloud_routes|cloud_api_endpoints)\.py$'

printf '%s\n' '== Exact-head focused tests from Git object =='
git show "$head:$test_file" | nl -ba | sed -n '1,130p'
git show "$head:$test_file" | nl -ba | sed -n '560,690p'

printf '%s\n' '== Test changes across full PR range =='
git diff --no-ext-diff --no-renames "$base" "$head" -- "$test_file"

printf '%s\n' '== Route changes and surrounding parent context =='
git show "$base:$source_file" | nl -ba | sed -n '145,205p'
git show "$head:$source_file" | nl -ba | sed -n '145,215p'

printf '%s\n' '== Dependency declarations for FastAPI, Starlette, and Pydantic =='
for file in pyproject.toml requirements.txt requirements-dev.txt setup.cfg setup.py; do
  if git cat-file -e "$head:$file" 2>/dev/null; then
    printf '%s\n' "--- $file ---"
    git show "$head:$file" | rg -n -i -C 3 'fastapi|starlette|pydantic' || true
  fi
done

printf '%s\n' '== Full exact-head schema/model references =='
git grep -n -E 'CloudTaskPayload|process-video-task|model_json_schema|openapi_extra|model_validate_json|errors\(include_input' "$head" -- '*.py'

Length of output: 1947


🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='42939e0012ed2d3bab18de445655c98da0ad6980'
source_file='src/youtube_extension/backend/cloud_api_endpoints.py'
test_file='tests/unit/test_cloud_routes.py'
base='abd93326b1a58cb45aaf1c120d3737174b74a201'
number_lines() { awk '{printf "%6d  %s\n", NR, $0}'; }

printf '%s\n' '== Exact-head test setup and focused test section =='
git show "$head:$test_file" | sed -n '1,140p' | number_lines
git show "$head:$test_file" | sed -n '560,690p' | awk '{printf "%6d  %s\n", NR+559, $0}'

printf '%s\n' '== Complete test changes across the PR range =='
git diff --no-ext-diff --no-renames "$base" "$head" -- "$test_file"

printf '%s\n' '== Route before and after the PR =='
git show "$base:$source_file" | sed -n '145,210p' | awk '{printf "%6d  %s\n", NR+144, $0}'
git show "$head:$source_file" | sed -n '145,220p' | awk '{printf "%6d  %s\n", NR+144, $0}'

printf '%s\n' '== Dependency declarations =='
for file in pyproject.toml requirements.txt requirements-dev.txt setup.cfg setup.py; do
  if git cat-file -e "$head:$file" 2>/dev/null; then
    printf '%s\n' "--- $file ---"
    git show "$head:$file" | rg -n -i -C 3 'fastapi|starlette|pydantic' || true
  fi
done

printf '%s\n' '== Exact-head references to the endpoint and payload model =='
git grep -n -E 'CloudTaskPayload|process-video-task|model_json_schema|openapi_extra|model_validate_json|errors\(include_input' "$head" -- '*.py'

Length of output: 28107


@groupthinking I completed an independent static review of exact head 42939e0012ed2d3bab18de445655c98da0ad6980 against #1134.

I found no blocking defects.

  • process_video_task_handler checks X-CloudTasks-TaskName before it reads or validates the request body.
  • Unauthorized malformed payloads now return 403.
  • Authorized malformed payloads return 422.
  • The handler uses exc.errors(include_input=False). This removes the raw Pydantic input value from the response. Invalid UTF-8 bytes cannot reach FastAPI's JSON bytes encoder through this error detail.
  • The regression test sends b"\xff" with X-CloudTasks-TaskName and verifies 422 with json_invalid.
  • The openapi_extra.requestBody declaration restores the JSON request-body schema for CloudTaskPayload.
  • The OpenAPI regression test verifies that the body is required and that video_id and video_url are required fields.

I verified the requested commit is the current PR head. I did not execute repository tests because the review sandbox permits static inspection only.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copilot AI commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

@copilot review
@coderabbitai review
...

Reviewed head 42939e0 against #1134. No changes required; nothing new pushed.

Ordering (403 before 422) — the X-CloudTasks-TaskName gate still precedes CloudTaskPayload.model_validate_json(await request.body()). Verified: unauthorized + malformed body → 403; authorized + missing video_url422 with detail[0].type == "missing". The 422 raise remains above the broad except Exception, so it is not masked as 500.

Invalid UTF-8 path (exc.errors(include_input=False))POST with body b"\xff" and a task header returns 422 with {"type": "json_invalid", "loc": [], "msg": "Invalid JSON: ...", "ctx": {"error": "..."}}. No bytes remain in the payload, so jsonable_encoder is no longer needed and the response serializes natively. Mutation-checked the guard: reverting to exc.errors() makes test_process_video_task_with_header_invalid_utf8_returns_422 fail with TypeError: Object of type bytes is not JSON serializable, so the regression test is effective rather than vacuous.

include_input is available at the declared floor: installed pydantic==2.5.0 (matching pyproject.toml's pydantic>=2.5.0) in a clean venv and errors(include_input=False) returns the same serializable shape — no version drift risk.

OpenAPI request bodyapp.openapi() for /api/v3/process-video-task now yields requestBody.required: true with the CloudTaskPayload schema (required: ["video_id", "video_url"], plus priority/callback_url/metadata). CloudTaskPayload has no nested models, so the ref_template produces no dangling $refs.

tests/unit/test_cloud_routes.py: 93 passed.

CI at exact head — CI, CodeQL, Coverage, Security Scan, Secret Scan, and Dependency Review are green; E2E is repository-skipped. The three failures (PR Checksagent-completion/truth-gate, Agent completion enforcement, branch-cleanup.yml) are governance/lease publication gates, not code or test failures; they are the pending metadata/truth gate already noted in the PR body. Left in draft; no production change.

Copy link
Copy Markdown
Owner

Closing in favour of #1361

Both PRs implement "authenticate Cloud Tasks requests before payload validation". Measured against current main:

#1132 (this) #1361
Files changed 233 2
Lines +24,941 / −4,737 +59 / −3
Commits ahead 1,503 1
Commits behind main 55 16

This branch has absorbed a very large amount of unrelated history; a two-file authentication-ordering fix should not present as a 233-file diff. #1361 is the same fix expressed in 2 files and 59 lines.

This PR has also been parked since Jul 31 on missing_intent_snapshot — the retroactive-intent rule that made the truth gate unsatisfiable here. That root cause is fixed in #1377, so the successor does not inherit the block.

Closing this; #1361 is canonical for the Cloud Tasks auth fix. Branch retained and will be archive-tagged before any deletion.


Generated by Claude Code

Copy link
Copy Markdown
Owner

Correction to the numbers above. The close stands, but I gave the wrong mechanism.

The "233 files / +24,941" figure was wrong — it came from a diff against an empty merge base, which degraded to a working-tree diff. The real situation:

$ git merge-base origin/main origin/copilot/fix-payload-parameter-issue
(empty — no common ancestor)

This branch shares no ancestry with current main — it predates the secret-purge force-push (CLAUDE.md, Repo Hygiene), so it is orphaned, not merely stale, and no rebase recovers it.

Disposition is unchanged: #1361 is canonical for the Cloud Tasks auth fix, and it is rooted in current main.


Generated by Claude Code

groupthinking added a commit that referenced this pull request Aug 29, 2026
Port the canonical two-file artifact from PR #1132 (exact head
42939e0) onto current main:
/api/v3/process-video-task now checks the X-CloudTasks-TaskName gate
before parsing the body, so unauthorized malformed calls return 403
while authorized malformed calls keep strict 422 validation (including
invalid UTF-8 bodies via errors(include_input=False)). The request-body
schema stays documented in OpenAPI via openapi_extra.

Generated with [Linear](https://linear.app/myxstack/issue/GRV-198/execution-enforce-cloud-tasks-authentication-before-payload-validation#agent-session-95a9c398)

Co-authored-by: linear-code[bot] <222613912+linear-code[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Execution: enforce Cloud Tasks authentication before payload validation (PR #1132)

2 participants