Skip to content

Remember all previously verified configurations  #300

New issue
New issue
Open
Open
Remember all previously verified configurations #300
Labels
enhancement
Milestone
1.0.0

Description

@deivid-rodriguez
deivid-rodriguez
opened on Nov 26, 2015

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!

Activity

  1. trotzig commented on Nov 26, 2015

    @trotzig
    trotzig
    on Nov 26, 2015
    Contributor

    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

  2. deivid-rodriguez commented on Nov 27, 2015

    @deivid-rodriguez
    deivid-rodriguez
    on Nov 27, 2015
    Author

    @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.

  3. sds
    added
    enhancement
    on Dec 1, 2015

  4. sds commented on Dec 1, 2015

    @sds
    sds
    on Dec 1, 2015
    Owner

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

  5. sds
    changed the title [-]Can't rebase due to signature verification[/-] [+]Remember all previously verified configurations [/+] on Dec 1, 2015

  6. gerrywastaken commented on Aug 14, 2016

    @gerrywastaken
    gerrywastaken
    on Aug 14, 2016

    @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.

  7. sds
    added this to the 1.0.0 milestone on Oct 14, 2016

  8. ryanclark2 commented on Jan 26, 2017

    @ryanclark2
    ryanclark2
    on Jan 26, 2017

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

  9. benbernard commented on Aug 7, 2017

    @benbernard
    benbernard
    on Aug 7, 2017

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

  10. sds commented on Aug 7, 2017

    @sds
    sds
    on Aug 7, 2017
    Owner

    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!

  11. benbernard commented on Aug 7, 2017

    @benbernard
    benbernard
    on Aug 7, 2017

    @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!

  12. benbernard commented on Aug 7, 2017

    @benbernard
    benbernard
    on Aug 7, 2017

    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

  13. benbernard commented on Aug 7, 2017

    @benbernard
    benbernard
    on Aug 7, 2017

    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

  14. sds commented on Aug 7, 2017

    @sds
    sds
    on Aug 7, 2017
    Owner

    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.

  15. benbernard commented on Aug 7, 2017

    @benbernard
    benbernard
    on Aug 7, 2017

    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

  16. sds commented on Aug 7, 2017

    @sds
    sds
    on Aug 7, 2017
    Owner

    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.

  17. ncri commented on Mar 29, 2019

    @ncri
    ncri
    on Mar 29, 2019
    Contributor

    Any progress on this?

  18. afn commented on May 27, 2022

    @afn
    afn
    on May 27, 2022

    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?

  19. benbernard commented on May 27, 2022

    @benbernard
    benbernard
    on May 27, 2022 via email
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancement

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions