-
Notifications
You must be signed in to change notification settings - Fork 0
fix: harden ingestion worker leases and address review follow-ups #937
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
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
ba8273c
fix: harden ingestion worker leases and address review follow-ups
cursoragent 97cf7bf
fix: resolve python3 on Linux and polish ingestion worker lease handling
cursoragent 672d06e
style: format document extractor after python bin fix
cursoragent 41559d4
fix: complete deferred review follow-ups and harden PR policy workflow
cursoragent 430ee1e
fix: address CodeRabbit review findings before merge
cursoragent cc78491
style: format ingestion worker after conservative completion guard
cursoragent 3aa1c4a
fix: trim PYTHON_BIN before spawning python extractor
cursoragent afad557
merge: sync main into ingestion review follow-ups branch
cursoragent File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| import fs from "node:fs"; | ||
|
|
||
| import { yamlBlock } from "./yaml-contract.mjs"; | ||
|
|
||
| const workflowPath = ".github/workflows/pr-policy.yml"; | ||
| const ciWorkflowPath = ".github/workflows/ci.yml"; | ||
| const workflow = fs.readFileSync(workflowPath, "utf8"); | ||
| const ciWorkflow = fs.readFileSync(ciWorkflowPath, "utf8"); | ||
| const githubScriptPin = "3a2844b7e9c422d3c10d287c895573f7108da1b3"; | ||
|
|
||
| const failures = []; | ||
|
|
||
| function collectCheckoutRefs(block) { | ||
| return block | ||
| .split(/\r?\n/) | ||
| .map((line) => line.match(/^\s+ref:\s*(.+?)\s*(?:#.*)?$/)?.[1]?.trim()) | ||
| .filter(Boolean); | ||
| } | ||
|
|
||
| function assertCheckoutRefs(block, label, { allowed = [], forbidden = [] }) { | ||
| const refs = collectCheckoutRefs(block); | ||
| for (const ref of refs) { | ||
| if (forbidden.some((pattern) => pattern.test(ref))) { | ||
| failures.push(`${label} must not checkout untrusted ref ${ref}.`); | ||
| } | ||
| if (allowed.length > 0 && !allowed.includes(ref)) { | ||
| failures.push(`${label} checkout ref ${ref} is not in the allowed trusted set.`); | ||
| } | ||
| } | ||
| if (refs.length === 0) { | ||
| failures.push(`${label} is missing an actions/checkout ref declaration.`); | ||
| } | ||
| } | ||
|
|
||
| function assertPersistCredentialsFalse(block, label) { | ||
| if (!/persist-credentials:\s*false/.test(block)) { | ||
| failures.push(`${label} must set persist-credentials: false on checkout steps.`); | ||
| } | ||
| if (/persist-credentials:\s*true/.test(block)) { | ||
| failures.push(`${label} must not persist checkout credentials.`); | ||
| } | ||
| } | ||
|
|
||
| const policyJob = yamlBlock(workflow, "policy:", 2); | ||
| if (!policyJob) { | ||
| failures.push("pr-policy.yml is missing the policy job."); | ||
| } else { | ||
| const checkoutStep = yamlBlock(policyJob, "- name: Checkout trusted policy", 6); | ||
| if (!checkoutStep) { | ||
| failures.push("pr-policy.yml policy job is missing the trusted checkout step."); | ||
| } else { | ||
| assertCheckoutRefs(checkoutStep, "PR policy trusted checkout", { | ||
| allowed: ["${{ github.workflow_sha }}"], | ||
| forbidden: [/github\.event\.pull_request\.head/, /github\.base_ref/, /github\.event\.pull_request\.base\.sha/], | ||
| }); | ||
| assertPersistCredentialsFalse(checkoutStep, "PR policy trusted checkout"); | ||
| } | ||
|
|
||
| const validateStep = yamlBlock(policyJob, "- name: Validate pull request evidence", 6); | ||
| if (!validateStep) { | ||
| failures.push("pr-policy.yml policy job is missing the validation step."); | ||
| } else { | ||
| if (!validateStep.includes("GITHUB_WORKSPACE}/scripts/pr-policy.mjs")) { | ||
| failures.push("PR policy validation must import scripts/pr-policy.mjs from the trusted checkout."); | ||
| } | ||
| if (!validateStep.includes(`uses: actions/github-script@${githubScriptPin} # v9.0.0`)) { | ||
| failures.push("PR policy validation must use the pinned github-script action."); | ||
| } | ||
| } | ||
|
|
||
| if ( | ||
| !/^permissions:\s*$/m.test(workflow) || | ||
| !workflow.includes("contents: read") || | ||
| !workflow.includes("pull-requests: read") | ||
| ) { | ||
| failures.push("pr-policy.yml must declare read-only workflow permissions."); | ||
| } | ||
| if (/pull-requests:\s*write/.test(policyJob) || /contents:\s*write/.test(policyJob)) { | ||
| failures.push("pr-policy.yml policy job must not request write permissions."); | ||
| } | ||
| } | ||
|
|
||
| const syncJob = yamlBlock(ciWorkflow, "sync-pr-policy-body:", 2); | ||
| if (!syncJob) { | ||
| failures.push("ci.yml is missing the sync-pr-policy-body job."); | ||
| } else { | ||
| const trustedCheckout = yamlBlock(syncJob, "- name: Checkout trusted policy metadata", 6); | ||
| if (!trustedCheckout) { | ||
| failures.push("sync-pr-policy-body is missing the trusted policy metadata checkout step."); | ||
| } else { | ||
| assertCheckoutRefs(trustedCheckout, "sync-pr-policy-body trusted policy checkout", { | ||
| allowed: ["${{ github.event.pull_request.base.sha }}"], | ||
| forbidden: [/github\.event\.pull_request\.head/], | ||
| }); | ||
| assertPersistCredentialsFalse(trustedCheckout, "sync-pr-policy-body trusted policy checkout"); | ||
| } | ||
|
|
||
| const applyStep = yamlBlock(syncJob, "- name: Apply PR_POLICY_BODY.md to pull request description", 6); | ||
| if (!applyStep) { | ||
| failures.push("sync-pr-policy-body is missing the PR body sync step."); | ||
| } else { | ||
| if (!applyStep.includes("trusted-policy/scripts/pr-policy.mjs")) { | ||
| failures.push("sync-pr-policy-body must import pr-policy.mjs from the trusted base checkout only."); | ||
| } | ||
| if (!applyStep.includes("existingCheckedItems")) { | ||
| failures.push("sync-pr-policy-body must preserve existing governance attestations."); | ||
| } | ||
| if (/map\(\(item\) => `\s*-\s*\[x\]/i.test(applyStep)) { | ||
| failures.push("sync-pr-policy-body must not synthesize completed Clinical Governance Preflight items."); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (failures.length > 0) { | ||
| console.error("PR policy workflow guard failed:"); | ||
| for (const failure of failures) { | ||
| console.error(`- ${failure}`); | ||
| } | ||
| process.exit(1); | ||
| } | ||
|
|
||
| console.log("PR policy workflow guard passed."); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| /** Shared Python executable resolution for extraction and worker prerequisites. */ | ||
| export function resolvePythonBin(explicit = process.env.PYTHON_BIN): string { | ||
| const trimmed = explicit?.trim(); | ||
| if (trimmed) return trimmed; | ||
| return process.platform === "win32" ? "python" : "python3"; | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.