Skip to content

Improve PowerShell EncodedCommand usage#325

Merged
kke merged 1 commit into
mainfrom
powershell-command-logging
May 4, 2026
Merged

Improve PowerShell EncodedCommand usage#325
kke merged 1 commit into
mainfrom
powershell-command-logging

Conversation

@kke
Copy link
Copy Markdown
Contributor

@kke kke commented Apr 30, 2026

The powershell.Cmd now auto-detects if the script contains \n, \r, or " and uses -EncodedCommand (safe for cmd.exe), otherwise it uses -Command "..." so the script is visible in logs. Simple one-liners like $env:COMPUTERNAME, [DateTimeOffset]::UtcNow.ToUnixTimeSeconds(), registry reads, etc. are now human-readable.

Part of the rig v2 last-mile effort, breaking API is not a problem as rig v2 has not been released.

@kke kke added the enhancement New feature or request label Apr 30, 2026
@kke kke requested a review from Copilot April 30, 2026 12:22
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the PowerShell command builder to prefer human-readable -Command "..." for simple one-liners while falling back to -EncodedCommand for scripts that are more likely to break when routed through cmd.exe, improving log readability as part of the rig v2 effort.

Changes:

  • Update powershell.Cmd to choose between -Command and -EncodedCommand based on script contents, and document the behavior.
  • Add unit tests covering selection logic and log readability expectations.
  • Update a Windows FS test comment to reflect the new matching strategy (prefix-based, not always encoded).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
powershell/powershell.go Implements conditional -Command vs -EncodedCommand selection and updates function docs.
powershell/powershell_test.go Adds tests validating when Cmd uses -Command vs -E and that simple scripts remain visible.
remotefs/hostinfo_test.go Updates comment explaining PowerShell command matching assumptions in tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread powershell/powershell.go Outdated
Comment thread powershell/powershell.go Outdated
@kke kke force-pushed the powershell-command-logging branch from 61ebb42 to 66d9515 Compare May 4, 2026 08:34
@kke kke requested a review from Copilot May 4, 2026 08:34
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread remotefs/hostinfo_test.go
Comment thread powershell/powershell_test.go
@kke kke force-pushed the powershell-command-logging branch from 66d9515 to 4c9df42 Compare May 4, 2026 08:41
@kke kke requested a review from Copilot May 4, 2026 08:41
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread powershell/powershell.go Outdated
Comment thread powershell/powershell.go
Comment thread powershell/powershell_test.go Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread powershell/powershell.go
Comment thread powershell/powershell.go
@kke kke force-pushed the powershell-command-logging branch from 622c280 to c1d610e Compare May 4, 2026 08:59
@kke kke requested a review from Copilot May 4, 2026 08:59
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

cmd/executor.go:205

  • decodeUTF16LE / decodeEncoded behavior changed (now UTF-16LE decoding for PowerShell -EncodedCommand), but there’s no unit test coverage for this log-decoding path in cmd. Adding a small internal test (package cmd) that feeds a known powershell.exe ... -E <payload> string and asserts the decoded output would help prevent regressions (and would catch index/precedence issues here).
func decodeUTF16LE(encoded string) (string, error) {
	raw, err := base64.StdEncoding.DecodeString(encoded)
	if err != nil {
		return "", err
	}
	if len(raw)%2 != 0 {
		return "", fmt.Errorf("odd byte length in UTF-16LE payload")
	}
	words := make([]uint16, len(raw)/2)
	for i := range words {
		words[i] = uint16(raw[i*2]) | uint16(raw[i*2+1])<<8
	}
	return string(utf16.Decode(words)), nil
}

func decodeEncoded(cmd string) string {
	if !strings.Contains(cmd, "powershell") {
		return cmd
	}

	parts := strings.Split(cmd, " ")
	for i, p := range parts {
		if p == "-E" || p == "-EncodedCommand" && len(parts) > i+1 {
			if plain, err := decodeUTF16LE(parts[i+1]); err == nil {
				parts[i+1] = plain
			}
		}
	}
	return strings.Join(parts, " ")
}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cmd/executor.go Outdated
Comment thread powershell/powershell.go
@kke kke force-pushed the powershell-command-logging branch from c1d610e to 0809ab0 Compare May 4, 2026 09:11
@kke kke requested a review from Copilot May 4, 2026 09:12
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread powershell/powershell.go
Signed-off-by: Kimmo Lehto <klehto@mirantis.com>
@kke kke force-pushed the powershell-command-logging branch from 0809ab0 to b5b16f6 Compare May 4, 2026 10:14
@kke kke requested a review from Copilot May 4, 2026 10:14
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@kke kke marked this pull request as ready for review May 4, 2026 10:31
@kke kke merged commit 9f9ecd4 into main May 4, 2026
14 of 15 checks passed
@kke kke deleted the powershell-command-logging branch May 4, 2026 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants