Skip to content

Conversation

stefanberger
Copy link
Contributor

Summary

This PR adds support for an option --ignore_unsigned_files to ignore files that are not part of the signature manifest. This allows to for example ignore files that were added after a signature was created. Existing test cases are extended to cover additional symlinks and regular files added after signature created and tests for expected passes and failures depending on whether --ignore_unsigned_files is used.

Checklist
  • All commits are signed-off, using DCO
  • All new code has docstrings and type annotations
  • All new code is covered by tests. Aim for at least 90% coverage. CI is configured to highlight lines not covered by tests.
  • Public facing changes are paired with documentation changes
  • Release note has been added to CHANGELOG.md if needed

@stefanberger stefanberger requested review from a team as code owners July 19, 2025 16:17
@stefanberger stefanberger force-pushed the ignore-addition-files branch 2 times, most recently from 8d3c5e0 to 7577475 Compare July 20, 2025 01:57
Copy link
Collaborator

@mihaimaruseac mihaimaruseac left a comment

Choose a reason for hiding this comment

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

My main worry is that we're pushing the concerns all the way to the serialization, whereas this ignore only belongs in verification. Let's still compute the manifest with all the files that exist on disk and only ignore entries during verification.

@stefanberger
Copy link
Contributor Author

My main worry is that we're pushing the concerns all the way to the serialization, whereas this ignore only belongs in verification. Let's still compute the manifest with all the files that exist on disk and only ignore entries during verification.

I thought that's what I was doing by only passing in the files_to_hash list from the verification side and leaving it None otherwise.

@stefanberger
Copy link
Contributor Author

stefanberger commented Jul 20, 2025

... I mean it's most efficient to only hash those files that are being asked for rather than hashing all of them first and filtering after. Is this what you would want me to do, filter-out unwanted files just before comparing the manifests?

@stefanberger
Copy link
Contributor Author

Let me open a 2nd PR for this. I think disk access is and hashing is too expensive to filter for later...

@mihaimaruseac
Copy link
Collaborator

We should probably benchmark this, based on real scenarios. We already have an ignore_paths in the serialization.

The most reticence comes from having to add a new flag, which we might not need in the future. We'll need to think more here.

@stefanberger stefanberger force-pushed the ignore-addition-files branch from 7577475 to 5258ef0 Compare July 21, 2025 15:40
@stefanberger
Copy link
Contributor Author

We should probably benchmark this, based on real scenarios. We already have an ignore_paths in the serialization.

In some cases it may be desirable to let the signature drive which files are being hashed rather than the filesystem contents. To use ignore_paths, one first needs to find out which files are part of the signature and which files are in the filesystem and then one can determine which files need to be ignored. With the new option you have this automatically.

@stefanberger stefanberger force-pushed the ignore-addition-files branch from 5258ef0 to 31b7caa Compare July 21, 2025 16:47
@stefanberger stefanberger changed the title Ignore unsigned files Ignore unsigned files (only read and hash files contained in signature manifest) Jul 21, 2025
@stefanberger stefanberger force-pushed the ignore-addition-files branch from 31b7caa to a8eaa4f Compare July 30, 2025 13:35
mihaimaruseac
mihaimaruseac previously approved these changes Jul 30, 2025
Copy link
Collaborator

@mihaimaruseac mihaimaruseac left a comment

Choose a reason for hiding this comment

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

Approving instead of #502.

@mihaimaruseac
Copy link
Collaborator

Will need to resolve conflicts, sorry for the delay that caused them

Add support for command line option --ignore-unsigned-files that allows to
ignore files that are not part of the signature (= not listed in the
manifest). This allows to ignore files that for example were added after a
signature was created and those files' presence would make the signature
verification fail. Another use case for this is the presence of multiple
signatures in the same directory where it is necessary to ignore the
files that are not covered by each signature.

Add support for this option for all verification methods. If this
option is set, then create a list of files_to_hash that is derived from
the list of files in the signature manifest.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Add test cases covering the cases of a symlink and an additional file
created after a signature was created. Test expected failures and
passes with and without the new option --ignore_unsigned_files.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
@stefanberger stefanberger force-pushed the ignore-addition-files branch from 65c541a to 40f27d3 Compare July 30, 2025 16:00
@mihaimaruseac mihaimaruseac merged commit 997395a into sigstore:main Jul 30, 2025
51 checks passed
@stefanberger
Copy link
Contributor Author

Thanks for merging.

FYI: vllm-project/vllm#21957

@stefanberger stefanberger deleted the ignore-addition-files branch July 30, 2025 18:01
@mihaimaruseac
Copy link
Collaborator

Oh, this is awesome!!

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