Skip to content

security: hardening batch (pin actions, sanitize PR inputs, verify pushers, escape comments)#159

Open
cotti wants to merge 6 commits into
mainfrom
fix/hardening-batch
Open

security: hardening batch (pin actions, sanitize PR inputs, verify pushers, escape comments)#159
cotti wants to merge 6 commits into
mainfrom
fix/hardening-batch

Conversation

@cotti
Copy link
Copy Markdown
Contributor

@cotti cotti commented May 16, 2026

Summary

This PR ships the bulk of the P0 security remediation work tracked in
elastic/docs-eng-team#532
for the docs-actions repository — six independent fixes that can land together.

Issues addressed

# Issue Fix
1 #498 Pin elastic/vale-rules/{lint,report} to SHA f1d7270d (v1.4.0).
2 #509 Same SHA pin removes the floating-tag risk on the action that holds pull-requests: write.
3 #518 (consumer, part 1/2) docs-deploy.yml and docs-builder/setup/action.yml's edge path now resolve ghcr.io/elastic/docs-builder:edge to a RepoDigest immediately after docker pull and use the digest reference for the rest of the run.
4 #505 changelog/submit/evaluate/scripts/fetch-pr-data.js now sanitizes inline metadata (title, labels) and writes the PR body to ${RUNNER_TEMP}/changelog-pr-body.md (capped at 64 KiB) instead of a step output. Both changelog/submit/evaluate/action.yml and changelog/validate/action.yml pass PR_BODY_FILE to docs-builder. Rejects label names containing ,.
5 #497 changelog/submit/apply/scripts/*.js use new helpers in comment-helper.js: escapeMarkdown (now also escapes <, >, &), wrapCodeFence (auto-sizes backtick fence so attacker-supplied YAML cannot break out of post-comment-only.js's code block), and wrapInlineCode (auto-sizes inline backtick run for paths/labels). Trust-boundary banners at the top of each script document which env vars are attacker-influenced.
6 #511 github/is-elastic-org-member rewritten as a JS composite action that auto-collects every login involved in a workflow_run (PR opener + actor + triggering_actor + head-commit author/committer) and requires every one to be an elastic member. Both docs-deploy.yml and changelog-submit.yml preflight jobs now use it; previous 90-line resolution scripts collapsed to a single step.

Commits

  • 3c55db6 Pin vale lint/report
  • 6379be9 Use a digest reference for the docs-builder image
  • bbcba11 Increase pr fields boundaries (PR body via file)
  • 2a9ebea Tighten trust boundaries (markdown comment escaping)
  • 549cfd7 Check all involved pushers (initial bash impl)
  • 5b0f2a6 Move logic to the appropriate action (JS rewrite + auto-collect)

Test plan

  • Open a same-repo PR with a normal title/body — evaluate and apply succeed.
  • Open a fork PR from an org-member account — preflight passes, changelog generates, comment posted (read-only mode for forks from members).
  • Open a fork PR where the PR opener is an org member but a non-member pushes a follow-up commit — preflight gate now blocks (was previously bypassed). Verify non-members output lists the offending login.
  • PR title containing YAML special chars (-, :, #, embedded newlines) round-trips through the changelog YAML and the PR comment without breaking either.
  • PR body >= 64 KiB is truncated cleanly via the file path; no env-var blowups.

Made with Cursor

@cotti cotti requested a review from a team as a code owner May 16, 2026 23:11
@cotti cotti added the bug Something isn't working label May 16, 2026
@cotti cotti requested a review from technige May 16, 2026 23:11
- name: Run Vale Linter
id: lint
uses: elastic/vale-rules/lint@main
uses: elastic/vale-rules/lint@f1d7270dfe289989a3acbdfcaab8c97a4a32d7d1 # v1.4.0
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@theletterf
Copy link
Copy Markdown
Member

The SHA pins for elastic/vale-rules/lint and elastic/vale-rules/report introduced in this PR are no longer needed.

Since this PR landed, both actions were moved in-house into this repo:

  • .github/workflows/docs-build.yml now uses elastic/docs-actions/vale/lint@v1
  • .github/workflows/docs-deploy.yml now uses elastic/docs-actions/vale/report@v1

The original security concern was a floating @main tag on an external repo (elastic/vale-rules) holding pull-requests: write — a genuine supply-chain risk. That risk is gone: the workflows now reference actions within elastic/docs-actions itself, so there is no external dependency to pin.

Suggest removing the SHA pins from this PR (or following up with a cleanup PR if it has already landed) to reduce maintenance overhead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants