Skip to content

enhancement: add additional command hints for PowerShell & CMD#33548

Merged
wxiaoguang merged 6 commits into
go-gitea:mainfrom
jason19970210:main
Feb 10, 2025
Merged

enhancement: add additional command hints for PowerShell & CMD#33548
wxiaoguang merged 6 commits into
go-gitea:mainfrom
jason19970210:main

Conversation

@jason19970210

Copy link
Copy Markdown
Contributor
  • resolving wrong signature calculations for SSH key verification

Fixed #22693

@jason19970210

jason19970210 commented Feb 10, 2025

Copy link
Copy Markdown
Contributor Author

As the command hint by SSH key verification shows echo -n "token" | ... which including trailing new line symbol by PowerShell and makes the wrong signature for the verification process.

That means only Linux and macOS can work properly with the origin command hint.
This PR shows what the command hints that Windows user could be easily calculated the correct signature value as Linux does.

ref: PowerShell/PowerShell#5974 (comment)

And I also make the command hints collapse to minimize the section.

Output

Screenshot 2025-02-11 at 12 54 48 AM Screenshot 2025-02-11 at 12 55 09 AM

remove additional space before '=' for lint checking
@wxiaoguang

Copy link
Copy Markdown
Contributor

I think we could remove the "Linux & macOS" hint because it works for all POSIX standard shells.

And I think we can simplify the template by removing the printf, will make some changes here, just a moment.

@wxiaoguang

Copy link
Copy Markdown
Contributor

And one more question: are these "quotes" necessary?

image

@lunny

lunny commented Feb 10, 2025

Copy link
Copy Markdown
Member

Should we detect user's OS from the web browser and display the right command?

@wxiaoguang

Copy link
Copy Markdown
Contributor

Should we detect user's OS from the web browser and display the right command?

I prefer to keep it simple, do not introduce too much JS code

@jason19970210

Copy link
Copy Markdown
Contributor Author

And one more question: are these "quotes" necessary?

image

As the output I just tried on my PowerShell, the quotes are necessary.

@wxiaoguang

Copy link
Copy Markdown
Contributor

As the output I just tried on my PowerShell, the quotes are necessary.

But the real output is a string which only contains letters&numbers, there is no < or >. Then is the `" still necessary?

@lunny lunny added the type/docs This PR mainly updates/creates documentation label Feb 10, 2025
@lunny lunny added this to the 1.24.0 milestone Feb 10, 2025
@jason19970210

jason19970210 commented Feb 10, 2025

Copy link
Copy Markdown
Contributor Author

As the output I just tried on my PowerShell, the quotes are necessary.

But the real output is a string which only contains letters&numbers, there is no < or >. Then is the `" still necessary?

So here is my output

## Correct
PS > cmd /c "<NUL set /p=`"b6320474e4ee14cbedf20856d5371904f23bc8beea20c5358d1cee7379cf3d66`"| ssh-keygen -Y sign -n gitea -f id_ed25519"
Signing data on standard input
-----BEGIN SSH SIGNATURE-----
U1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAgmjx3mDaLWkLlHptkELlfNhgA8J
FcHFn3m+ODd68ufggAAAAFZ2l0ZWEAAAAAAAAABnNoYTUxMgAAAFMAAAALc3NoLWVkMjU1
MTkAAABA0AR+1fJyD9G+at8fSEdQJsx+f7Od0FmvFcr3P53V9bTDKwIJAMs0SR3Q86CpHT
ELsuRdTlR3S62Bb9J9JDbyAg==
-----END SSH SIGNATURE-----

## Wrong
PS > cmd /c "<NUL set /p=b6320474e4ee14cbedf20856d5371904f23bc8beea20c5358d1cee7379cf3d66| ssh-keygen -Y sign -n gitea -f id_ed25519"
Signing data on standard input
-----BEGIN SSH SIGNATURE-----
U1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAgmjx3mDaLWkLlHptkELlfNhgA8J
FcHFn3m+ODd68ufggAAAAFZ2l0ZWEAAAAAAAAABnNoYTUxMgAAAFMAAAALc3NoLWVkMjU1
MTkAAABAVrx4k+c7wELor/jRErKKABPaOjiTJoCJ0iQgZeyRhxaLRF0LxdwB6EeEe2G2lh
BPBwnr6DiLeqt89Tg7nPgoDg==
-----END SSH SIGNATURE-----

And I also tried with /p=`xxxx`, the output isn't the correct one

@wxiaoguang wxiaoguang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting .... Windows command line has too many quirks ....

Now I think now the generated commands should be the same as the initial ones by your first commit.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Feb 10, 2025
@jason19970210

Copy link
Copy Markdown
Contributor Author

Interesting .... Windows command line has too many quirks ....

Now I think now the generated commands should be the same as the initial ones by your first commit.

kind of magic, both of PowerShell & CMD

@lunny lunny left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trust LGTM

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Feb 10, 2025
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 10, 2025
@wxiaoguang wxiaoguang merged commit e3adb68 into go-gitea:main Feb 10, 2025
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 10, 2025
zjjhot added a commit to zjjhot/gitea that referenced this pull request Feb 11, 2025
* giteaofficial/main:
  Enhance routers for the Actions runner operations (go-gitea#33549)
  [skip ci] Updated translations via Crowdin
  Run yamllint with strict mode, fix issue (go-gitea#33551)
  Enhance routers for the Actions variable operations (go-gitea#33547)
  enhancement: add additional command hints for PowerShell & CMD (go-gitea#33548)
  Feature: Support workflow event dispatch via API (go-gitea#33545)
  Optimize the dashboard (go-gitea#32990)
  Rework suggestion backend (go-gitea#33538)
  Revert "Feature: Support workflow event dispatch via API (go-gitea#32059)" (go-gitea#33541)
project-mirrors-bot-tu Bot pushed a commit to project-mirrors/forgejo-as-gitea-fork that referenced this pull request Feb 27, 2025
…tea#33548)

- resolving wrong signature calculations for SSH key verification

Fixed go-gitea#22693

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: Giteabot <teabot@gitea.io>
(cherry picked from commit e3adb68)
@go-gitea go-gitea locked as resolved and limited conversation to collaborators May 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/docs This PR mainly updates/creates documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

When I add ssh key to an account, I get Can not verify your SSH key: ... asn1: structure error: tags don't match

4 participants