Skip to content

fix: enforce minLength on update_release body to block placeholder submissions #39713

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.

Sign up for GitHub

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
pelikhan merged 1 commit into from
Jun 17, 2026
+763 −243
Merged

fix: enforce minLength on update_release body to block placeholder submissions #39713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
35 changes: 35 additions & 0 deletions actions/setup/js/safe_output_type_validator.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,14 @@ const SAMPLE_VALIDATION_CONFIG = {
},
},
},
update_release: {
defaultMax: 1,
fields: {
tag: { type: "string", sanitize: true, maxLength: 256 },
operation: { required: true, type: "string", enum: ["replace", "append", "prepend"] },
body: { required: true, type: "string", sanitize: true, maxLength: 65000, minLength: 20 },
},
},
};

const ISSUE_CLOSING_KEYWORDS = ["fix", "fixes", "fixed", "close", "closes", "closed", "resolve", "resolves", "resolved"];
Expand Down Expand Up @@ -861,6 +869,33 @@ describe("safe_output_type_validator", () => {
expect(result.isValid).toBe(false);
expect(result.error).toContain("too short");
});

it("should reject update_release body shorter than minLength (e.g. 'test')", async () => {

@github-actions github-actions Bot Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] All three new tests use operation: "prepend". Since minLength enforces a floor regardless of operation, adding one test with operation: "replace" (the most common case) would confirm the constraint is operation-agnostic and guard against any future per-operation branching in the validator.

💡 Suggested addition 
it("should reject update_release body shorter than minLength for replace operation", async () => {
  const { validateItem } = await import("./safe_output_type_validator.cjs");
  const result = validateItem(
    { type: "update_release", tag: "v1.0.0", operation: "replace", body: "test" },
    "update_release", 1
  );
  expect(result.isValid).toBe(false);
  expect(result.error).toContain("too short");
});

const { validateItem } = await import("./safe_output_type_validator.cjs");

const result = validateItem({ type: "update_release", tag: "v1.0.0", operation: "prepend", body: "test" }, "update_release", 1);

expect(result.isValid).toBe(false);
expect(result.error).toContain("too short");
expect(result.error).toContain("20");
});

it("should accept update_release body that meets minLength", async () => {
const { validateItem } = await import("./safe_output_type_validator.cjs");

const result = validateItem({ type: "update_release", tag: "v1.0.0", operation: "prepend", body: "Patch release with bug fixes and improvements." }, "update_release", 1);

expect(result.isValid).toBe(true);
});

it("should reject update_release body that is only whitespace below minLength", async () => {

@github-actions github-actions Bot Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] Misleading test name: the body value " test " is whitespace-padded text, not purely whitespace. An actual pure-whitespace string (e.g., " ") is a distinct edge case that is currently untested.

💡 Suggested additions

Rename the existing test to reflect what it actually covers:

it("should reject update_release body whose trimmed length is below minLength (whitespace-padded)", async () => {

And add a dedicated purely-whitespace test:

it("should reject update_release body that is purely whitespace", async () => {
  const { validateItem } = await import("./safe_output_type_validator.cjs");
  const result = validateItem(
    { type: "update_release", tag: "v1.0.0", operation: "replace", body: "                    " },
    "update_release", 1
  );
  expect(result.isValid).toBe(false);
  expect(result.error).toContain("too short");
});

This closes the gap between what the test name promises and what it actually verifies.

const { validateItem } = await import("./safe_output_type_validator.cjs");

const result = validateItem({ type: "update_release", tag: "v1.0.0", operation: "prepend", body: " test " }, "update_release", 1);

@github-actions github-actions Bot Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whitespace test uses wrong input — doesn't exercise the trim path at all.

The test name promises coverage of "only whitespace below minLength", but " test " (10 chars raw) fails because 10 < 20 — the trim call is irrelevant here. The test is a near-duplicate of the first test and doesn't add useful coverage.

💡 What to test instead

The dangerous case is content whose raw length meets the minLength threshold but whose trimmed length falls below it — e.g. 20+ spaces. The existing create_issue suite covers this correctly:

// create_issue's whitespace test — 25 spaces, raw length ≥ 20, trimmed → "" → rejected
validateItem({ ..., body: "                         " }, ...)

The update_release version should mirror that:

it("should reject update_release body that is only whitespace even when raw length meets minLength", async () => {
  const { validateItem } = await import("./safe_output_type_validator.cjs");
  // 20 spaces: passes JSON Schema minLength:20, but trim() → "" → runtime should reject it
  const result = validateItem(
    { type: "update_release", tag: "v1.0.0", operation: "prepend", body: "                    " },
    "update_release", 1
  );
  expect(result.isValid).toBe(false);
  expect(result.error).toContain("too short");
});

Without this, removing the .trim() call on line 401 of the validator would silently pass all existing update_release tests while breaking the intended protection.


Comment on lines +891 to +895
expect(result.isValid).toBe(false);
expect(result.error).toContain("too short");
});
});

describe("array validation", () => {
Expand Down
Loading
Loading