Skip to content

Sort fingerprints when writing .sobelow-skips#185

Open
palm86 wants to merge 1 commit into
nccgroup:masterfrom
palm86:deterministic-skip-output
Open

Sort fingerprints when writing .sobelow-skips#185
palm86 wants to merge 1 commit into
nccgroup:masterfrom
palm86:deterministic-skip-output

Conversation

@palm86
Copy link
Copy Markdown

@palm86 palm86 commented May 15, 2026

Summary

Sobelow.mark_skip_all/1 writes the deduplicated set of finding
fingerprints to .sobelow-skips. The list comes from
Fingerprint.new_skips/0, which returns MapSet.to_list/1 — and
MapSet iteration order is implementation-defined, not stable.

Today the writer joins that list as-is:

fingerprints = Enum.join(fingerprints, "\n")

So running mix sobelow --mark-skip-all twice over the same set of
findings can produce different line orderings in .sobelow-skips,
which is noisy in code review and in git history when the file is
committed.

This change sorts the fingerprints before joining, making the on-disk
output deterministic for a given set of skipped findings:

fingerprints = fingerprints |> Enum.sort() |> Enum.join("\n")

Note on tests

There is no existing coverage for the .sobelow-skips write path. I
considered adding one but the function is defp and exercising it
end-to-end via the Mix task felt disproportionate to the size of this
fix. Happy to add coverage if you'd like to weigh in on the preferred
shape.

`Fingerprint.new_skips/0` returns `MapSet.to_list/1`, whose order is
implementation-defined. The previous write path joined the list as-is,
so re-running `--mark-skip-all` over the same set of findings could
produce different line orderings, churning the file in version
control. Sorting at the write boundary makes the on-disk output
stable.
@palm86 palm86 marked this pull request as ready for review May 15, 2026 06:39
@houllette
Copy link
Copy Markdown
Contributor

Hey @palm86 - thank you so much for the contribution! If you could, would you mind opening this as a PR on the new Sobelow repo? This PR belongs to the unmaintained version and it will no longer be receiving updates.

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.

2 participants