From 18bc97a2d0c8e73b3691db0f919302316f4b71b4 Mon Sep 17 00:00:00 2001 From: Gary <26419401+Gary-H9@users.noreply.github.com> Date: Fri, 8 Dec 2023 11:37:05 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=A4=9D=20=20Sync=20`alphagov/main`=20into?= =?UTF-8?q?=20this=20repository=20(#5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: ChrisBAshton Co-authored-by: Laurence de Bruxelles Co-authored-by: Laurence de Bruxelles Co-authored-by: eddgrant --- .github/workflows/publish.yaml | 15 +- .github/workflows/test.yaml | 7 +- .nvmrc | 2 +- .rubocop.yml | 3 + .ruby-version | 1 - CHANGELOG.md | 20 + Gemfile | 2 +- govuk_tech_docs.gemspec | 22 +- lib/assets/javascripts/_modules/search.js | 4 +- lib/govuk_tech_docs.rb | 8 +- .../api_reference/api_reference_extension.rb | 2 +- .../api_reference/api_reference_renderer.rb | 20 +- lib/govuk_tech_docs/path_helpers.rb | 50 +- lib/govuk_tech_docs/redirects.rb | 4 +- .../table_of_contents/heading.rb | 4 +- .../heading_tree_renderer.rb | 10 +- .../table_of_contents/helpers.rb | 6 +- lib/govuk_tech_docs/version.rb | 2 +- lib/source/layouts/_footer.erb | 2 +- lib/source/layouts/_header.erb | 4 +- package-lock.json | 2710 ++++++++++++++++- spec/api_reference/renderer_spec.rb | 1 - spec/govuk_tech_docs/doubles.rb | 5 + spec/govuk_tech_docs/pages_spec.rb | 10 +- spec/govuk_tech_docs/path_helpers_spec.rb | 104 +- .../tech_docs_html_renderer_spec.rb | 21 +- spec/javascripts/search-spec.js | 6 + spec/spec_helper.rb | 104 +- .../heading_tree_renderer_spec.rb | 12 +- .../headings_builder_spec.rb | 4 +- spec/table_of_contents/helpers_spec.rb | 169 +- 31 files changed, 3089 insertions(+), 245 deletions(-) delete mode 100644 .ruby-version create mode 100644 spec/govuk_tech_docs/doubles.rb diff --git a/.github/workflows/publish.yaml b/.github/workflows/publish.yaml index ec31583a..e9447c30 100644 --- a/.github/workflows/publish.yaml +++ b/.github/workflows/publish.yaml @@ -19,19 +19,21 @@ jobs: - uses: actions/checkout@v3 - uses: ruby/setup-ruby@v1 + with: + ruby-version: '3' - name: Check if new version to release id: gem_version run: | - gem_version=$( ruby -r rubygems -e "puts Gem::Specification::load('govuk_tech_docs.gemspec').version" ) - echo "gem_version=${gem_version}" >> ${GITHUB_OUTPUT} + gem_version=$(ruby -r rubygems -e "puts Gem::Specification::load('govuk_tech_docs.gemspec').version") + echo "gem_version=$gem_version" >> "$GITHUB_OUTPUT" if git fetch origin "refs/tags/v${gem_version}" >/dev/null 2>&1 then - echo "Tag 'v${gem_version}' already exists" - echo "new_version=false" >> ${GITHUB_OUTPUT} + echo "Tag 'v$gem_version' already exists" + echo "new_version=false" >> "$GITHUB_OUTPUT" else - echo "new_version=true" >> ${GITHUB_OUTPUT} + echo "new_version=true" >> "$GITHUB_OUTPUT" fi deploy: @@ -49,11 +51,12 @@ jobs: - uses: actions/setup-node@v3 with: + node-version-file: '.nvmrc' cache: 'npm' - node-version: '14' - uses: ruby/setup-ruby@v1 with: + ruby-version: '3' bundler-cache: true - name: Publish diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 12d102c4..de8e8c8d 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -7,16 +7,21 @@ jobs: name: Test runs-on: ubuntu-latest + strategy: + matrix: + ruby: ['2.7', '3.2'] + steps: - uses: actions/checkout@v3 - uses: actions/setup-node@v3 with: - node-version: '14' + node-version-file: '.nvmrc' cache: 'npm' - uses: ruby/setup-ruby@v1 with: + ruby-version: ${{ matrix.ruby }} bundler-cache: true - name: Run tests diff --git a/.nvmrc b/.nvmrc index 8351c193..3c032078 100644 --- a/.nvmrc +++ b/.nvmrc @@ -1 +1 @@ -14 +18 diff --git a/.rubocop.yml b/.rubocop.yml index 908c247f..f2276a6d 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -2,6 +2,9 @@ inherit_gem: rubocop-govuk: - config/default.yml +AllCops: + TargetRubyVersion: 2.7 + Layout/HeredocIndentation: Enabled: false diff --git a/.ruby-version b/.ruby-version deleted file mode 100644 index 37c2961c..00000000 --- a/.ruby-version +++ /dev/null @@ -1 +0,0 @@ -2.7.2 diff --git a/CHANGELOG.md b/CHANGELOG.md index 68fd0ad2..672c9594 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,12 +2,32 @@ ## Unreleased +## 3.4.0 + +### New features + +- Footer and header links now work with relative links. Thanks to [@eddgrant](https://github.com/eddgrant) for contributing this feature. + + See [pull request #325: Support sites deployed on a path other than "/" when generating header and footer links](https://github.com/alphagov/tech-docs-gem/pull/325) for more details. + +### Fixes + +- You no longer need to downgrade Haml yourself, `bundle install` will now make sure Haml 6 is not installed (see issue [#318: Error: Filters is not a module](https://github.com/alphagov/tech-docs/gem/issues/318)). + ## 3.3.2 +_from ministryofjustice_ Added `lib/source/images/govuk-large.png` from `https://github.com/alphagov/govuk-frontend/blob/main/src/govuk/assets/images/govuk-opengraph-image.png` ## 3.3.1 +_from alphagov_ +This change solves a potential security issue with HTML snippets. Pages indexed in search results have their entire contents indexed, including any HTML code snippets. These HTML snippets would appear in the search results unsanitised, making it possible to render arbitrary HTML or run arbitrary scripts. + +You can see more detail about this issue at [#323: Fix XSS vulnerability on search results page](https://github.com/alphagov/tech-docs-gem/pull/323) + +_from ministryofjustice_ + If your tech-docs live in a directory within a repo (such as `docs`), you can add the following to `config/tech-docs.yml` ```yaml diff --git a/Gemfile b/Gemfile index 46e0e931..e3c78d49 100644 --- a/Gemfile +++ b/Gemfile @@ -1,4 +1,4 @@ -source 'https://rubygems.org' +source "https://rubygems.org" # Specify your gem's dependencies in govuk_tech_docs.gemspec gemspec diff --git a/govuk_tech_docs.gemspec b/govuk_tech_docs.gemspec index a2984102..7b0035a6 100644 --- a/govuk_tech_docs.gemspec +++ b/govuk_tech_docs.gemspec @@ -1,13 +1,14 @@ -# coding: utf-8 -lib = File.expand_path("../lib", __FILE__) +require "English" + +lib = File.expand_path("lib", __dir__) $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) require "govuk_tech_docs/version" `npm install` -abort 'npm install failed' unless $?.success? +abort "npm install failed" unless $CHILD_STATUS.success? -unless File.exist?('node_modules/govuk-frontend/govuk/all.scss') - abort 'govuk-frontend npm package not installed' +unless File.exist?("node_modules/govuk-frontend/govuk/all.scss") + abort "govuk-frontend npm package not installed" end Gem::Specification.new do |spec| @@ -16,8 +17,8 @@ Gem::Specification.new do |spec| spec.authors = ["Government Digital Service"] spec.email = ["govuk-dev@digital.cabinet-office.gov.uk"] - spec.summary = %q{Gem to distribute the GOV.UK Tech Docs Template} - spec.description = %q{Gem to distribute the GOV.UK Tech Docs Template. See https://github.com/alphagov/tech-docs-gem for the project.} + spec.summary = "Gem to distribute the GOV.UK Tech Docs Template" + spec.description = "Gem to distribute the GOV.UK Tech Docs Template. See https://github.com/alphagov/tech-docs-gem for the project." spec.homepage = "https://github.com/alphagov/tech-docs-gem" spec.license = "MIT" @@ -30,10 +31,13 @@ Gem::Specification.new do |spec| spec.bindir = "exe" spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } - spec.require_paths = ["lib"] + spec.require_paths = %w[lib] + + spec.required_ruby_version = ">= 2.7.0" spec.add_dependency "autoprefixer-rails", "~> 10.2" spec.add_dependency "chronic", "~> 0.10.2" + spec.add_dependency "haml", "< 6.0.0" spec.add_dependency "middleman", "~> 4.0" spec.add_dependency "middleman-autoprefixer", "~> 2.10.0" spec.add_dependency "middleman-compass", ">= 4.0.0" @@ -50,5 +54,5 @@ Gem::Specification.new do |spec| spec.add_development_dependency "jasmine", "~> 3.5.0" spec.add_development_dependency "rake", "~> 13.0" spec.add_development_dependency "rspec", "~> 3.9.0" - spec.add_development_dependency "rubocop-govuk", "~> 3.5.0" + spec.add_development_dependency "rubocop-govuk", "~> 4.10.0" end diff --git a/lib/assets/javascripts/_modules/search.js b/lib/assets/javascripts/_modules/search.js index 0fa20ecf..03564058 100644 --- a/lib/assets/javascripts/_modules/search.js +++ b/lib/assets/javascripts/_modules/search.js @@ -169,8 +169,8 @@ this.processContent = function processContent (content, query) { var output - content = '
' + content + '
' - content = $(content).mark(query) + var sanitizedContent = $('
').text(content).html() + content = $('
' + sanitizedContent + '
').mark(query) // Split content by sentence. var sentences = content.html().replace(/(\.+|:|!|\?|\r|\n)("*|'*|\)*|}*|]*)/gm, '|').split('|') diff --git a/lib/govuk_tech_docs.rb b/lib/govuk_tech_docs.rb index 4cacd978..d6491ae2 100644 --- a/lib/govuk_tech_docs.rb +++ b/lib/govuk_tech_docs.rb @@ -86,8 +86,8 @@ def format_date(date) def active_page(page_path) [ page_path == "/" && current_page.path == "index.html", - ("/" + current_page.path) == page_path, - current_page.data.parent != nil && current_page.data.parent.to_s == page_path, + "/#{current_page.path}" == page_path, + !current_page.data.parent.nil? && current_page.data.parent.to_s == page_path, ].any? end end @@ -109,9 +109,9 @@ def active_page(page_path) search.resources = [""] search.fields = { - title: { boost: 100, store: true, required: true }, + title: { boost: 100, store: true, required: true }, content: { boost: 50, store: true }, - url: { index: false, store: true }, + url: { index: false, store: true }, } search.pipeline_remove = %w[stemmer stopWordFilter] diff --git a/lib/govuk_tech_docs/api_reference/api_reference_extension.rb b/lib/govuk_tech_docs/api_reference/api_reference_extension.rb index 5b6ced46..4e27ac16 100644 --- a/lib/govuk_tech_docs/api_reference/api_reference_extension.rb +++ b/lib/govuk_tech_docs/api_reference/api_reference_extension.rb @@ -37,7 +37,7 @@ def initialize(app, options_hash = {}, &block) def uri?(string) uri = URI.parse(string) - %w(http https).include?(uri.scheme) + %w[http https].include?(uri.scheme) rescue URI::BadURIError false rescue URI::InvalidURIError diff --git a/lib/govuk_tech_docs/api_reference/api_reference_renderer.rb b/lib/govuk_tech_docs/api_reference/api_reference_renderer.rb index cfb84e27..8561b2fc 100644 --- a/lib/govuk_tech_docs/api_reference/api_reference_renderer.rb +++ b/lib/govuk_tech_docs/api_reference/api_reference_renderer.rb @@ -124,15 +124,15 @@ def schema_properties(schema_data) properties.merge!(all_of_schema.properties.to_h) end - properties.each_with_object({}) do |(name, schema), memo| - memo[name] = case schema.type - when "object" - schema_properties(schema.items || schema) - when "array" - schema.items ? [schema_properties(schema.items)] : [] - else - schema.example || schema.type - end + properties.transform_values do |schema| + case schema.type + when "object" + schema_properties(schema.items || schema) + when "array" + schema.items ? [schema_properties(schema.items)] : [] + else + schema.example || schema.type + end end end @@ -144,7 +144,7 @@ def build_redcarpet(app) end def get_renderer(file) - template_path = File.join(File.dirname(__FILE__), "templates/" + file) + template_path = File.join(File.dirname(__FILE__), "templates/#{file}") template = File.open(template_path, "r").read ERB.new(template) end diff --git a/lib/govuk_tech_docs/path_helpers.rb b/lib/govuk_tech_docs/path_helpers.rb index 8fdad288..3619f298 100644 --- a/lib/govuk_tech_docs/path_helpers.rb +++ b/lib/govuk_tech_docs/path_helpers.rb @@ -1,18 +1,29 @@ +require "uri" module GovukTechDocs module PathHelpers - def get_path_to_resource(config, resource, current_page) - if config[:relative_links] - resource_path_segments = resource.path.split("/").reject(&:empty?)[0..-2] - resource_file_name = resource.path.split("/")[-1] + # Some useful notes from https://www.rubydoc.info/github/middleman/middleman/Middleman/Sitemap/Resource#url-instance_method : + # 'resource.path' is "The source path of this resource (relative to the source directory, without template extensions)." + # 'resource.destination_path', which is: "The output path in the build directory for this resource." + # 'resource.url' is based on 'resource.destination_path', but is further tweaked to optionally strip the index file and prefixed with any :http_prefix. - path_to_site_root = path_to_site_root config, current_page.path - resource_path = path_to_site_root + resource_path_segments - .push(resource_file_name) - .join("/") + # Calculates the path to the sought resource, taking in to account whether the site has been configured + # to generate relative or absolute links. + # Identifies whether the sought resource is an internal or external target: External targets are returned untouched. Path calculation is performed for internal targets. + # Works for both "Middleman::Sitemap::Resource" resources and plain strings (which may be passed from the site configuration when generating header links). + # + # @param [Object] config + # @param [Object] resource + # @param [Object] current_page + def get_path_to_resource(config, resource, current_page) + if resource.is_a?(Middleman::Sitemap::Resource) + config[:relative_links] ? get_resource_path_relative_to_current_page(config, current_page.path, resource.path) : resource.url + elsif external_url?(resource) + resource + elsif config[:relative_links] + get_resource_path_relative_to_current_page(config, current_page.path, resource) else - resource_path = resource.url + resource end - resource_path end def path_to_site_root(config, page_path) @@ -26,5 +37,24 @@ def path_to_site_root(config, page_path) end path_to_site_root end + + private + + # Calculates the path to the sought resource, relative to the current page. + # @param [Object] config Middleman config. + # @param [String] current_page path of the current page, from the site root. + # @param [String] resource_path_from_site_root path of the sought resource, from the site root. + def get_resource_path_relative_to_current_page(config, current_page, resource_path_from_site_root) + path_segments = resource_path_from_site_root.split("/").reject(&:empty?)[0..-2] + path_file_name = resource_path_from_site_root.split("/")[-1] + + path_to_site_root = path_to_site_root config, current_page + path_to_site_root + path_segments.push(path_file_name).join("/") + end + + def external_url?(url) + uri = URI.parse(url) + uri.scheme || uri.to_s.split("/")[0]&.include?(".") + end end end diff --git a/lib/govuk_tech_docs/redirects.rb b/lib/govuk_tech_docs/redirects.rb index 1c0ae948..d2248e16 100644 --- a/lib/govuk_tech_docs/redirects.rb +++ b/lib/govuk_tech_docs/redirects.rb @@ -1,6 +1,6 @@ module GovukTechDocs class Redirects - LEADING_SLASH = %r[\A\/].freeze + LEADING_SLASH = %r{\A/}.freeze def initialize(context) @context = context @@ -11,7 +11,7 @@ def redirects all_redirects.map do |from, to| # Middleman needs paths without leading slashes - [from.sub(LEADING_SLASH, ""), to: to.sub(LEADING_SLASH, "")] + [from.sub(LEADING_SLASH, ""), { to: to.sub(LEADING_SLASH, "") }] end end diff --git a/lib/govuk_tech_docs/table_of_contents/heading.rb b/lib/govuk_tech_docs/table_of_contents/heading.rb index 120772d8..321afb00 100644 --- a/lib/govuk_tech_docs/table_of_contents/heading.rb +++ b/lib/govuk_tech_docs/table_of_contents/heading.rb @@ -9,14 +9,14 @@ def initialize(element_name:, text:, attributes:, page_url: "") end def size - @element_name.scan(/h(\d)/) && $1 && Integer($1) + @element_name.scan(/h(\d)/) && ::Regexp.last_match(1) && Integer(::Regexp.last_match(1)) end def href if @page_url != "" && size == 1 @page_url else - @page_url + "#" + @attributes["id"] + "#{@page_url}##{@attributes['id']}" end end diff --git a/lib/govuk_tech_docs/table_of_contents/heading_tree_renderer.rb b/lib/govuk_tech_docs/table_of_contents/heading_tree_renderer.rb index 843775a0..25f8cc6b 100644 --- a/lib/govuk_tech_docs/table_of_contents/heading_tree_renderer.rb +++ b/lib/govuk_tech_docs/table_of_contents/heading_tree_renderer.rb @@ -20,23 +20,23 @@ def render_tree(tree, indentation: DEFAULT_INDENTATION, level: nil) output = "" if tree.heading - output += indentation + %{#{tree.heading.title}\n} + output += indentation + %(#{tree.heading.title}\n) end if tree.children.any? && level < @max_level - output += indentation + "
    \n" unless level.zero? + output += "#{indentation}
      \n" unless level.zero? tree.children.each do |child| - output += indentation + INDENTATION_INCREMENT + "
    • \n" + output += "#{indentation}#{INDENTATION_INCREMENT}
    • \n" output += render_tree( child, indentation: indentation + INDENTATION_INCREMENT * 2, level: level + 1, ) - output += indentation + INDENTATION_INCREMENT + "
      • \n" + output += "#{indentation}#{INDENTATION_INCREMENT}\n" end - output += indentation + "
    \n" unless level.zero? + output += "#{indentation}
\n" unless level.zero? end output diff --git a/lib/govuk_tech_docs/table_of_contents/helpers.rb b/lib/govuk_tech_docs/table_of_contents/helpers.rb index b83c7109..a4ec3ba5 100644 --- a/lib/govuk_tech_docs/table_of_contents/helpers.rb +++ b/lib/govuk_tech_docs/table_of_contents/helpers.rb @@ -32,7 +32,7 @@ def list_items_from_headings(html, url: "", max_level: nil) headings = HeadingsBuilder.new(html, url).headings if headings.none? { |heading| heading.size == 1 } - raise "No H1 tag found. You have to at least add one H1 heading to the page: " + url + raise "No H1 tag found. You have to at least add one H1 heading to the page: #{url}" end tree = HeadingTreeBuilder.new(headings).tree @@ -67,12 +67,12 @@ def render_page_tree(resources, current_page, config, current_page_html) if config[:http_prefix].end_with?("/") config[:http_prefix] else - config[:http_prefix] + "/" + "#{config[:http_prefix]}/" end link_value = get_path_to_resource(config, resource, current_page) if resource.children.any? && resource.url != home_url - output += %{
  • #{resource.data.title}\n} + output += %(
  • #{resource.data.title}\n) output += render_page_tree(resource.children, current_page, config, current_page_html) output += "
    • \n" else diff --git a/lib/govuk_tech_docs/version.rb b/lib/govuk_tech_docs/version.rb index d349ea99..f220b124 100644 --- a/lib/govuk_tech_docs/version.rb +++ b/lib/govuk_tech_docs/version.rb @@ -1,3 +1,3 @@ module GovukTechDocs - VERSION = "3.3.2".freeze + VERSION = "3.4.0".freeze end diff --git a/lib/source/layouts/_footer.erb b/lib/source/layouts/_footer.erb index 049eb3f2..1c75bc80 100644 --- a/lib/source/layouts/_footer.erb +++ b/lib/source/layouts/_footer.erb @@ -6,7 +6,7 @@ diff --git a/lib/source/layouts/_header.erb b/lib/source/layouts/_header.erb index 2c442992..0493287f 100644 --- a/lib/source/layouts/_header.erb +++ b/lib/source/layouts/_header.erb @@ -2,7 +2,7 @@