Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unintentional reading/writing of repo configuration (approach 2) #4078

Merged
merged 8 commits into from Jan 24, 2020

Commits on Jan 22, 2020

  1. TST: Demo some flavor of dataladgh-4071

    But this is not yet the big fish, can't seem to find it...
    mih committed Jan 22, 2020
    Copy the full SHA
    19e7a83 View commit details
    Browse the repository at this point in the history
  2. BF: Make git-config dump parser aware of CWD

    This fixes some aspect of dataladgh-4071.
    mih committed Jan 22, 2020
    Copy the full SHA
    58f13ad View commit details
    Browse the repository at this point in the history

Commits on Jan 23, 2020

  1. TST: Adjust test to show config leak

    mih committed Jan 23, 2020
    Copy the full SHA
    1895b05 View commit details
    Browse the repository at this point in the history

Commits on Jan 24, 2020

  1. ENH: ConfigManager(source='dataset-local')

    Be able to ignore global and system configuration.
    
    Modified-by: Kyle Meyer <kyle@kyleam.com>
      Absorb the "dataset is None and 'dataset-local'" guard from
      dataladgh-4072's 7fe9f9a (BF: Prevent accidential read from AND write to
      undesired datasets, 2020-01-23).
    mih authored and kyleam committed Jan 24, 2020
    Copy the full SHA
    6d9c2ae View commit details
    Browse the repository at this point in the history
  2. TST: Verify ConfigManager doesn't read from a random bystander dataset

    with `dataset=None`.
    mih authored and kyleam committed Jan 24, 2020
    Copy the full SHA
    c905abd View commit details
    Browse the repository at this point in the history
  3. BF: config: Prevent accidental read/write to PWD's repository

    When ConfigManager() is given a dataset, the 'git config --list' is
    executed from its top-level directory.  Otherwise, execution is from
    the current working directory.  Problematically, if the PWD belongs to
    a repository, its configuration is considered for both reading and
    writing.  In practice this doesn't happen much in the code base.
    There are two main paths to trigger it:
    
      1) datalad.cfg, which isn't supposed to be attached to any dataset,
         will contain the configuration for the PWD's
    
      2) instantiating Dataset() with a path that does not yet exist while
         the PWD belongs to another repo.  In this case, Dataset.config
         calls ConfigManager() with dataset=None.  But once the dataset is
         created, the configuration comes from the correct repository
    
    The unmerged 7fe9f9a (BF: Prevent accidential read from AND write to
    undesired datasets, dataladgh-4072) fixes the above issues by passing either
    --system or --global to 'git config' when a dataset isn't specified or
    its path doesn't exist.  --system versus --global is chosen based on
    whether ~/.gitconfig exists.  This approach introduces other problems.
    Currently the configuration from both --system and --global sources is
    considered, but that change would limit it to just one of those
    sources.  Also, the approach assumes that the user's "global"
    configuration is in ~/.gitconfig, but it could also be in
    $XDG_CONFIG_HOME/git/config.
    
    As an alternative fix that avoids passing --system or --global
    explicitly, let's prevent 'git config' from reading or writing
    repository-based configuration values whenever a dataset is not given.
    Do this by setting $GIT_DIR to an empty string.
    
    One argument against this is that, as far as I can see, the empty
    string behavior isn't explicitly documented.  However, based on trying
    with Git v2.9.0 (2016) [*] and the latest Git (v2.25.0), it appears
    this behavior is stable.  And if the behavior does change, the tests
    we have in place should detect it.
    
    Fixes datalad#4071.
    Supersedes and closes datalad#4072.
    
    [*] This was the earliest Git I had luck compiling locally.
    kyleam committed Jan 24, 2020
    Copy the full SHA
    1d081af View commit details
    Browse the repository at this point in the history
  4. BF: Test relied on faulty ConfigManager behavior

    which read from PWD no matter what. Use proper patch, instead of
    a hack.
    mih committed Jan 24, 2020
    Copy the full SHA
    c3ed4e5 View commit details
    Browse the repository at this point in the history
  5. TST: Disable broken test to unblock config fix

    It looks as if @yarikoptic can bring it back with his 2FA setup.
    mih committed Jan 24, 2020
    Copy the full SHA
    cad795b View commit details
    Browse the repository at this point in the history