From bd213d0218cffa6ee1c9b18a1fd3017363d13fdc Mon Sep 17 00:00:00 2001 From: Jonathan Chun Date: Thu, 6 Aug 2026 13:04:18 -0400 Subject: [PATCH 1/3] feat(manifests): unblock the diagnostics gaps from the 2026-08-05 session A recorded 3DEXPERIENCE/SQL Server incident session logged every command the validator rejected. Four were real gaps: - Format-Table -Wrap: without it long Message fields truncate mid-sentence, forcing the far more verbose Format-List. - Get-ChildItem -Include: -Filter takes one Win32 glob; searching for crash dumps, heap dumps, and javacores took three passes. - Select-String -Context: the grep -B/-A equivalent. - net accounts: the only reader for lockout/password policy; the Get-* cmdlets the denial pointed to cannot show it. net accounts is allowed via subcommand dispatch so bare net, net.exe, and every other net subcommand stay denied. The same slash switches that display the policy also set it when given values, and they reach the validator as positionals - so this adds no_positional_args, a manifest field that rejects every positional argument, keeping only the bare read-only form. positional_allowlist cannot express that: an empty values list is rejected at load time as a likely manifest bug. Co-Authored-By: Claude Fable 5 --- manifest/manifest.go | 9 +++ manifest/manifest_test.go | 2 +- manifest/manifests/powershell/denied/net.yaml | 4 +- .../manifests/powershell/format-table.yaml | 2 + .../manifests/powershell/get-childitem.yaml | 3 + .../manifests/powershell/net_accounts.yaml | 15 ++++ .../manifests/powershell/select-string.yaml | 3 + validator/validator.go | 3 + validator/windows_diagnostics_test.go | 79 +++++++++++++++++++ 9 files changed, 118 insertions(+), 2 deletions(-) create mode 100644 manifest/manifests/powershell/net_accounts.yaml create mode 100644 validator/windows_diagnostics_test.go diff --git a/manifest/manifest.go b/manifest/manifest.go index 9f2fc43..5e5e1e9 100644 --- a/manifest/manifest.go +++ b/manifest/manifest.go @@ -27,6 +27,7 @@ var SubcommandCommands = map[string]bool{ "abaqus": true, "apachectl": true, "openssl": true, + "net": true, } //go:embed manifests/*.yaml manifests/denied/*.yaml manifests/powershell/*.yaml manifests/powershell/denied/*.yaml manifests/packs/*/*.yaml @@ -91,6 +92,13 @@ type Manifest struct { // (after the pid), and jcmd GC.run, VM.set_flag, and JVMTI.agent_load all // change the target JVM. Without this, such a tool is all-or-nothing. PositionalAllowlist *PositionalAllowlist `yaml:"positional_allowlist"` + // NoPositionalArgs rejects every positional argument. This is for tools + // that are read-only bare but write-capable with arguments - `net accounts` + // displays policy, while `net accounts /forcelogoff:30` sets it. Slash-style + // switches reach the validator as positionals, so flag validation alone + // cannot catch them. positional_allowlist cannot express this: an empty + // values list is rejected at load time as a likely manifest bug. + NoPositionalArgs bool `yaml:"no_positional_args"` // AllowsFlagBundling re-enables POSIX-style short-flag bundling for a // manifest that declares shell: powershell. Native Windows executables // (netstat, tracert, ping) run on a Windows host but use DOS-style bundled @@ -299,6 +307,7 @@ func parseManifest(data map[string]any, filePath string) (*Manifest, error) { RequiresOneOf: requiresOneOf, AllowsFlagBundling: defaultBool(data, "allows_flag_bundling"), PositionalAllowlist: positionalAllowlist, + NoPositionalArgs: defaultBool(data, "no_positional_args"), }, nil } diff --git a/manifest/manifest_test.go b/manifest/manifest_test.go index c67d21f..04b38d6 100644 --- a/manifest/manifest_test.go +++ b/manifest/manifest_test.go @@ -20,7 +20,7 @@ func mustLoadEmbedded(t *testing.T) map[string]*Manifest { func TestLoadEmbeddedCountAndNameMatch(t *testing.T) { registry := mustLoadEmbedded(t) - if got, want := len(registry), 363; got != want { + if got, want := len(registry), 364; got != want { t.Fatalf("len(registry) = %d, want %d", got, want) } diff --git a/manifest/manifests/powershell/denied/net.yaml b/manifest/manifests/powershell/denied/net.yaml index 0d15779..4455e19 100644 --- a/manifest/manifests/powershell/denied/net.yaml +++ b/manifest/manifests/powershell/denied/net.yaml @@ -1,4 +1,6 @@ name: net shell: powershell deny: true -reason: "Use Get-Service, Get-NetAdapter, etc. instead." +reason: + "Only 'net accounts' (account/lockout policy, no arguments) is allowed. For everything + else use Get-LocalUser, Get-LocalGroupMember, Get-SmbShare, Get-Service, etc. instead." diff --git a/manifest/manifests/powershell/format-table.yaml b/manifest/manifests/powershell/format-table.yaml index 2b4ec6f..08eb3da 100644 --- a/manifest/manifests/powershell/format-table.yaml +++ b/manifest/manifests/powershell/format-table.yaml @@ -6,5 +6,7 @@ flags: - flag: "-Property" takes_value: true - flag: "-AutoSize" + - flag: "-Wrap" + description: "Wrap long cell text (e.g. event Message columns) instead of truncating it." stdin: true stdout: true diff --git a/manifest/manifests/powershell/get-childitem.yaml b/manifest/manifests/powershell/get-childitem.yaml index e015ec7..b01c36a 100644 --- a/manifest/manifests/powershell/get-childitem.yaml +++ b/manifest/manifests/powershell/get-childitem.yaml @@ -8,6 +8,9 @@ flags: - flag: "-Recurse" - flag: "-Filter" takes_value: true + - flag: "-Include" + takes_value: true + description: "Comma-separated patterns, e.g. 'hs_err_pid*.log','*.hprof'. Unlike -Filter, matches several patterns in one pass; combine with -Recurse." - flag: "-LiteralPath" takes_value: true - flag: "-Name" diff --git a/manifest/manifests/powershell/net_accounts.yaml b/manifest/manifests/powershell/net_accounts.yaml new file mode 100644 index 0000000..eee9f4a --- /dev/null +++ b/manifest/manifests/powershell/net_accounts.yaml @@ -0,0 +1,15 @@ +name: net_accounts +description: + Show the local account policy - lockout threshold, lockout duration, observation + window, password age limits. No Get-* cmdlet exposes this policy. +category: security +shell: powershell +# `net accounts` is read-only ONLY when bare: the same switches that would +# display nothing (/forcelogoff:N, /lockoutthreshold:N, ...) SET the policy. +# Switches are slash-style, so they reach the validator as positionals and +# flag validation cannot catch them. Only the bare read-only form is allowed; +# bare `net` and all other subcommands stay denied. +no_positional_args: true +flags: [] +stdin: false +stdout: true diff --git a/manifest/manifests/powershell/select-string.yaml b/manifest/manifests/powershell/select-string.yaml index 12af415..dfa8447 100644 --- a/manifest/manifests/powershell/select-string.yaml +++ b/manifest/manifests/powershell/select-string.yaml @@ -9,5 +9,8 @@ flags: - flag: "-Path" takes_value: true - flag: "-SimpleMatch" + - flag: "-Context" + takes_value: true + description: "Lines of context around each match, e.g. -Context 2,5 (2 before, 5 after). The grep -B/-A equivalent." stdin: true stdout: true diff --git a/validator/validator.go b/validator/validator.go index 4e01753..b9f94af 100644 --- a/validator/validator.go +++ b/validator/validator.go @@ -462,6 +462,9 @@ func validateArgs(command string, args []string, m *manifest.Manifest) error { } } } else { + if m.NoPositionalArgs { + return &ValidationError{Message: fmt.Sprintf("'%s' does not accept arguments.", command)} + } if m.AllowsPathArgs { if err := checkRestrictedPath(arg, m); err != nil { return err diff --git a/validator/windows_diagnostics_test.go b/validator/windows_diagnostics_test.go new file mode 100644 index 0000000..4c9d8f9 --- /dev/null +++ b/validator/windows_diagnostics_test.go @@ -0,0 +1,79 @@ +package validator + +import ( + "strings" + "testing" + + "github.com/fawdyinc/shellguard/parser" +) + +// These commands come from a recorded 3DEXPERIENCE/SQL Server incident session +// (2026-08-05). Each was needed during diagnosis and rejected by the validator, +// costing turns or degrading the investigation. + +func validatePS(t *testing.T, command string) error { + t.Helper() + p, err := parser.ParsePowerShell(command) + if err != nil { + t.Fatalf("parse %q: %v", command, err) + } + return ValidatePipeline(p, testRegistry(t)) +} + +// Without -Wrap, long Message fields truncate mid-sentence in table view, +// forcing a fallback to the far more verbose Format-List. +func TestFormatTableWrap(t *testing.T) { + if err := validatePS(t, "Get-Process | Format-Table Name, CPU -AutoSize -Wrap"); err != nil { + t.Errorf("Format-Table -Wrap should be allowed: %v", err) + } +} + +// -Filter takes a single Win32 glob; -Include is the only way to search for +// several file patterns (crash dumps, heap dumps, javacores) in one pass. +func TestGetChildItemInclude(t *testing.T) { + if err := validatePS(t, `Get-ChildItem -Path 'C:\logs' -Recurse -Include 'hs_err_pid*.log','*.hprof'`); err != nil { + t.Errorf("Get-ChildItem -Include should be allowed: %v", err) + } +} + +// -Context is grep -B/-A: log lines around a match, not just the match. +func TestSelectStringContext(t *testing.T) { + if err := validatePS(t, `Select-String -Path 'C:\logs\app.log' -Pattern 'OutOfMemoryError' -Context 2,5`); err != nil { + t.Errorf("Select-String -Context should be allowed: %v", err) + } +} + +// Bare `net accounts` is the only reader for lockout/password policy; the +// Get-* cmdlets the net denial points to cannot show it. +func TestNetAccountsAllowed(t *testing.T) { + if err := validatePS(t, "net accounts"); err != nil { + t.Errorf("net accounts should be allowed: %v", err) + } +} + +// net accounts with any argument is a write - /forcelogoff:30 and friends set +// the policy they otherwise display. +func TestNetAccountsRejectsArguments(t *testing.T) { + if err := validateOne(t, "net", "accounts", "/forcelogoff:30"); err == nil { + t.Error("net accounts with arguments must be rejected") + } +} + +func TestNetOtherSubcommandsStayDenied(t *testing.T) { + for _, args := range [][]string{ + {"user"}, + {"user", "eve", "/add"}, + {"localgroup", "administrators"}, + {"share"}, + } { + if err := validateOne(t, "net", args...); err == nil { + t.Errorf("net %s must stay denied", strings.Join(args, " ")) + } + } +} + +func TestBareNetStaysDenied(t *testing.T) { + if err := validateOne(t, "net"); err == nil { + t.Error("bare net must stay denied") + } +} From ead04cc82293e4528964817a0e778860f99c20c8 Mon Sep 17 00:00:00 2001 From: Jonathan Chun Date: Thu, 6 Aug 2026 13:04:25 -0400 Subject: [PATCH 2/3] fix(parser): describe the safe subset when a script block fails to parse The old message said "Script blocks are not supported", which stopped being true when the safe-expression grammar landed. The incident session showed the cost: the agent never attempted calculated properties or Where-Object blocks because the first rejection primed it to avoid every construct with braces. Name what blocks do support and what they don't. Co-Authored-By: Claude Fable 5 --- parser/powershell.go | 2 +- parser/powershell_test.go | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/parser/powershell.go b/parser/powershell.go index 8bab20f..67dedcd 100644 --- a/parser/powershell.go +++ b/parser/powershell.go @@ -359,7 +359,7 @@ func diagnoseParseError(input string, parseErr error) error { return &ParseError{Message: "Subexpressions and method calls are not allowed. Use cmdlet parameters instead."} case '{': if i == 0 || input[i-1] != '@' { - return &ParseError{Message: "Script blocks are not supported. Use simplified Where-Object syntax: Where-Object PropertyName -eq Value"} + return &ParseError{Message: "This script block does not parse under the safe subset. Blocks support only: $_ with property access, comparison/arithmetic operators, and [math]::/[datetime]:: calls - e.g. Where-Object { $_.Id -eq 4625 } or @{N='GB';E={[math]::Round($_.WorkingSet64/1GB,2)}}. Cmdlet calls and assignments inside blocks are not allowed."} } case ';': if !insideHashtable(input, i) { diff --git a/parser/powershell_test.go b/parser/powershell_test.go index 26e1f96..b086943 100644 --- a/parser/powershell_test.go +++ b/parser/powershell_test.go @@ -478,3 +478,20 @@ func TestIPLiteralDoesNotShadowExistingTokens(t *testing.T) { } } } + +// The pre-safe-expression error said only "Script blocks are not supported", +// which primed agents to avoid constructs that now work (Where-Object blocks, +// calculated properties). When a block fails to parse, the error must describe +// the supported subset rather than deny that blocks exist. +func TestScriptBlockParseErrorDescribesSafeSubset(t *testing.T) { + _, err := ParsePowerShell("Get-Process | ForEach-Object { Stop-Process $_ }") + if err == nil { + t.Fatal("cmdlet call inside a script block should not parse") + } + msg := err.Error() + for _, want := range []string{"$_", "[math]::"} { + if !strings.Contains(msg, want) { + t.Errorf("script-block parse error should mention %q, got: %v", want, msg) + } + } +} From 535c3656e913ac5a23174872a0d852871bb4aa7f Mon Sep 17 00:00:00 2001 From: Jonathan Chun Date: Thu, 6 Aug 2026 13:04:33 -0400 Subject: [PATCH 3/3] fix(server): advertise the safe-expression grammar in the WinRM hint The connect hint still said "no $ or {} or ;" - stale since the safe-expression grammar landed, and agents obey it completely. In the 2026-08-05 incident session the agent documented WorkingSet64 as unreachable and skipped jstack entirely, both because of this hint, while the validator would have accepted both. The hint now names what works: $_ in script blocks, calculated properties, [math]::/[datetime]:: calls, FilterHashtable time bounds, the JVM tools, and net accounts - plus what still doesn't. Co-Authored-By: Claude Fable 5 --- server/hint_test.go | 22 ++++++++++++++++++++++ server/server.go | 12 +++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 server/hint_test.go diff --git a/server/hint_test.go b/server/hint_test.go new file mode 100644 index 0000000..8312dee --- /dev/null +++ b/server/hint_test.go @@ -0,0 +1,22 @@ +package server + +import ( + "strings" + "testing" +) + +// The connect hint is the first thing an agent learns about the PowerShell +// dialect, and agents obey it: a recorded 2026-08-05 incident session never +// attempted $_, calculated properties, or the JVM tools because the hint said +// "no $ or {} or ;" - a claim that stopped being true when the safe-expression +// grammar landed. The hint must advertise what the dialect supports. +func TestWinRMConnectHintAdvertisesSafeExpressions(t *testing.T) { + for _, want := range []string{"$_", "[math]::", "jps", "jstack", "FilterHashtable"} { + if !strings.Contains(winrmConnectHint, want) { + t.Errorf("winrm connect hint should mention %q\nhint: %s", want, winrmConnectHint) + } + } + if strings.Contains(winrmConnectHint, "no $ or {}") { + t.Errorf("winrm connect hint still carries the stale no-$-or-{} claim\nhint: %s", winrmConnectHint) + } +} diff --git a/server/server.go b/server/server.go index 8b518c5..1a3e275 100644 --- a/server/server.go +++ b/server/server.go @@ -369,10 +369,20 @@ func (c *Core) connectWinRM(ctx context.Context, in ConnectInput, start time.Tim "host": in.Host, "shell": "powershell", "message": message, - "hint": "Use PowerShell cmdlets (Get-Process, Get-Service, Get-WinEvent, etc.). Single quotes only, no $ or {} or ;. Use | to pipe between commands.", + "hint": winrmConnectHint, }, nil } +// winrmConnectHint describes the PowerShell dialect to the connecting agent. +// Keep it in sync with the parser's safe-expression grammar: agents trust this +// hint completely and never attempt constructs it says are unsupported. +const winrmConnectHint = "Use PowerShell cmdlets (Get-Process, Get-Service, Get-WinEvent, etc.), single-quoted strings, and | to pipe. " + + "Script blocks support $_ and [math]::/[datetime]:: calls: calculated properties like Select-Object @{N='GB';E={[math]::Round($_.WorkingSet64/1GB,2)}} " + + "and filters like Where-Object { $_.Id -eq 4625 } work. " + + "Get-WinEvent -FilterHashtable accepts time bounds: @{LogName='System'; StartTime='2026-08-05 03:00'}. " + + "JVM tools (jps, jstack, jcmd, jmap, jstat) and net accounts are available. " + + "Not supported: variables/assignment, subexpressions $(...), semicolons outside hashtables, redirection." + func (c *Core) Execute(ctx context.Context, in ExecuteInput) (output.CommandResult, error) { if strings.TrimSpace(in.Command) == "" { return output.CommandResult{}, errors.New("command is required")