Remember all previously verified configurations

[deivid-rodriguez]

I can't rebase to past revisions with different overcommit configurations. I end up in a detached state. I tried disabling signature verification but it will only work for commits after disabling the check.

This behavior is weird since I've already acknowledged all past revisions of overcommit's config, so I shouldn't need to verify them again. The only solution I've found it to completely uninstall overcommit hooks just for the rebase.

Let me know if you need more information.

Thanks!

[trotzig]

Echoing what @sds informed me about when I ran into the same issue:

Because of the nature of the latest overcommit version, you needed to pull/rebase with OVERCOMMIT_DISABLE=1.

So something like

OVERCOMMIT_DISABLE=1 git pull origin master

[deivid-rodriguez]

@trotzig @sds Thanks to both for that. It's certainly more convenient than the workaround I ended up using:

bundle exec overcommit --uninstall
git rebase ...
bundle exec overcommit --install

I still think this is undesirable and shouldn't be necessary but if the fix is difficult to implement, I find the workaround acceptable.

[sds]

You raise a good point, @deivid-rodriguez. It would be nice if Overcommit remembered all previously-signed configurations.

[gerrywastaken]

@sds also even if it hasn't remembered previous states, it probably shouldn't end up leaving the branch in a detached state with no obvious way to recover or continue the rebase after signing.

[ryanclark2]

Is this still alive? Ran into this today after making changes to overcommit.yml then rebasing a branch onto master.

[benbernard]

would really like this to land. Assume you folks would accept a pull request? Any pointers for where to start with the signatures?

[sds]

Would love a PR addressing this, @benbernard!

You'll need to modify the HookSigner class to support searching through a list of previously-verified signatures.

This may involve creating a different storage format for the configuration value so that a quick lookup can be performed (emphasis on quick—the list of signatures will potentially grow quite large with time, so this needs to be something supporting a constant-time lookup).

Hope that helps!

[benbernard]

@sds are you suggesting better than O(fileSize) like you want to do a search on disk? That seems like overkill. If we are going to read the whole file into memory to load it into a hash (like with a protobuf or a json file) it seems like that will get an O(fileSize) step, and so an addition single O(fileSize) search for an exact match in the set of strings won't actually take a long time.

Seems like signatures will grow at a rate O(number of .overcommit or custom .githooks) changes, that doesn't seem prohibitively large. Something simple like a SHA/signature per line seems like it would be reasonable?

I'm just trying to answer some questions on approach before I dive in :) Thanks for the assist!

[benbernard]

I could also keep the most recent signature on "top" of the file and probably keep O() costs the same if the signature exists and is the most recent signature (the happy case) and only incur O(fileSize) search when there is no immediate match

[benbernard]

Another option would be to limit the # of signatures that we store, like cap it at like 1,000, that would prevent it from growing too large

[sds]

Hey @benbernard, agree it's probably not crucial to address this, but I've seen some pretty weird use cases from users of this tool so I tend to not rule these possibilities out.

In this case, I like the idea of capping the number of signatures and storing the most-recently-created signature at the beginning of the file. Note that storing at the beginning of the file will require we store this outside of the .git/config file (which is what I think you were suggesting, but I just want to be clear). Storing inside .git/config will slow down every git invocation that needs to read this configuration file.

[benbernard]

yeah 100%, don't store in .git/config. Do you think we can store in .git/? like .git/overcommit_signatures? Could drop it in the repo root, but that seems problematic since we would rather avoid people checking the file in... Could also do a ~/.overcommit_signatures or something like that... .git/ seems best to me if we are comfortable with shoving a file in there

[sds]

Storing in .git makes sense to me. Agreed this should not be tracked by Git since that allows attackers to inject signatures in a pull request, bypassing the safety check.

[ncri]

Any progress on this?

[afn]

I'd love to have this as well! If nobody is currently working on it, I might open a PR to address it.

What I'd propose is:

• Add a new git config variable, overcommit.configuration.signaturehistory, which determines how many historical signatures we keep. Maybe default it to 5 or so.
• When a new signature is generated:
  • Let VAR be the name of the config variable it's stored in (e.g. overcommit.configuration.signature or overcommit.HookType.HookName.signature)
  • If a signature already exists, and it's different from the new one:
    • For any signature variables with the same name but a numeric suffix (VAR.1, VAR.2, etc), increment the key, dropping the last one if it exceeds overcommit.configuration.signaturehistory
    • Store the current signature in VAR.1
    • Store the new signature in VAR

Thoughts?

[benbernard]

Sounds like a plan to me, the code also needs some tests, as I didn't have time to write any when I originally developed this

On Fri, May 27, 2022, 9:47 AM Tony Novak @.***> wrote:

I'd love to have this as well! If nobody is currently working on it, I might open a PR to address it.

What I'd propose is:

• Add a new git config variable, overcommit.configuration.signaturehistory, which determines how many historical signatures we keep. Maybe default it to 5 or so.
• When a new signature is generated:
  • Let VAR be the name of the config variable it's stored in (e.g. overcommit.configuration.signature or overcommit.HookType.HookName.signature)
  • If a signature already exists, and it's different from the new one:
    • For any signature variables with the same name but a numeric suffix (VAR.1, VAR.2, etc), increment the key, dropping the last one if it exceeds overcommit.configuration.signaturehistory
    • Store the current signature in VAR.1
    • Store the new signature in VAR

Thoughts?

— Reply to this email directly, view it on GitHub https://github.com/sds/overcommit/issues/300#issuecomment-1139791270, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADFKE3BFT7T5YYG664DSC3VMD4CDANCNFSM4BVITAAA . You are receiving this because you were mentioned.Message ID: @.***>