Skip to content

ci: add ability to debug SSH sessions in CI#47819

Merged
jkleinsc merged 1 commit intomainfrom
ssh-debugging
Jul 23, 2025
Merged

ci: add ability to debug SSH sessions in CI#47819
jkleinsc merged 1 commit intomainfrom
ssh-debugging

Conversation

@codebytere
Copy link
Member

@codebytere codebytere commented Jul 18, 2025

Description of Change

Enable SSH access to macOS actions runners for debugging. I've set it up to work similarly to CircleCI's old "rerun with ssh" - to enable it in actions, choose "rerun in debug mode" in the actions UI.

Successful Connection

Screenshot 2025-07-18 at 9 57 53 AM

Checklist

Release Notes

Notes: none

@codebytere codebytere requested a review from a team as a code owner July 18, 2025 07:59
@codebytere codebytere added semver/none target/36-x-y PR should also be added to the "36-x-y" branch. target/37-x-y PR should also be added to the "37-x-y" branch. target/38-x-y PR should also be added to the "38-x-y" branch. labels Jul 18, 2025
@codebytere codebytere force-pushed the ssh-debugging branch 3 times, most recently from cb36e1e to 9bb90bd Compare July 21, 2025 15:33
@jkleinsc jkleinsc merged commit 670da27 into main Jul 23, 2025
57 checks passed
@jkleinsc jkleinsc deleted the ssh-debugging branch July 23, 2025 14:57
@release-clerk
Copy link

release-clerk bot commented Jul 23, 2025

No Release Notes

@trop
Copy link
Contributor

trop bot commented Jul 23, 2025

I have automatically backported this PR to "38-x-y", please check out #47874

@trop trop bot removed the target/38-x-y PR should also be added to the "38-x-y" branch. label Jul 23, 2025
@trop
Copy link
Contributor

trop bot commented Jul 23, 2025

I have automatically backported this PR to "37-x-y", please check out #47875

@trop
Copy link
Contributor

trop bot commented Jul 23, 2025

I have automatically backported this PR to "36-x-y", please check out #47876

@trop trop bot added in-flight/37-x-y in-flight/36-x-y and removed target/37-x-y PR should also be added to the "37-x-y" branch. target/36-x-y PR should also be added to the "36-x-y" branch. labels Jul 23, 2025
CHROMIUM_GIT_COOKIE_WINDOWS_STRING: ${{ secrets.CHROMIUM_GIT_COOKIE_WINDOWS_STRING }}
ELECTRON_OUT_DIR: Default
ELECTRON_RBE_JWT: ${{ secrets.ELECTRON_RBE_JWT }}
ACTIONS_STEP_DEBUG: ${{ secrets.ACTIONS_STEP_DEBUG }}
Copy link
Member

Choose a reason for hiding this comment

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

This is going to enable debugging for all pipelines, instead of just one. I think this secret should be a branch name or something so that the check is current_branch == secrets.debug_branch_name or something.

Comment on lines +3 to +17
get_authorized_keys() {
if [ -z "$AUTHORIZED_USERS" ] || ! echo "$AUTHORIZED_USERS" | grep -q "\b$GITHUB_ACTOR\b"; then
return 1
fi

api_response=$(curl -s "https://api.github.com/users/$GITHUB_ACTOR/keys")

if echo "$api_response" | jq -e 'type == "object" and has("message")' >/dev/null; then
error_msg=$(echo "$api_response" | jq -r '.message')
echo "Error: $error_msg"
return 1
else
echo "$api_response" | jq -r '.[].key'
fi
}
Copy link
Member

Choose a reason for hiding this comment

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

I think the secret should actually contain the SSH keys. And those should be fetched from the infra repo.

We can in terraform use a github_secret resource and folks should have to hardcode the SSH key they want to use to connect there. That avoids the API call here and avoids any risk around GITHUB_ACTOR somehow being an injection vector.

fi

if [ "$TUNNEL" != "true" ]; then
echo "SSH tunneling is disabled. Set enable-tunnel: true to enable remote access."
Copy link
Member

Choose a reason for hiding this comment

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

It's just called tunnel now.

echo ' '
echo '📋 Copy and run this command to connect:'
echo ' '
if [ -n "$TUNNEL_HOSTNAME" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

TUNNEL_HOSTNAME is always set no?


echo 'Starting Cloudflare tunnel...'

./cloudflared tunnel --no-autoupdate run --token "$CLOUDFLARE_TUNNEL_TOKEN" 2>&1 | tee cloudflared.log | sed -u 's/^/cloudflared: /' &
Copy link
Member

Choose a reason for hiding this comment

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

Separate to this, maybe we shouldn't use authorized_keys at all and instead should rely on cloudflare access zero trust rules for protecting our ssh access. We can configure this hostname as an SSH target in cloudflare zero trust and then assign IDP roles (wg-infra) as having access to that hostname.

@trop trop bot removed the in-flight/38-x-y label Jul 30, 2025
@trop trop bot added merged/38-x-y PR was merged to the "38-x-y" branch. merged/37-x-y PR was merged to the "37-x-y" branch. and removed in-flight/37-x-y labels Jul 30, 2025
@trop trop bot added merged/36-x-y PR was merged to the "36-x-y" branch. and removed in-flight/36-x-y labels Aug 10, 2025
kigh-ota pushed a commit to kigh-ota/electron that referenced this pull request Sep 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged/36-x-y PR was merged to the "36-x-y" branch. merged/37-x-y PR was merged to the "37-x-y" branch. merged/38-x-y PR was merged to the "38-x-y" branch. semver/none

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants