Re: [PATCH v7] fetch/local: verify checksums for file:// urls

[Paul Barker <paul@pbarker.dev>]

On Thu, 2026-10-01 at 22:53 +0200, Savvas Etairidis wrote:
> [YOCTO #9993]
> 
> A checksum given in a file:// url (e.g. UNINATIVE_CHECKSUM when
> UNINATIVE_URL points at a local file://) was never verified, because
> Local disables the donestamp and so verify_checksum() was never called.
> Override verify_donestamp() in Local to check the file against those
> checksums. A mismatch raises ChecksumError before the rename/mirror
> handling, so the user's file is never renamed.
> 
> Based on a patch by Jhonata Poma-Hansen.
> 
> AI-Generated: Uses Claude Code (Claude Opus 5.5)
> Signed-off-by: Savvas Etairidis <setairidis@gmail.com>
> 
> ---
> changes in v7:
> - We shouldn't assume that anything coming from a file:// URL is
>   "typically small", or that it is on fast local storage (it could be on a
>   HDD array accessed over NFS).
> * Rework the code to avoid re-hashing the file on every fetch, and
>   instead only re-hash it when the checksum is actually expected to be verified.
> - You don't need to mention this if it didn't result in actual change.
> * Wont do in the future.
> - Such as? This is a completely different approach so it's useful to know
>   which bits of feedback were actually relevant and which were made
>   obsolete by the change in approach.
> * Hoped I did better this time.
> - What does "edited with an old mtime" mean?
> * In that test the file attributes are modified to have an old mtime,
>   which is a corner case that was not handled correctly before.
>   This test was removed in v6 because the new approach doesn't have
>   that issue anymore.
> - Why complicate this by using different hash algorithms for the different
>   tests?
>   The test wants to cover the case where we have a valid md5sum but an
>   invalid sha256sum or a invalid md5sum but a valid sha256sum.
> - Added a test to covern new functionality.

This is now inventing a new stamp file mechanism, and the tests are
significantly more complicated than they were in the previous version of
the patch. I'm sorry to say that this is moving further away from a
solution that we can merge, instead of closer to one.

This should look like the other fetchers and use the existing done
stamps mechanism. That may not be able to detect changes in files if the
mtime is moved backwards, but the local filesystem is not a security
boundary so this is probably ok. It's also better to get something in
place and document the limitations in the bugzilla entry rather than
continuing to iterate and add complexity.

There is also no need to test with different hash algorithms here. Two
simple test cases, one for a matching checksum and one for a mismatch,
should be enough, and they should use the same hash algorithm for
consistency. The different hash algorithms should be tested in separate
unit tests that focus on those features. AI tools really get carried
away when adding test cases and some judgement is definitely needed to
keep them under control.

Best regards,

-- 
Paul Barker