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

--dry-run shouldn't test push permissions #2232

Open
kristof-mattei opened this issue Nov 5, 2021 · 2 comments · May be fixed by #2426
Open

--dry-run shouldn't test push permissions #2232

kristof-mattei opened this issue Nov 5, 2021 · 2 comments · May be fixed by #2426

Comments

@kristof-mattei
Copy link

New feature motivation

Or at least we should be able to disable the validation of push permissions.
When trying to validate a release in the context of a CI we do this with the least amount of permissions possible.

Running semantic-release --dry-run stops simply because it cannot push to the underlying repo.

Requiring these kind of permissions in the current security landscape where we run with the least permissions possible.

New feature description

Separate the validation of credentials and the dry-running to allow for a reduced set of permissions passed in.

New feature implementation

Separating the 2 allows the user to pass in both flags if they wish to retain current behavior.

OR we could introduce a new switch that disables push validation as part of the dry-run.

If the team feels that this will be helpful, I'm happy to take a stab at it.

@thislooksfun
Copy link
Contributor

thislooksfun commented Feb 2, 2022

I for one would absolutely love to see this implemented. I have semantic-release configured as a GitHub Action, but I often want to do a --dry-run before pushing to make sure the changelog is what I expect it to be. The problem is that in order to do so right now I have to manually specify the repository URL (because the repo url resolution prefers the package.json url, which is set to git+https: to allow anonymous read access, but my local machine is only set up for ssh auth) and comment out the ENOGHTOKEN check, just to get it to do a dry run.

I personally would expect --dry-run to disable these checks, but I do see the value in having them if you are trying to get a full pipeline set up, so I would also be ok with adding a new flags. Perhaps add a flag --no-git-verify which skips these checks, but only works if --dry-run is also set?

As far as I can tell it would only need to disable

semantic-release/index.js

Lines 83 to 101 in 2c30e26

try {
try {
await verifyAuth(options.repositoryUrl, context.branch.name, {cwd, env});
} catch (error) {
if (!(await isBranchUpToDate(options.repositoryUrl, context.branch.name, {cwd, env}))) {
logger.log(
`The local branch ${context.branch.name} is behind the remote one, therefore a new version won't be published.`
);
return false;
}
throw error;
}
} catch (error) {
logger.error(`The command "${error.command}" failed with the error message ${error.stderr}.`);
throw getError('EGITNOPERMISSION', context);
}
logger.success(`Allowed to push to the Git repository`);

and

https://github.com/semantic-release/github/blob/f51c88afa2736ca054ee7990030a7653db0bee22/lib/verify.js#L96-L98

I am also happy to take a crack at implementing this myself. In fact I likely will shortly if someone else doesn't beat me to the punch.

@wlfbck
Copy link

wlfbck commented Mar 3, 2022

Wouldn't it be better if it simply checks if push is possible, but continues anyway if it fails? I just started with semantic-release and from the documentation on --dry-run i actually expected it to behave like that. It also just makes sense.

thislooksfun added a commit to thislooksfun/semantic-release that referenced this issue Apr 29, 2022
This new option will bypass checking remote access. It only works in dry
run mode. This addresses semantic-release#2232.
@thislooksfun thislooksfun linked a pull request Apr 30, 2022 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants