Lakshmikanth2001 / GitHooks

Git Hooks VS-Code Extension
https://marketplace.visualstudio.com/items?itemName=lakshmikanthayyadevara.githooks
Other
7 stars 2 forks source link

Local paths always stored in the settings.json #13

Closed KrisTC closed 6 months ago

KrisTC commented 1 year ago

I have found this extension quite useful. But I noticed it has now started always updating setting.json for my workspace. And it is using an absolute path. This isn't good for my projects as they they are shared as part of the team. The settings.json are useful for syncing with other team members. But the absolute path for the git hooks isn't good. Ideally, it wouldn't always save the default path. Ideally, it would support the ${workspaceFolder} variable, so at least if it does save. It could then save the path relative to the workspace folder.

Thanks

Lakshmikanth2001 commented 1 year ago

Thank you for reporting the issue , i am unable to reproduce this issue in version v1.3.3

the following is the code which will set your settings.json file

getHooksDir().then((hooksDir) => {

  const currentConfiguration = vscode.workspace
  .getConfiguration('GitHooks', workspaceFolder);

  let currentHooksDir = currentConfiguration?.['hooksDirectory']??"";

  if(!path.isAbsolute(currentHooksDir)){
      currentHooksDir = path.resolve(workspaceFolder?.uri.fsPath??"", currentHooksDir);
  }

  // if hooks directory is not set in configuration
  if(!currentConfiguration?.['hooksDirectory']??false) {
      currentConfiguration?.update('hooksDirectory', hooksDir, vscode.ConfigurationTarget.Workspace);
  }

  if(path.isAbsolute(currentHooksDir) && currentHooksDir !== hooksDir){
      // mismatch in hooks directory configuration

      // update the configuration with git config core.hooksPath
      logger.warn(`GitHooks: hooksDirectory configuration mismatch with git config core.hooksPath`);
      vscode.window.showWarningMessage(`GitHooks: hooksDirectory configuration is not matching with git config core.hooksPath. Updating the configuration with git config core.hooksPath value`);

      currentConfiguration?.update('hooksDirectory', hooksDir, vscode.ConfigurationTarget.Workspace);
  }

  //adding initial context
  vscode.commands.executeCommand('setContext', 'GitHooks.hooksDirectory', hooksDir);
  vscode.commands.executeCommand('setContext', 'GitHooks.hooksDirectoryList', [hooksDir]);

  registerHookTreeDataProvider();
  intialHooksDirectorySet = false;
  }).catch((err) => {
  logger.error(`Unable to read or locate hooksDirectory failed with following error : \n ${err}`);
});

only case i see here is the line

logger.warn(`GitHooks: hooksDirectory configuration mismatch with git config core.hooksPath`);

is getting executed so it is setting your githooks path

please attach your OS details and your extension version so that i can reproduce this issue

Thank you reporting 😀

KrisTC commented 1 year ago

Hi thanks for the quick reply.

I am on windows 10, but the loading codebase in WSL (Remote WSL connection).

I am using 1.3.3 of the extension

Here is the githooks output.

[2023-08-28 13:36:20.276] [INFO]: GitHooks extension is now active!
[2023-08-28 13:36:20.465] [WARN]: GitHooks: hooksDirectory configuration mismatch with git config core.hooksPath
[2023-08-28 13:36:20.467] [WARN]: unable to detect python launguage path
[2023-08-28 13:36:20.467] [WARN]: unable to detect cpp launguage path
[2023-08-28 13:36:21.020] [INFO]: core.hooksPath is set to /root/projects/my-project/.git/hooks

If I try using the ${workspaceFolder} I get the following:

[2023-08-28 11:21:25.040] [INFO]: GitHooks extension is now active!
[2023-08-28 11:21:25.620] [WARN]: GitHooks: hooksDirectory configuration mismatch with git config core.hooksPath
[2023-08-28 11:21:25.624] [ERROR]: Unable to read hooks directory : /root/projects/my-project/${workspaceFolder}
[2023-08-28 11:21:25.626] [WARN]: unable to detect python launguage path
[2023-08-28 11:21:25.627] [WARN]: unable to detect cpp launguage path
[2023-08-28 11:21:26.134] [INFO]: core.hooksPath is set to /root/projects/my-project/.git/hooks

Looking at how it has been appended the ${workspaceFolder} I see that I can use .git/hooks and it behaves better

Lakshmikanth2001 commented 1 year ago

Thank you for your input, even i though of having variables inside settings.json but that will make me prone to shell injection attack , so it will be a security vulnerability so can just give relative path for now, but in future i would try to fix the issue and include variables substitution

It's really nice to know that you liked my extension, please feel free to make any features or enhancements request which you feel are needed to improve this extension

Finally if your issue is resolved please feel free to close the issue

Lakshmikanth2001 commented 1 year ago

Thank you for your feedback, as I haven't received any further updates from you, I think this issue is resolved, in upcoming releases the support for vscode variables will be added which I think will improve functionality

Thank You

piranna commented 8 months ago

I have the same issue, in this case with Ubuntu Linux, and it's a bit annoying. Can you explain about the shell injection issue? I don't think it could happen because we are just prefixing with the project root path... I think paths can be relative in the settings.json file by default from the project root path, and left to use absolute ones to whoever wants them, maybe as an option in the settings to change the default behaviour.

Lakshmikanth2001 commented 8 months ago

ok i will try to resolve this issue, thanks for the comment i am reopening the issue now

piranna commented 8 months ago

You are welcome :-)

Lakshmikanth2001 commented 8 months ago
hooksDir = path.resolve(workspaceFolder.uri.fsPath, hooksDir);

can you be more specific @piranna i see even if you provide path like "GitHooks.hooksDirectory": "./.git/hooks" its not

changing on refresh or reload

piranna commented 8 months ago

I do nothing, the file gets added automatically, so It seems to be related to some kind of auto detection the one filling It with absolute paths instead of relative ones.

Lakshmikanth2001 commented 8 months ago

ok will try to resolve that part

piranna commented 8 months ago

Captura de pantalla -2024-03-10 11-46-15

As can be seen in the capture, the extension have automatically added the new GitHooks.hooksDirectory to the .vscode/settings.json file of a third-party project I'm looking at. That's a bit annoying for two reasons:

  1. the path is an absolute one that's not portable, so commited is useless. It's ok to allow to have absolute paths, but I think should be recommended to use project relative ones by default, both with relative paths or starting with ${workspaceFolder} as shown at https://code.visualstudio.com/docs/editor/variables-reference, or something like this
  2. it's a third-party repo and it's unconditionally setting the default value once I did a change in the other apps/sample/templates/ file. Being the default value, it should not set anything at all and left the .vscode/settings.json file unmodified
Lakshmikanth2001 commented 8 months ago

yes what even you said makes sense, I will try to handle that case and resolve this bug in 1 or 2 weeks, thanks for your effort 😃

piranna commented 8 months ago

Welcome :-)

Lakshmikanth2001 commented 7 months ago

will take a little more time; apologies for the delay

Lakshmikanth2001 commented 7 months ago

Thank for reporting this issue , i have tried to resolve this one in 5e24ce16b85d550be5cb77ebc9225592bd771554 in release v2.1.1 please test it and give your valuable feedback

Lakshmikanth2001 commented 6 months ago

i am closing this issue; please feel free to open if you still have the same issue