empri-devops / git-privacy Goto Github PK

In most cases, this would only be possible with significant history rewrites, since we only notice rewirtes after the fact. However, we should detail the various scenarios and what's possible.

Would be nice if git-privacy could warn if someone accidentally committed with a different email address than usual.
This happens for instance if one contributes from a different setup than usual with a different global Git configuration.
A detection would prevent unwanted usage of, e.g., private email addresses.
Maybe other hooks than pre-push are better suited.

The detection could alert if

• the address is not part of the history already AND
• it is read from a non-local user.email config.

Current password and salt config values are fairly user-unfriendly. Especially the salt makes multi-device usage cumbersome.

Possible alternatives:

• get rid of the salt
• use random secrets stored in the FS (identities; also has the advantage of an easier handling of past identities)

git-privacy currently doesn't ensure that the encrypted timestamp doesn't end up in the second line of the commit message.
This will lead some tools to assume that it's part of the commit title and will be shown as such.

I think the problem in my case is that JGit doesn't add an empty newline to a commit message that only contains a title line.

Would be great to ensure that the Git-Privacy line always has at least one empty line above?

With git-replace(1) we could register the redated commit object as a replacement for the original commit.
Advantage: The original commit ref (hash) would then "redirect" to the new commit, which is useful in case other tools or the user hold on to the original ref to some reason.

Expected: Should warn only about not yet pushed commits

Currently the post-commit hook fails and leaves an unredacted commit. It would be better to catch a missing config pre-commit to avoid timestamp leaks.

Unclear what triggers the diversion.

A bit of documentation on this would be nice.

Maybe this information could also be added to the help information returned by the CLI.

git-filter-repo is officially recommended by the Git project instead of filter-branch for bulk history rewriting tasks. Its available as a Python module, too. Sounds amazing.

Python 3.9.7, win10

Traceback (most recent call last):
  File "C:\Anaconda3\lib\runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Anaconda3\lib\runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "C:\Anaconda3\Scripts\git-privacy.exe\__main__.py", line 7, in <module>
  File "C:\Anaconda3\lib\site-packages\click\core.py", line 1128, in __call__
    return self.main(*args, **kwargs)
  File "C:\Anaconda3\lib\site-packages\click\core.py", line 1053, in main
    rv = self.invoke(ctx)
  File "C:\Anaconda3\lib\site-packages\click\core.py", line 1659, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "C:\Anaconda3\lib\site-packages\click\core.py", line 1395, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "C:\Anaconda3\lib\site-packages\click\core.py", line 754, in invoke
    return __callback(*args, **kwargs)
  File "C:\Anaconda3\lib\site-packages\click\decorators.py", line 26, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "C:\Anaconda3\lib\site-packages\gitprivacy\gitprivacy.py", line 131, in do_init
    copy_hook(git_dir, "post-commit")
  File "C:\Anaconda3\lib\site-packages\gitprivacy\gitprivacy.py", line 185, in copy_hook
    os.chmod(dst.fileno(), stat.S_IRWXU | stat.S_IRGRP | stat.S_IXGRP |
TypeError: chmod: path should be string, bytes or os.PathLike, not int

Have a look at keyring.

... instead of hiding it if the redacted date and the original dates are identical.
Otherwise this might lead users into thinking the decryption failed or something else is wrong.

... and handle exceptions gracefully.

I.e., anonymize_repo.

The RealDate shown via git-privacy log is one hour into the future.
When I save a commit without git-privacy the commit timestamp is correct, so it shouldn't be a problem of my git setup.
I looked at the raw epoch seconds and they're already wrong, so the error doesn't get introduced during decoding / parsing. My timezone offset is also properly detected and saved.
This is especially curious as the times seem to get loaded from the commit object:

Setup

Ubuntu 18.04
Python 3.6.9
git version 2.17.1
git-privacy 1.4

... checks seems to look for remote rhash but cannot find it locally.

Using git commit --amend on an empty commit fails unless --allow-empty is specified again. This is currently not the case and thus causes the post-commit hook to fail.

... by using a pre-push hook that can abort the push and inform the user if unredacted timestamps exist.

This is happens, e.g., if you partially add and commit changes with an enabled post-commit hook.
Then filter-branch complains with error message

Cannot rewrite branches: You have unstaged changes.

Example:

git-privacy keys -h
Usage: git-privacy [OPTIONS] COMMAND [ARGS]...

Error: not a git repository: '/home/fap'

Description

To run Git Hooks under Windows with Eclipse you have to have installed cygwin and have to have the cygwin \bin directory in your %PATH%.
Running git-privacy in a cygwin powered bash works fine.
But git-privacy fails when executed via Eclipse / EGit.
To me it looks like the working directory is passed as an absolute path (somehow concatenated with the cygwin based path?!) and the code can't handle it.
Looks like an Eclipse bug, but maybe we can also somehow handle it for now?
@cburkert WDYT?

EDIT: This should be an issue with the output of git rev-parse --show-toplevel in the cygwin environment when started through Eclipse, right?

Stacktrace

Traceback (most recent call last):
  File "/usr/local/bin/git-privacy", line 8, in <module>
    sys.exit(cli())
  File "/usr/local/lib/python3.8/site-packages/click/core.py", line 1137, in __call__
    return self.main(*args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/click/core.py", line 1062, in main
    rv = self.invoke(ctx)
  File "/usr/local/lib/python3.8/site-packages/click/core.py", line 1665, in invoke
    super().invoke(ctx)
  File "/usr/local/lib/python3.8/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/local/lib/python3.8/site-packages/click/core.py", line 763, in invoke
    return __callback(*args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/click/decorators.py", line 26, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/gitprivacy/gitprivacy.py", line 110, in cli
    ctx.obj = GitPrivacyConfig(gitdir)
  File "/usr/local/lib/python3.8/site-packages/gitprivacy/gitprivacy.py", line 35, in __init__
    self.repo = git.Repo(gitdir, search_parent_directories=True)
  File "/usr/local/lib/python3.8/site-packages/git/repo/base.py", line 146, in __init__
    raise NoSuchPathError(epath)
git.exc.NoSuchPathError: /cygdrive/c/Users/jnerl/Desktop/git-privacy-demo/C:\Users/jnerl/Desktop/git-privacy-demo

I think we should save the information about the used cypher, so we can switch algorithms more easily later on without breaking compatibility. Then again, since there are encoded commit dates out there without this, we have to support the version without cypher info moving forward anyway?

Newer versions of Git (since when?) now trigger post-commit hooks during a cherry-pick which invokes redate. This causes a different user experience than for older version where no redating took place during cherry-picks.

Eclipse / EGit can't commit any more once the post-commit hook has been added to a repository.

Hook receives multiple input lines with references each but expects only one input line

... instead requiring users to manually add a config snippet to .git/config.

I think using git rev-list is a very elegant solution but I don't really like the user having to understand how rev-list actually works.
The intuitive understanding of <startpoint> actually being the startpoint of the redate process doesn't hold if <starpoint> is not reachable from the current HEAD as the redate process will start after their first shared ancestor.
I suggest at least asking the user if they really know what they are doing if such an input is given.

In egit-privacy I decided to not even offer the option to do a redate starting from such a startpoint as the resulting behavior would be confusing when triggered through the GUI.

@cburkert What do you think?

Committer dates have to be modifiable independently of author dates to handle cases like rebases or cherry-picks for preservation of encrypted real dates (related to #10)

Currently check simply looks for changes in the timezone comparing the authored date of the HEAD and current timezone. This disregards the fact that the HEAD might be from a different committer, that is and always has been in a different timezone.
Instead check should only consider commits from the same user.

In my git config file, I accidentally put quotes around my user.email key. Surprisingly regular Git understands this fine and strips the quotes when committing, but this causes git-privacy tzcheck to skip timezone checks altogether.

When check_timezone_changes() runs user_email = cr.get_value("user", "email", ""), the returned email consists of '"[email protected]"', where user_email[0] == user_email[-1] == ". When calling repo.iter_commits and searching for this email, it finds zero results and reaches if last_commit is None: return False # no previous commits by this user. Consequently, timezone leaks are completely ignored with no warning.

There are multiple strategies that could be used to avoid this issue.

One approach is to strip (double, and single too?) quotes around the email using cr.get_value("user", "email", "").strip('"').

Another approach (making leaks more obvious) is to print a warning message on each return False branch, like "info: no commits found, using timezone {current_tz} and skipping timezone checks." and "info: no commits found by email {user_email}, using timezone {current_tz} and skipping timezone checks". These messages will only pop up on the first commit made by a user under a repo, and I think they're both useful to explain to users how git-privacy operates (it trusts the first timezone by that user, and warns/errors on different timezones), and as debugging output to make it more obvious when git-privacy is failing to protect timezones (which the user is counting on it doing). Unfortunately, it's trickier than I had hoped to to word the messages in a way that's clear to users.