Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/ISSUE_TEMPLATE/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ contact_links:
url: https://github.com/lidge-jun/opencodex/security/advisories/new
about: Report undisclosed vulnerabilities privately to the maintainers. Do not open a public issue.
- name: Security policy
url: https://github.com/lidge-jun/opencodex/blob/main/SECURITY.md
url: https://github.com/lidge-jun/opencodex/blob/dev/SECURITY.md
about: Read the supported-version and reporting guidance before sharing security-sensitive details.
- name: Contributing guide
url: https://opencodex.me/contributing/
Expand Down
23 changes: 11 additions & 12 deletions .github/scripts/enforce-pr-target.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -165,24 +165,23 @@ describe("enforce-pr-target workflow", () => {
.split("- name: Checkout trusted PR-quality scripts")[1]
.split(/\n {6}- name:/)[0];
assert.match(checkoutStep, /actions\/checkout@[0-9a-f]{40}/);
// The trusted ref comes from a fixed set of integration branches, never
// from the PR's own base commit: a stacked child's base is another open
// PR's head, and `base.sha` would let that unpromoted commit choose the
// code that runs with this job's write-capable token.
// The trusted ref is the single integration branch, never the PR's own
// base commit: a stacked child's base is another open PR's head, and
// `base.sha` would let that unpromoted commit choose the code that runs
// with this job's write-capable token.
//
// `status` has no pull_request payload and sources from the default branch
// that supplied the privileged workflow. A `main`-targeting PR sources
// from `main` so the workflow definition and the scripts match. Everything
// else resolves to `dev`.
// that supplied the privileged workflow. Every pull_request event resolves
// to `dev`, the only integration branch.
//
// Exact equality, not fragment matching: separate checks for `status`,
// `main`, and `dev` would all pass with the operator grouping wrong or a
// surviving `base.sha` fallback.
// Exact equality, not fragment matching: separate checks for `status` and
// `dev` would both pass with the operator grouping wrong or a surviving
// `base.sha` fallback.
const ref = checkoutStep.match(/^\s*ref:\s*(.+)$/m)?.[1];
assert.ok(ref, "trusted checkout must declare ref");
assert.equal(
ref.replace(/\s+/g, " ").trim(),
"${{ github.event_name == 'status' && github.event.repository.default_branch || (github.event.pull_request.base.ref == 'main' && 'main' || 'dev') }}",
"${{ github.event_name == 'status' && github.event.repository.default_branch || 'dev' }}",
);
assert.doesNotMatch(ref, /base\.sha|head\.(?:sha|ref)/);
// Pinning the checkout ref only gates one step. A later `run:` or
Expand Down Expand Up @@ -215,7 +214,7 @@ describe("enforce-pr-target workflow", () => {
const checkouts = workflow.match(/uses:\s*actions\/checkout@[\s\S]*?(?=\n {6}- name:|$)/g) ?? [];
for (const step of checkouts) {
const stepRef = (step.match(/^\s*ref:\s*(.+)$/m)?.[1] ?? "").replace(/\s+/g, " ").trim();
assert.equal(stepRef, "${{ github.event_name == 'status' && github.event.repository.default_branch || (github.event.pull_request.base.ref == 'main' && 'main' || 'dev') }}", "every checkout must use the trusted ref");
assert.equal(stepRef, "${{ github.event_name == 'status' && github.event.repository.default_branch || 'dev' }}", "every checkout must use the trusted ref");
assert.doesNotMatch(step, /repository:/, "a checkout must not retarget its repository");
}
// Checkout is not the only way to obtain PR-controlled code. A `run:` step
Expand Down
4 changes: 2 additions & 2 deletions .github/scripts/pr-hygiene.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ describe("pr-hygiene workflow trust boundary", () => {
assert.ok(ref, "trusted checkout must declare ref");
assert.equal(
ref.replace(/\s+/g, " ").trim(),
"${{ github.event.pull_request.base.ref == 'main' && 'main' || 'dev' }}",
"dev",
);
// A stacked child PR's base is another open PR's head; neither a base nor
// a head ref may select the code that runs with the write-capable token.
Expand Down Expand Up @@ -355,7 +355,7 @@ describe("pr-hygiene workflow trust boundary", () => {
const checkouts = workflow.match(/uses:\s*actions\/checkout@[\s\S]*?(?=\n {6}- name:|$)/g) ?? [];
for (const step of checkouts) {
const stepRef = (step.match(/^\s*ref:\s*(.+)$/m)?.[1] ?? "").replace(/\s+/g, " ").trim();
assert.equal(stepRef, "${{ github.event.pull_request.base.ref == 'main' && 'main' || 'dev' }}", "every checkout must use the trusted ref");
assert.equal(stepRef, "dev", "every checkout must use the trusted ref");
assert.doesNotMatch(step, /repository:/, "a checkout must not retarget its repository");
}
// Checkout is not the only way to obtain PR-controlled code. A `run:` step
Expand Down
15 changes: 1 addition & 14 deletions .github/scripts/pr-quality-messages.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -274,17 +274,7 @@ function buildFailureSections(failures, { pr, allowedBases, defaultBase }) {
"",
`This pull request currently targets ${inlineCode(pr.base.ref)}, but pull requests must target one of ${allowedBases.map(inlineCode).join(" or ")}.`,
"",
`@${pr.user.login} Please retarget this PR to ${inlineCode(defaultBase)}. All contributions go to ${inlineCode(defaultBase)}; \`main\` receives only release promotions. See our [Contributing guide](https://lidge-jun.github.io/opencodex/contributing/) for details. Thanks! 🙏`
);
}

if (failures.some(failure => failure.code === "wrong_ancestry")) {
sections.push(
"⚠️ **Wrong branch ancestry**",
"",
`This pull request targets ${inlineCode(pr.base.ref)}, but its head appears to sit on the current ${inlineCode("main")} tip while being far behind ${inlineCode(pr.base.ref)}.`,
"",
`@${pr.user.login} Rebase onto the current ${inlineCode(pr.base.ref)} branch instead of opening from ${inlineCode("main")}. That keeps already-released commits out of the integration branch.`
`@${pr.user.login} Please retarget this PR to ${inlineCode(defaultBase)}. All contributions go to ${inlineCode(defaultBase)}. See our [Contributing guide](https://lidge-jun.github.io/opencodex/contributing/) for details. Thanks! 🙏`
);
}

Expand Down Expand Up @@ -322,9 +312,6 @@ function failureSummary(failures, { pr }) {
if (failure.code === "wrong_base") {
return `wrong base (${pr.base.ref})`;
}
if (failure.code === "wrong_ancestry") {
return "wrong ancestry";
}
if (failure.code === "bad_description") {
return `bad description (${failure.reason})`;
}
Expand Down
9 changes: 3 additions & 6 deletions .github/scripts/pr-quality-messages.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -143,19 +143,17 @@ describe("buildFailureSections", () => {
assert.match(sections, /@contributor Please retarget this PR to `dev`/);
});

it("builds ancestry, description, and screenshot sections", () => {
it("builds description and screenshot sections", () => {
const sections = buildFailureSections(
[
{ code: "wrong_ancestry" },
{ code: "bad_description", reason: "empty" },
{ code: "missing_ui_screenshot" }
],
{ pr: PR, allowedBases: ALLOWED_BASES, defaultBase: DEFAULT_BASE },
).join("\n");
assert.match(sections, /Wrong branch ancestry/);
assert.match(sections, /Pull request description/);
assert.match(sections, /UI screenshot required/);
assert.match(sections, /Rebase onto the current `main`/);
assert.doesNotMatch(sections, /Wrong branch ancestry/);
});
});

Expand All @@ -165,14 +163,13 @@ describe("failureSummary", () => {
failureSummary(
[
{ code: "wrong_base" },
{ code: "wrong_ancestry" },
{ code: "bad_description", reason: "empty" },
{ code: "missing_ui_screenshot" },
{ code: "mystery" }
],
{ pr: PR },
),
"wrong base (main); wrong ancestry; bad description (empty); missing UI screenshot; mystery",
"wrong base (main); bad description (empty); missing UI screenshot; mystery",
);
});
});
Expand Down
41 changes: 0 additions & 41 deletions .github/scripts/pr-quality.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@ const { READINESS_LATEST_DEV_BEHIND_MAX } = require(
path.join(__dirname, "pr-quality-state.cjs")
);

const ANCESTRY_BEHIND_THRESHOLD = 20;
/** Cap on ahead_by vs main so stale `dev` forks (many commits ahead of main) are not flagged. */
const ANCESTRY_AHEAD_MAIN_MAX = 5;
const MIN_SECTION_LEN = 40;
const MIN_RICH_SECTIONS = 2;
const UNSTRUCTURED_MIN_LEN = 120;
Expand Down Expand Up @@ -106,20 +103,6 @@ const MARKDOWN_REFERENCE_DEF_RE = /^\s*\[([^\]]+)\]:\s*\S+/gm;
/** Embedded HTML image with a renderable `src` (`<img ... src="...">`). */
const HTML_IMAGE_RE = /<img\b[^>]*\bsrc\s*=\s*(?:"[^"]+"|'[^']+'|[^\s>"']+)[^>]*>/i;

function isWrongAncestry({
behindMain,
behindBase,
aheadMain = 0,
threshold = ANCESTRY_BEHIND_THRESHOLD,
aheadMainMax = ANCESTRY_AHEAD_MAIN_MAX,
}) {
return (
behindMain === 0 &&
behindBase >= threshold &&
aheadMain <= aheadMainMax
);
}

function authorHasPushPermission(permission) {
return permission === "admin" || permission === "maintain" || permission === "write";
}
Expand Down Expand Up @@ -528,12 +511,6 @@ function collectPrQualityFailures({
allowedBases,
title = "",
body,
behindMain,
behindBase,
aheadMain = 0,
authorPermission,
permissionLookupFailed = false,
ancestryLookupFailed = false,
/** True when baseRef is another open PR's head (stacked child). */
stackedBase = false,
/** Issue comments; a maintainer comment waives the GUI-screenshot gate. */
Expand All @@ -550,21 +527,6 @@ function collectPrQualityFailures({
const wrongBase = !allowedBases.includes(baseRef) && !stackedBase;
if (wrongBase) {
failures.push({ code: "wrong_base" });
} else {
// Permission lookup fails closed (still evaluate ancestry). Compare API
// failures skip ancestry — zeros would falsely pass the #644 heuristic.
// Stacked children skip ancestry against the integration base; their parent
// PR is the temporary target.
const skipAncestry =
stackedBase ||
ancestryLookupFailed ||
(!permissionLookupFailed && authorHasPushPermission(authorPermission));
if (
!skipAncestry &&
isWrongAncestry({ behindMain, behindBase, aheadMain })
) {
failures.push({ code: "wrong_ancestry" });
}
}

const desc = assessPrDescription(body);
Expand All @@ -587,12 +549,9 @@ function collectPrQualityFailures({
}

module.exports = {
ANCESTRY_BEHIND_THRESHOLD,
ANCESTRY_AHEAD_MAIN_MAX,
REVIEW_READINESS_ITEMS,
REVIEW_READINESS_START,
REVIEW_READINESS_END,
isWrongAncestry,
authorHasPushPermission,
assessPrDescription,
guiPathsChanged,
Expand Down
Loading
Loading