Improve group SSH and Telnet defaults#2124
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8aa08e2464
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const hostHasManualSshCredentials = !host.identityId && Boolean( | ||
| host.username?.trim() || | ||
| host.password !== undefined || | ||
| host.authMethod !== undefined || |
There was a problem hiding this comment.
When a host has no host-specific credential but still carries the default authMethod field, this marks it as having manual SSH credentials and causes the loop below to skip inheriting groupDefaults.identityId. HostDetailsPanel initializes new hosts with authMethod: "password", so newly-created hosts in a group with a reusable SSH identity won't inherit that identity unless the field is somehow removed, and existing hosts with only a default auth method are affected the same way.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ceb114b840
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| host.username?.trim() || | ||
| host.password !== undefined || | ||
| host.identityFileId || | ||
| host.identityFilePaths?.length, |
There was a problem hiding this comment.
When an existing vault host has only a username (for example the default root) and its group is later assigned a reusable identity, resolveEffectiveTerminalHost reaches this check and treats that username alone as manual SSH credentials. That skips inheriting identityId, so the effective host no longer has access to the group's identity password/key and connecting or copying credentials won't use the new group identity unless every host username is manually cleared. Only actual host auth material (password/key/explicit auth override) should block the inherited identity.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67a68c04df
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ); | ||
|
|
||
| for (const key of INHERITABLE_KEYS) { | ||
| if (key === 'identityId' && hostHasManualSshCredentials) continue; |
There was a problem hiding this comment.
When a host has its own password/key but no identityId, this guard skips only the inherited identityId. INHERITABLE_KEYS applies authMethod/savePassword before this line, so a group default created from a reusable key identity can still stamp authMethod: "key" onto an older/imported manual-password host; downstream resolveHostAuth then sees key auth even though the group identity was intentionally ignored. Skip or clear the full identity credential bundle when hostHasManualSshCredentials is true.
Useful? React with 👍 / 👎.
| if (!identityId) { | ||
| return { | ||
| ...form, | ||
| identityId: inheritedIdentityId ? "" : undefined, |
There was a problem hiding this comment.
This introduces identityId: "" as the marker for clearing an inherited SSH identity, but the panel still initializes sshEnabled via hasSshFields, which checks !!c.identityId. A group whose only SSH setting is this opt-out reopens with the SSH section disabled, and the next save omits identityId, causing hosts in that group to inherit the parent identity again; treat identityId !== undefined as an SSH field like Telnet now does.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8bc11cb8e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const hostHasManualSshCredentials = !host.identityId && Boolean( | ||
| host.password !== undefined || | ||
| host.savePassword === false || | ||
| host.identityFileId || | ||
| host.identityFilePaths?.length, |
There was a problem hiding this comment.
When a host has only a custom username (for example ubuntu) and relies on prompting or agent auth, this predicate stays false because it only treats passwords/save flags/keys as manual credentials. A group identityId is then inherited by the later shouldSkipGroupSshCredentialBundle check, and resolveHostAuth prefers the identity's username/password over host.username, so enabling a group identity can silently change existing hosts that had manual usernames but no saved password/key. Please count an explicit username as a host credential, or otherwise distinguish default usernames before inheriting the identity bundle.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Closes #2114
What changed
Why
Group connection settings received saved identities but did not expose them, and the free-form encoding field did not show supported choices. Nested groups also needed an explicit way to stop inheriting a parent identity.
Checks