From: Simon Marchi <simark@simark.ca>
To: Tom de Vries <tdevries@suse.de>, gdb-patches@sourceware.org
Subject: Re: [PATCH] [pre-commit] Add shellcheck
Date: Fri, 19 Jun 2026 13:56:36 -0400 [thread overview]
Message-ID: <cf536458-f94c-40cb-a226-8afc0778adf3@simark.ca> (raw)
In-Reply-To: <f44ef818-71fb-4931-a383-444dd84a6a54@suse.de>
On 2026-06-19 07:19, Tom de Vries wrote:
> On 6/18/26 7:05 PM, Simon Marchi wrote:
>>
>>
>> On 2026-06-18 11:19, Tom de Vries wrote:
>>> I found a pure python implementation of shellcheck [1].
>>>
>>> Use it to run shellcheck on scripts in the repo.
>>>
>>> Exclude any scripts that are not currently clean.
>>>
>>> Running it seems reasonably fast:
>>> ...
>>> $ pre-commit run shellcheck --all-files -v
>>> shellcheck...............................................................Passed
>>> - hook id: shellcheck
>>> - duration: 0.06s
>>> ...
>>>
>>> For information on other solutions, see this RFC [2].
>>>
>>> [1] https://pypi.org/project/pureshellcheck/0.2.2/
>>> [2] https://sourceware.org/pipermail/gdb-patches/2024-November/213400.html
>>
>> While I'm sympathetic to the use of pre-commit (I added the first
>> hooks), I'm starting to get a bit worried that we kind of blindly pull
>> hooks from random places without paying much attention. This is going
>> to get executed on the machines of many GDB devs, and probably some CI
>> too.
>>
>> I had this thought because this one has the "random project on github
>> vibes" (it appears to be a vibe coded project started a week ago). More
>> established projects (like black) are not immune to being compromised,
>> but they are easier to trust I guess.
>>
>> Assuming you gave a quick look at the code of this project and judge
>> that it's fine,
>
> Yeah, I did, that is, I cloned the github repository, checked it out at main/v0.2.2, and asked claude code to review it on safety aspects.
Cool, thanks.
>> how can we ensure that whatever pre-commit pulls is what
>> you reviewed?
>
> The mechanism to get the tool proposed in this patch is via pip. The pypi index mentions the github repo as source, so I'm relying on that.
Ok I didn't get this at first. You specify the tool (and exact version)
using additional_dependencies, that downloads it in the venv, and then
it runs "pureshellcheck" since it's specified as the entry point.
I previously thought it would get it from the github repo directly, but
no (the github repo URL isn't even mentioned anywhere in the hook).
Getting a specific version from pypi seems safe-ish, in that it's not
possible (from what I've read) to swap a file with another, keeping the
same name. Although I read that it might be possible for someone to
upload a source package at first, and then only later upload a binary
one for the same version (which could differ and contain something
nasty). pip would then prefer the binary one as soon as it's uploaded.
From what I understand, with hooks like flake8, pre-commit clones the
specified ref, then does "pip install" in it to install it in the venv.
So we use the flake8 code as specified by that ref, but its dependencies
are still downloaded from pypi, we can't get around that.
>> We use a git tag, but is that sufficient? Could a
>> (malicious or compromised) project publish a tag, and then replaced that
>> tag with something else later?
>
> I suppose it's possible.
I found out about
$ pre-commit autoupdate --freeze
it transforms:
rev: 26.5.1
into
rev: 4160603246a6b365d4a2af661c6d71b0a0f50478 # frozen: 26.5.1
It won't solve all the integrity concerns, but we might as well use
that, I don't see a downside.
>> An alternative could be to point to a specific commit hash. A more
>> radical alternative would be to vendor (put in our repo) the code of the
>> hooks we use.
>>
>
> Yeah, that is safer. It would mean though using github as the source. But the repo as is doesn't have pre-commit hooks. I worked around that before by using my own github account (see commit 7f6c7a5bb37 ("[pre-commit] Add tclint hook")). I'll submit a v2 shortly that uses this approach.
>
> FWIW, I'm open to any other solutions.
Could you maybe submit a .pre-commit-hooks.yaml to that project? And
then we can point to a specific tag/hash of that repo. If we don't hear
anything from them after a few weeks then we could consider your fork,
but if we can have the hooks upstream from the start it's less
maintenance.
Simon
next prev parent reply other threads:[~2026-06-19 17:57 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-18 15:19 Tom de Vries
2026-06-18 17:05 ` Simon Marchi
2026-06-19 11:19 ` Tom de Vries
2026-06-19 17:56 ` Simon Marchi [this message]
2026-06-22 8:58 ` Tom de Vries
2026-06-22 5:48 ` Thiago Jung Bauermann
2026-06-22 10:02 ` Tom de Vries
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cf536458-f94c-40cb-a226-8afc0778adf3@simark.ca \
--to=simark@simark.ca \
--cc=gdb-patches@sourceware.org \
--cc=tdevries@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox