From 8b0f063588ad22991e3e50fceb7d6c931821a7f0 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Sun, 3 May 2026 21:37:55 +0200 Subject: [PATCH 1/7] Fix docs-build-pr not triggering on elastic/docs PRs The docs-build-pr pipeline has a branch filter in Buildkite that only allows master. All other repos in job 0 use always_trigger_branch:master so they always build against master and pass the filter. The standalone elastic/docs job (job 1) has no always_trigger_branch, so the pr-bot triggers against the PR's feature branch which the pipeline filter silently rejects. Previously builds appeared to work because the doc-preview GitHub Actions workflow posts a comment containing the text "run docs-build", which accidentally matched the old unanchored trigger_comment_regex. Tightening the regex with ^ and $ anchors (#3327) inadvertently removed that accidental trigger. Setting ignore_pipeline_branch_filters: true tells the Buildkite API to bypass the pipeline's branch filter, allowing build_on_commit and comment triggers to work as intended per the buildkite-pr-bot design. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .buildkite/pull-requests.org-wide.json | 1 + 1 file changed, 1 insertion(+) diff --git a/.buildkite/pull-requests.org-wide.json b/.buildkite/pull-requests.org-wide.json index fa7d2ddb737c..9c77344dcc22 100644 --- a/.buildkite/pull-requests.org-wide.json +++ b/.buildkite/pull-requests.org-wide.json @@ -91,6 +91,7 @@ "allowed_list": ["github-actions[bot]", "renovate[bot]", "mergify[bot]"], "build_on_commit": true, "build_on_comment": true, + "ignore_pipeline_branch_filters": true, "trigger_comment_regex": "^run docs-build ?(?rebuild)? ?(?warnlinkcheck)? ?(?skiplinkcheck)?\\s*?$", "always_trigger_comment_regex": "^buildkite test this ?(?rebuild)? ?(?warnlinkcheck)? ?(?skiplinkcheck)?\\s*?$", "skip_ci_labels": [ From dcbe011885d724a31abad6d820eca4d50a4e2ca3 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Mon, 4 May 2026 15:00:33 +0200 Subject: [PATCH 2/7] Fix image path check: anchor to book output dir (outdir) not chapter dir Co-Authored-By: Claude Sonnet 4.6 (1M context) --- lib/ES/Util.pm | 3 ++ .../asciidoctor/lib/copy_images/copier.rb | 31 ++++++++++--------- .../asciidoctor/spec/copy_images_spec.rb | 31 +++++++++++++++++++ 3 files changed, 50 insertions(+), 15 deletions(-) diff --git a/lib/ES/Util.pm b/lib/ES/Util.pm index f1a0a358ad1c..dfc86f405f56 100644 --- a/lib/ES/Util.pm +++ b/lib/ES/Util.pm @@ -228,6 +228,9 @@ sub build_single { $page_header ? ('-a' => "page-header=$page_header") : (), # Turn on asciidoctor's table of contents generation if we want a TOC $toc ? ('-a' => 'toc') : (), + # Asciidoctor doesn't pass the destination directory down to + # the converter so we do so here explicitly + '-a' => 'outdir=' . $raw_dest, '--destination-dir=' . $raw_dest, docinfo($index), $index diff --git a/resources/asciidoctor/lib/copy_images/copier.rb b/resources/asciidoctor/lib/copy_images/copier.rb index 666ac86ffd85..2963401a95fe 100644 --- a/resources/asciidoctor/lib/copy_images/copier.rb +++ b/resources/asciidoctor/lib/copy_images/copier.rb @@ -12,7 +12,7 @@ module CopyImages class Copier include LogUtil - ALLOWED_IMAGE_EXTENSIONS = %w[.png .jpg .jpeg .gif .svg .webp].freeze + ALLOWED_IMAGE_EXTENSIONS = %w[.png .jpg .jpeg .gif .svg .ico .webp].freeze def initialize # TODO: store this set on the document so we don't duplicate copies @@ -38,30 +38,31 @@ def copy_image(block, uri) def perform_copy(block, uri, source) unless ALLOWED_IMAGE_EXTENSIONS.include?(File.extname(uri).downcase) - warn block: block, message: "Refusing to copy non-image file: #{uri}" - return + return warn block: block, + message: "Refusing to copy non-image file: #{uri}" end if File.symlink?(source) - warn block: block, message: "Refusing to copy symlink: #{source}" - return + return warn block: block, + message: "Refusing to copy symlink: #{source}" end - to_dir = File.expand_path(block.document.options[:to_dir]) - destination = File.expand_path(File.join(to_dir, uri)) + doc = block.document + outdir = File.expand_path(doc.attr('outdir') || doc.options[:to_dir]) + dest = File.expand_path(uri, doc.options[:to_dir]) - unless destination.start_with?("#{to_dir}/") - warn block: block, message: "Image path escapes output dir: #{uri}" - return + unless dest.start_with?("#{outdir}/") + return warn block: block, + message: "Image path escapes output dir: #{uri}" end - if File.symlink?(destination) - warn block: block, message: "Destination is a symlink: #{destination}" - return + if File.symlink?(dest) + return warn block: block, + message: "Destination is a symlink: #{dest}" end - FileUtils.mkdir_p(File.dirname(destination)) - FileUtils.cp source, destination + FileUtils.mkdir_p(File.dirname(dest)) + FileUtils.cp source, dest end ## diff --git a/resources/asciidoctor/spec/copy_images_spec.rb b/resources/asciidoctor/spec/copy_images_spec.rb index 9e0f2d38536a..706102faa96e 100644 --- a/resources/asciidoctor/spec/copy_images_spec.rb +++ b/resources/asciidoctor/spec/copy_images_spec.rb @@ -457,4 +457,35 @@ def make_block(to_dir) expect(logs).to eq(expected_logs.strip) end end + + context 'input/output validation' do + let(:tmp) { Dir.mktmpdir } + let(:source_file) do + path = File.join(tmp, 'example.png') + FileUtils.touch(path) + path + end + let(:copier) { CopyImages::Copier.new } + + after(:example) { FileUtils.remove_entry tmp } + + def make_block(to_dir) + doc = double('document') + allow(doc).to receive(:options).and_return(to_dir: to_dir) + allow(doc).to receive(:attr).with('copy_image').and_return(nil) + allow(doc).to receive(:attr).with('outdir').and_return(to_dir) + block = double('block') + allow(block).to receive(:document).and_return(doc) + allow(block).to receive(:source_location).and_return(nil) + block + end + + context 'when the URI uses path traversal with a non-image extension' do + it 'rejects it via the extension check' do + block = make_block(tmp) + copier.perform_copy(block, '../../../evil.yml', source_file) + expect(File.exist?(File.join(tmp, '../../../evil.yml'))).to be false + end + end + end end From d6cac6872e7f534b207cf60c71af6d6c884d7f07 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Mon, 4 May 2026 15:02:07 +0200 Subject: [PATCH 3/7] Collapse return+warn to single lines to stay within MethodLength limit --- resources/asciidoctor/lib/copy_images/copier.rb | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/resources/asciidoctor/lib/copy_images/copier.rb b/resources/asciidoctor/lib/copy_images/copier.rb index 2963401a95fe..993d3ff2128a 100644 --- a/resources/asciidoctor/lib/copy_images/copier.rb +++ b/resources/asciidoctor/lib/copy_images/copier.rb @@ -38,13 +38,11 @@ def copy_image(block, uri) def perform_copy(block, uri, source) unless ALLOWED_IMAGE_EXTENSIONS.include?(File.extname(uri).downcase) - return warn block: block, - message: "Refusing to copy non-image file: #{uri}" + return warn block: block, message: "Non-image extension: #{uri}" end if File.symlink?(source) - return warn block: block, - message: "Refusing to copy symlink: #{source}" + return warn block: block, message: "Refusing to copy symlink: #{source}" end doc = block.document @@ -52,13 +50,11 @@ def perform_copy(block, uri, source) dest = File.expand_path(uri, doc.options[:to_dir]) unless dest.start_with?("#{outdir}/") - return warn block: block, - message: "Image path escapes output dir: #{uri}" + return warn block: block, message: "Image outside output dir: #{uri}" end if File.symlink?(dest) - return warn block: block, - message: "Destination is a symlink: #{dest}" + return warn block: block, message: "Destination is a symlink: #{dest}" end FileUtils.mkdir_p(File.dirname(dest)) From b48b0939de5ff8c58fcd6a68d1c14d069ac34d24 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Mon, 4 May 2026 16:12:32 +0200 Subject: [PATCH 4/7] Fix AbcSize: extract book_outdir helper; simplify make_block stubs --- resources/asciidoctor/lib/copy_images/copier.rb | 7 ++++++- resources/asciidoctor/spec/copy_images_spec.rb | 3 +-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/resources/asciidoctor/lib/copy_images/copier.rb b/resources/asciidoctor/lib/copy_images/copier.rb index 993d3ff2128a..0f1d64c9a936 100644 --- a/resources/asciidoctor/lib/copy_images/copier.rb +++ b/resources/asciidoctor/lib/copy_images/copier.rb @@ -46,7 +46,7 @@ def perform_copy(block, uri, source) end doc = block.document - outdir = File.expand_path(doc.attr('outdir') || doc.options[:to_dir]) + outdir = book_outdir doc dest = File.expand_path(uri, doc.options[:to_dir]) unless dest.start_with?("#{outdir}/") @@ -61,6 +61,11 @@ def perform_copy(block, uri, source) FileUtils.cp source, dest end + def book_outdir(doc) + File.expand_path(doc.attr('outdir') || doc.options[:to_dir]) + end + private :book_outdir + ## # Does a breadth first search starting at the base_dir of the document and # any referenced resources. This isn't super efficient but it is how a2x diff --git a/resources/asciidoctor/spec/copy_images_spec.rb b/resources/asciidoctor/spec/copy_images_spec.rb index 706102faa96e..d59c3eac0baf 100644 --- a/resources/asciidoctor/spec/copy_images_spec.rb +++ b/resources/asciidoctor/spec/copy_images_spec.rb @@ -472,8 +472,7 @@ def make_block(to_dir) def make_block(to_dir) doc = double('document') allow(doc).to receive(:options).and_return(to_dir: to_dir) - allow(doc).to receive(:attr).with('copy_image').and_return(nil) - allow(doc).to receive(:attr).with('outdir').and_return(to_dir) + allow(doc).to receive(:attr).and_return(nil) block = double('block') allow(block).to receive(:document).and_return(doc) allow(block).to receive(:source_location).and_return(nil) From 3bad5d5324b19e91ab87d7a41212c01f9fd93e90 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Mon, 4 May 2026 16:28:48 +0200 Subject: [PATCH 5/7] Remove duplicate input/output validation context from rebase --- .../asciidoctor/spec/copy_images_spec.rb | 66 ------------------- 1 file changed, 66 deletions(-) diff --git a/resources/asciidoctor/spec/copy_images_spec.rb b/resources/asciidoctor/spec/copy_images_spec.rb index d59c3eac0baf..faf8837ce4fa 100644 --- a/resources/asciidoctor/spec/copy_images_spec.rb +++ b/resources/asciidoctor/spec/copy_images_spec.rb @@ -361,72 +361,6 @@ end end - context 'input/output validation' do - let(:tmp) { Dir.mktmpdir } - after(:example) { FileUtils.remove_entry tmp } - - let(:source_file) do - path = File.join(tmp, 'real_image.png') - FileUtils.cp( - File.join(spec_dir, 'resources', 'copy_images', 'example1.png'), - path - ) - path - end - - let(:copier) { CopyImages::Copier.new } - - def make_block(to_dir) - doc = double('document') - allow(doc).to receive(:options).and_return(to_dir: to_dir) - allow(doc).to receive(:attr).with('copy_image').and_return(nil) - block = double('block') - allow(block).to receive(:document).and_return(doc) - allow(block).to receive(:source_location).and_return(nil) - block - end - - context 'when the URI contains path traversal' do - it 'rejects it with a warning and does not write outside to_dir' do - block = make_block(tmp) - outside = File.expand_path(File.join(tmp, '../../../outside.png')) - copier.perform_copy(block, '../../../outside.png', source_file) - expect(File.exist?(outside)).to be false - end - end - - context 'when the URI has a non-image extension' do - it 'rejects it with a warning and does not copy the file' do - block = make_block(tmp) - copier.perform_copy(block, 'evil.yml', source_file) - expect(File.exist?(File.join(tmp, 'evil.yml'))).to be false - end - end - - context 'when the source path is a symlink' do - it 'rejects it with a warning and does not copy' do - symlink_path = File.join(tmp, 'link_image.png') - File.symlink(source_file, symlink_path) - block = make_block(tmp) - copier.perform_copy(block, 'output_image.png', symlink_path) - expect(File.exist?(File.join(tmp, 'output_image.png'))).to be false - end - end - - context 'when the destination is already a symlink' do - it 'rejects it with a warning and does not overwrite the symlink' do - dest_path = File.join(tmp, 'dest_image.png') - # Create a symlink at destination pointing somewhere else - File.symlink('/dev/null', dest_path) - block = make_block(tmp) - copier.perform_copy(block, 'dest_image.png', source_file) - # The symlink should still point to /dev/null, not be replaced - expect(File.symlink?(dest_path)).to be true - expect(File.readlink(dest_path)).to eq('/dev/null') - end - end - end - context 'when the same image is referenced more than once' do let(:input) do <<~ASCIIDOC From 082621be45911a83cf802a4cdefbcb6f1571ed62 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Mon, 4 May 2026 18:40:12 +0200 Subject: [PATCH 6/7] Use private section for book_outdir instead of inline private --- resources/asciidoctor/lib/copy_images/copier.rb | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/resources/asciidoctor/lib/copy_images/copier.rb b/resources/asciidoctor/lib/copy_images/copier.rb index 0f1d64c9a936..267291526807 100644 --- a/resources/asciidoctor/lib/copy_images/copier.rb +++ b/resources/asciidoctor/lib/copy_images/copier.rb @@ -61,11 +61,6 @@ def perform_copy(block, uri, source) FileUtils.cp source, dest end - def book_outdir(doc) - File.expand_path(doc.attr('outdir') || doc.options[:to_dir]) - end - private :book_outdir - ## # Does a breadth first search starting at the base_dir of the document and # any referenced resources. This isn't super efficient but it is how a2x @@ -125,5 +120,11 @@ def log_missing(block, checked, uri) warn block: block, message: "can't read image [#{uri}] at any of #{checked}" end + + private + + def book_outdir(doc) + File.expand_path(doc.attr('outdir') || doc.options[:to_dir]) + end end end From a33a6ec3dc4a5b53f2ab62d00c73ec3d4048b0fa Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Mon, 4 May 2026 18:46:25 +0200 Subject: [PATCH 7/7] Allow images in sibling version dirs by using dirname(outdir) as boundary --- resources/asciidoctor/lib/copy_images/copier.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/resources/asciidoctor/lib/copy_images/copier.rb b/resources/asciidoctor/lib/copy_images/copier.rb index 267291526807..07c77bb46188 100644 --- a/resources/asciidoctor/lib/copy_images/copier.rb +++ b/resources/asciidoctor/lib/copy_images/copier.rb @@ -124,7 +124,8 @@ def log_missing(block, checked, uri) private def book_outdir(doc) - File.expand_path(doc.attr('outdir') || doc.options[:to_dir]) + raw = doc.attr('outdir') || doc.options[:to_dir] + File.expand_path(File.dirname(raw)) end end end