Skip to content

Adds scope to ServerV2 in SSH heartbeats#60524

Merged
eriktate merged 1 commit intomasterfrom
eriktate/scoped-ssh-heartbeat
Nov 10, 2025
Merged

Adds scope to ServerV2 in SSH heartbeats#60524
eriktate merged 1 commit intomasterfrom
eriktate/scoped-ssh-heartbeat

Conversation

@eriktate
Copy link
Copy Markdown
Contributor

@eriktate eriktate commented Oct 23, 2025

This PR adds scope validation to heartbeats and configures SSH servers to populate a scope value using the AgentScope found in its identity. Other server types do not yet populate a scope, so their scope validation during heartbeats will just verify that there is no scope

@eriktate eriktate force-pushed the eriktate/scoped-ssh-heartbeat branch 2 times, most recently from 9a3b158 to 4b91604 Compare October 23, 2025 20:50
@eriktate eriktate marked this pull request as ready for review October 23, 2025 20:50
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Oct 23, 2025

Amplify deployment status

Branch Commit Job ID Status Preview Updated (UTC)
eriktate/scoped-ssh-heartbeat 93e4c7b 14 ✅SUCCEED eriktate-scoped-ssh-heartbeat 2025-11-10 20:21:30

Copy link
Copy Markdown
Contributor

@rosstimothy rosstimothy left a comment

Choose a reason for hiding this comment

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

@fspmarshall @espadolini should we add a scope field to all agent resources before this gets in a release - even if nodes will be the only resources that populate this field for the near future?

@eriktate
Copy link
Copy Markdown
Contributor Author

should we add a scope field to all agent resources before this gets in a release - even if nodes will be the only resources that populate this field for the near future?

If the check on the scope of the agent can be done for all protocols, why not?

@rosstimothy @espadolini I added the scope field to the other server types, would you mind taking a quick peek before I merge? Just want a sanity check since proto changes aren't as easy to fix once they land

Copy link
Copy Markdown
Contributor

@espadolini espadolini left a comment

Choose a reason for hiding this comment

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

Can older auth versions running concurrently with scope-aware ones cause permanent problems, or will they only grant too many permissions while they are running?

types.UpdaterV2Info UpdaterInfo = 8;

// The scope that the instance should heartbeat against.
string scope = 9;
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.

Is the scope something that the agent needs to be aware of and tell the auth about (even if just for confirmation)?

@eriktate eriktate force-pushed the eriktate/scoped-ssh-heartbeat branch from a8130ee to 484df1a Compare October 29, 2025 16:26
@eriktate eriktate force-pushed the eriktate/scoped-ssh-heartbeat branch from bae72e3 to b8f04d3 Compare October 29, 2025 18:55
Copy link
Copy Markdown
Contributor

@espadolini espadolini left a comment

Choose a reason for hiding this comment

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

I think you should also add a check for the scope in services.CompareServers.

@eriktate eriktate force-pushed the eriktate/scoped-ssh-heartbeat branch from b8f04d3 to a5a6fb4 Compare November 4, 2025 21:02
@eriktate eriktate force-pushed the eriktate/scoped-ssh-heartbeat branch from a5a6fb4 to 93e4c7b Compare November 10, 2025 20:15
@eriktate eriktate added the no-changelog Indicates that a PR does not require a changelog entry label Nov 10, 2025
@eriktate eriktate added this pull request to the merge queue Nov 10, 2025
Merged via the queue into master with commit 25a2682 Nov 10, 2025
45 of 46 checks passed
@eriktate eriktate deleted the eriktate/scoped-ssh-heartbeat branch November 10, 2025 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation no-changelog Indicates that a PR does not require a changelog entry size/sm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants