Skip to content

Fix GitHub advisory YAML indentation#1092

Merged
simi merged 1 commit into
rubysec:masterfrom
StantonMatt:fix-github-advisory-yaml-indent
Jun 2, 2026
Merged

Fix GitHub advisory YAML indentation#1092
simi merged 1 commit into
rubysec:masterfrom
StantonMatt:fix-github-advisory-yaml-indent

Conversation

@StantonMatt
Copy link
Copy Markdown
Contributor

@StantonMatt StantonMatt commented Jun 2, 2026

Fixes #1091.

This updates the GitHub advisory sync output so generated sequence values under advisory keys are indented under their parent key, including:

  • patched_versions
  • related.url

The formatter validates that reindented YAML parses back to the same data before using it, so nested raw GitHub payloads and multiline scalar text do not get silently rewritten if a shape is not safe to reformat.

Verification:

GEM_HOME=.codex-tmp/ruby-advisory-db/gems-ruby26 \
GEM_PATH=.codex-tmp/ruby-advisory-db/gems-ruby26 \
ruby -rlogger -Ilib -Ispec .codex-tmp/ruby-advisory-db/gems-ruby26/bin/rspec spec/github_advisory_sync_spec.rb

ruby -c lib/github_advisory_sync.rb
ruby -c spec/github_advisory_sync_spec.rb
git diff --check

Signed-off-by: Matthew Stanton <stantonmatthewj@gmail.com>
@simi
Copy link
Copy Markdown
Contributor

simi commented Jun 2, 2026

Is it needed to reinvent the yaml lint/format? Can't we use some existing tooling to make it happen?

@StantonMatt
Copy link
Copy Markdown
Contributor Author

Good question. I did check the existing path before going this route: the repo currently has yamllint, but that only validates the advisory files; it does not rewrite the generated YAML. The Ruby stdlib/Psych emitter is also the source of the shape here. On the repo Ruby path, YAML.dump(data, indentation: 2) still emits sequences like:

related:
  url:
  - https://example.test/a
patched_versions:
- ">= 3.0.1"

So this PR is not trying to replace a formatter that is already wired in; it is a narrow post-process around Psych output, with a round-trip guard before writing it.

That said, I agree that a real formatter would be preferable if the project is comfortable adding one. I can look for a small existing tool that fits the repo and rework this, or keep this limited to generated GitHub advisory output if avoiding another dependency is the priority.

@jasnow
Copy link
Copy Markdown
Member

jasnow commented Jun 2, 2026

@StantonMatt - Thanks for your contribution.I will defer to the other @rubysec/maintainers to comment on @simi's comment and merge it if it is ready.

Saw this a couple days ago: https://rubygems.org/gems/rapidyaml

@StantonMatt
Copy link
Copy Markdown
Contributor Author

Thanks, I checked rapidyaml. It can emit the block-style sequence indentation this PR is after, so it is a real option.

The tradeoff is dependency fit: current rapidyaml declares Ruby >= 3.1.0 and looks like a native dependency. That is fine for this workflow’s Ruby 4.0 job, but it is a wider dependency change than the narrow Psych-output post-process here. Since this generator already uses Psych and this patch round-trips the YAML before writing it, I would lean toward keeping this PR narrow unless maintainers prefer adding a formatter dependency.

I can rework it in that direction if that is the preference.

@jasnow
Copy link
Copy Markdown
Member

jasnow commented Jun 2, 2026

I can rework it in that direction if that is the preference.

I would wait for others' feedback but do any further work in a separate PR.

@simi
Copy link
Copy Markdown
Contributor

simi commented Jun 2, 2026

@StantonMatt - Thanks for your contribution.I will defer to the other @rubysec/maintainers to comment on @simi's comment and merge it if it is ready.

Saw this a couple days ago: https://rubygems.org/gems/rapidyaml

If you're happy with this for now @jasnow, let's merge it to keep it consistent. We can check around YAML formatter/linter later.

@jasnow
Copy link
Copy Markdown
Member

jasnow commented Jun 2, 2026

If you're happy with this for now @jasnow, let's merge it to keep it consistent.
We can check around YAML formatter/linter later.

Agree

@simi simi merged commit d0d3035 into rubysec:master Jun 2, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix lib/github_advisory_sync.rb script to indent patched_versions: and related:/url: 2 spaces to the right

3 participants