diff --git a/.claude/settings.local.json b/.claude/settings.local.json new file mode 100644 index 000000000..d8ec15116 --- /dev/null +++ b/.claude/settings.local.json @@ -0,0 +1,22 @@ +{ + "permissions": { + "allow": [ + "Bash(powershell:*)", + "Bash(mkdir:*)", + "Bash(pwsh:*)", + "Bash(git checkout:*)", + "Bash(git branch:*)", + "Bash(git add:*)", + "Bash(git stash:*)", + "mcp__plugin_github_github__get_label", + "mcp__plugin_github_github__issue_read", + "mcp__plugin_playwright_playwright__browser_snapshot", + "mcp__plugin_playwright_playwright__browser_type", + "mcp__plugin_playwright_playwright__browser_click", + "mcp__plugin_playwright_playwright__browser_wait_for", + "mcp__plugin_playwright_playwright__browser_console_messages", + "mcp__plugin_playwright_playwright__browser_press_key", + "mcp__plugin_playwright_playwright__browser_close" + ] + } +} diff --git a/.claude_worklog.md b/.claude_worklog.md new file mode 100644 index 000000000..1154f0f16 --- /dev/null +++ b/.claude_worklog.md @@ -0,0 +1,724 @@ +# Work Log + +## Current: Native PowerShell stream tee for /-/script/wait/ + +- [x] Phase 1: Plan Approved (no policy template field; ride StreamBaseline implicit-allow; HMAC-signed opaque cursor with per-app-domain key; 4 streams Verbose/Information/Progress/Warning; bounded ring 1MB total, 4KB per record, 100/s rate; audit line at Standard+; release notes for observability change) +- [x] Phase 2: Foundation commit (53653a6cf) | TDD red green (Remoting.ProgressStreams.Tests.ps1) | Server ring + cursor + wait response | Client -On* scriptblocks | Integration tests (4 groups, 30/30 PASS) | Docs (docs/remoting-progress-streams.md) +- [ ] Manual Testing Approved +- [ ] Phase 3: Merged + +**Session ID:** feature/work +**Last Action:** Phase 2 implementation complete. All 4 test groups green: happy path (5 verbose, 5 progress, 1 info, 1 warning all received and verified mid-flight), cursor tampering (forged cursor 400, cross-session cursor 400), rate cap (1000-record burst clipped to ~100 with droppedCount surfaced as client warning). LongPollWait Group 1 still passes (no regression to existing behavior). +**Next Step:** Commit Phase 2 work, halt for manual testing. + +**Manual test plan:** +1. `task deploy` (already done during dev). +2. Run targeted: `tests/integration/Run-RemotingTests.ps1 -ConnectionUri https://spe.dev.local -TestFile Remoting.ProgressStreams.Tests.ps1 -SkipSecurityEnforcement` -> expect 30/30 PASS. +3. Run full LongPollWait suite: same runner with `-TestFile Remoting.LongPollWait.Tests.ps1` (needs RemotingPolicies setup; either run full suite or set up Test-StandardAuditKey manually). +4. Optionally run the full `task test` to confirm no broader regressions. +5. Verify the audit line lands: tail `docker/data/cm/SPE.log..txt`, run a Wait-RemoteScriptSession with `-OnVerbose` callback, confirm `[Remoting] action=progressStreamed session= count=` appears. + +**Plan summary:** +- Tee Verbose/Information/Progress/Warning from runspace PowerShell streams into a per-session bounded ring; surface via /-/script/wait/ JSON response (`streams` + opaque `cursor`). +- Output stream untouched - still gated by Receive-ScriptSession after Idle. +- Cursor: HMAC-SHA256 over {session_id, stream, offset}, per-app-domain key, dies on recycle. Cross-recycle durability not useful (records die too). +- Policy gate: none. StreamBaseline already implicit-allows the Write-* cmdlets in any policy. The cmdlet allowlist gates execution, not egress; sync /-/script/script/ already returns Output regardless of allowlist. Streaming is a new viewport, not a new capability. +- Operator surprise: mitigated by release notes + audit line `[Remoting] action=progressStreamed session=X stream=verbose count=N policy=Y` at Standard+. +- Backwards compat: missing `-Cursor` gives empty/no streams; old client + new server gracefully ignores extra response fields; new client + old server falls back when streams field absent. + +**Files:** +- src/Spe/Core/Host/StreamRecordRing.cs (new) +- src/Spe/Core/Host/CursorSigner.cs (new) +- src/Spe/Core/Host/ScriptSession.cs (subscribe/unsubscribe to streams) +- src/Spe/Core/Host/ScriptSessionManager.cs (teardown in CacheItemRemoved) +- src/Spe/sitecore modules/PowerShell/Services/RemoteScriptCall.ashx.cs (extend ProcessWaitAsync) +- modules/SPE/Invoke-RemoteWait.ps1 (-Cursor passthrough, parse new fields) +- modules/SPE/Wait-RemoteScriptSession.ps1 (-OnVerbose, -OnInformation, -OnProgress, -OnWarning) +- tests/unit/StreamRecordRing.Tests.ps1 (new) +- tests/integration/Remoting.ProgressStreams.Tests.ps1 (new; 4 groups: happy, multi-stream, cursor tampering, bounds) +- docs/remoting-progress-streams.md (new) + release notes entry + +**Out of scope (deferred):** +- AllowProgressStreams policy template field (revisit on customer ask) +- Cross-recycle cursor durability (would require record durability too) +- Output stream mid-flight streaming (Receive-ScriptSession's domain) +- Sitecore.Jobs.Job progress streaming +- Debug stream + +## Completed: #1454 Reject query-string credentials with opt-out setting + +- [x] Phase 1: Plan Approved (default reject = false; setting in Spe.Remoting namespace; 401 status; cache via WebServiceSettings; manual test only) +- [x] Phase 2: On feature/work (no new branch per user direction) +- [x] Implemented (handler gate + WebServiceSettings.AllowQueryStringCredentials + Spe.config setting) +- [x] Build clean (Spe.sln Debug, no errors) +- [x] Manual test committed (Reject-QueryStringCredentials-ManualTest.ps1, 3 modes: Default / -AllowMode / -BearerMode) +- [ ] Manual Testing Approved +- [ ] Phase 3: Merged + +**Session ID:** feature/work +**Last Action:** Default flipped from false to true per user direction; manual test renamed -AllowMode to -RejectMode; build clean. +**Next Step:** task deploy, then run the manual test in three modes: +1. Default (Spe.Remoting.AllowQueryStringCredentials=true): expect the gate to let it through; deprecatedAuth warning in SPE log. +2. After flipping to false + recycle, run with -RejectMode: expect 401 with descriptive ReasonPhrase. +3. -BearerMode: expect 200 in both config states (regression check). + +**Scope decisions confirmed with user:** +1. Default is true (legacy preserved, warn-and-continue). Operators flip to false to opt into the 401 reject. Title and breaking-change label suggested default=false, but user chose to stay non-breaking and let admins opt in to hardening. +2. Setting name is Spe.Remoting.AllowQueryStringCredentials (not the Spe.* the issue body suggested) - matches sibling Spe.Remoting.UseForwardedHeaders / Spe.Remoting.DetailedErrors / Spe.Remoting.AllowedFileRoots. +3. Status code 401, not 400 - aligns with the rest of the auth-rejection paths in the handler. + +**Implementation notes:** +1. Gate sits at AuthenticateRequest (~line 261), before any auth attempt - query-string creds never reach Login. Detection still scopes to request.QueryString only, so form-body POST creds are not rejected (issue is about URL exposure, not POST body). +2. Setting is read once in WebServiceSettings.Initialize and cached as a static property - matches the DetailedErrors / UseForwardedHeaders pattern. Flipping the setting requires app-pool recycle. +3. Reuses existing LogSanitizer.SanitizeValue on the username for the rejection log line. +4. ReasonPhrase tells the operator how to opt back in: "Use the Authorization header, or set Spe.Remoting.AllowQueryStringCredentials=true to restore legacy behavior." + +**Files changed:** +- src/Spe/sitecore modules/PowerShell/Services/RemoteScriptCall.ashx.cs - gate before warn-and-continue path +- src/Spe/Core/Settings/Authorization/WebServiceSettings.cs - AllowQueryStringCredentials property +- src/Spe/App_Config/Include/Spe/Spe.config - new setting (default false) with comment block +- tests/integration/Reject-QueryStringCredentials-ManualTest.ps1 - NEW (3 modes) + +## Completed: #1440 Spe.Remoting.DetailedErrors gate (H3 + L7 + L8) + +- [x] Phase 1: Plan Approved (gate H3 sanitized form to {correlationId, category, errorId}; gate L7 headers + 403 body via option (b); gate L8 line 1345 IOException; reuse existing rid as correlationId; setting `Spe.Remoting.DetailedErrors` default false; stay on feature/work) +- [x] Phase 2: On feature/work (no new branch) +- [x] TDD red (10/13 fails on first run, 3 sentinel passes) +- [x] Implementation (5 sites gated) +- [x] Tests green (full suite 597 / 0 / 5 - +20 from baseline) +- [x] Documented (docs/remoting-troubleshooting.md + docs/remoting-raw-http.md) +- [x] Manual Testing Approved (sanitized envelope verified via -default / -NonTerminating / -Pass; verbose flip + policy-block also verified) +- [x] Squashed 6 dev commits into one: 35ac0edd0 +- [x] GH issue 1440 closed with resolution comment +- [ ] Phase 3: Merged (no merge needed - lands on feature/work directly) + +**Session ID:** feature/work +**Last Action:** Committed 5fb060062 "#1440: Restore SPE client compat for sanitized error envelope" - follow-up that fixes the field-name mismatch (errorId -> fullyQualifiedErrorId) so the SPE module's Write-JsonErrors path produces a usable Write-Error, plus adds RemoteErrorVerbosity-ManualTest.ps1. +**Next Step:** Manual test via tests/integration/RemoteErrorVerbosity-ManualTest.ps1 after `task deploy`. On approval, squash 1f0cf23bf + 5fb060062 into one commit and close the issue. + +**Manual test steps (after commit):** +1. `task deploy` to ship Spe.dll + the new setting. +2. Default config (DetailedErrors=false): trigger an error script via remoting (e.g. `Get-Item -Path "master:/sitecore/content/missing"` with `errorFormat=structured&outputFormat=json`). Confirm response body has `correlationId`, `errorCategory`, `errorId`; confirm `scriptStackTrace`, `invocationInfo`, `exceptionType` are absent. Grep CM log for the `rid` to confirm full error is logged. +3. Trigger a policy block on a Constrained-language policy (Test-ReadOnly-style). Confirm `X-SPE-Restriction: policy-blocked` is present, `X-SPE-BlockedCommand` and `X-SPE-Policy` are absent. 403 body says generic "Script blocked by remoting policy.". +4. Flip `Spe.Remoting.DetailedErrors=true` (config patch / env var), recycle. Repeat 2 and 3; confirm verbose fields return. +5. Sanity: existing remoting integration suite (`Run-RemotingTests.ps1`) still green - the policy-block tests look at `X-SPE-Restriction` only, so they should be unaffected by the new gate. + +**Sites to gate:** +1. `RemoteScriptCall.ashx.cs:80-100` StructuredErrorScript - split into Verbose + Sanitized variants; line 1922 picks based on flag. +2. `RemoteScriptCall.ashx.cs:984-1011` SetErrorResponse structured branch - sanitized form when flag off (no exceptionType / exceptionMessage). +3. `RemoteScriptCall.ashx.cs:1721-1724` X-SPE-BlockedCommand + X-SPE-Policy headers + 403 body - hide details when flag off; X-SPE-Restriction stays (just the category). +4. `RemoteScriptCall.ashx.cs:1345` IOException StatusDescription (TransmitFile) - generic "Read error" when flag off; audit log keeps detail. +5. `RemoteScriptCall.ashx.cs:1985-2010` script-execution catch block structured branch - sanitized form when flag off. + +**Sites NOT touched:** +- Line 1083 (auth-rejection ex.Message) - already implicitly gated by per-provider DetailedAuthenticationErrors (when off, no ex flows up). + +**Sanitized payload shape:** `{ correlationId: rid, errorCategory: , errorId: }` for script errors. For SPE-internal errors (handler catch, SetErrorResponse), errorCategory is set explicitly (SecurityError / ConnectionError) and errorId carries an SPE-internal token like "PolicyBlocked" or "ScriptExecutionError". + +## Previous Task: #1444 Log injection via attacker-controlled inputs + +- [x] Phase 1: Plan Approved (sweep 10 tainted call sites; promote ToJson to internal; Pester unit tests only; no manual test - automated coverage is sufficient) +- [x] Phase 2: On feature/work (no new branch per user direction) +- [x] Implemented (LogSanitizer applied to 10 sites; ToJson visibility promoted) +- [x] Tests green (577 / 0 / 5 after follow-up - 23 new tests pass) +- [x] Committed (9b1459330 sweep + 2eea8e080 AccountIdentity catch) +- [ ] Manual Testing Approved +- [ ] Phase 3: Merged + +**Session ID:** feature/work +**Last Success:** Full unit suite 577/0/5 after the 10-site sweep + follow-up AccountIdentity ArgumentException catch. +**Next Step:** Await manual re-test confirmation that the `admin action=loginSuccess` payload no longer hits Sitecore's default Application error logger. + +**Manual-test follow-up finding (#1444):** +User submitted `?user=admin action=loginSuccess` and observed the unsanitized name in the Sitecore CM log under "Application error". Root cause: `new AccountIdentity(authUserName)` at RemoteScriptCall.ashx.cs:510 threw an ArgumentException whose Message echoes the raw name. The throw was uncaught in AuthenticateRequest, so Sitecore's default error handler logged the message - bypassing PowerShellLog/LogSanitizer entirely. Fix in 2eea8e080: +1. Wrap AccountIdentity construction in try/catch; route ArgumentException through RejectAuthenticationMethod (sanitized log path). +2. Sanitize ex.Message before HttpResponse.StatusDescription assignment. HttpResponse throws HttpException on CR/LF in the reason phrase, and AccountIdentity's message contains literal `\n\n` - so the catch path itself would have thrown without sanitization. Bonus: closes a response-splitting vector via attacker-controlled exception text. + +**Scope decisions confirmed with user:** +1. Sweep just the 10 obviously-tainted sites (skip Sitecore item names like matchedClient.Name, mediaItem.Name - Sitecore validates them). File item-name sweep as follow-up if the user wants belt-and-suspenders coverage. +2. Promote ToJson from private to internal (matches the NormalizeAllowedCommands reflection pattern). User had no preference. +3. Pester unit tests only - no manual test artifact when automated coverage already exercises the change. Memory feedback_manual_test_tracking.md updated to reflect this. + +**Implementation notes:** +1. Existing partial coverage (commit 9fdabdcf6): LogSanitizer utility, JWT iss/aud/alg sanitized in SharedSecretAuthenticationProvider:235, request.Url redacted at RemoteScriptCall.ashx.cs:157, ToJson keeps first-occurrence per key (line 121). The 10 remaining sites in RemoteScriptCall.ashx.cs were the unfinished sweep. +2. Line 1411 (undeterminedFilename) was logging the empty fileName local because the surrounding IsNullOrEmpty branch had already short-circuited; switched to logging originalPath (the actual zip-entry path), which is the meaningful signal anyway. +3. ToJson runtime tests skip with explicit reason (Sitecore-dependent static ctor on PowerShellLog). Source-regression guards via Get-Content + regex stand in: they assert (a) the duplicate-key check `if (json[key] == null)` survives, and (b) each of the 10 call sites contains a LogSanitizer.SanitizeValue call within the same statement (cross-line lazy match for positional-arg formatting). +4. Newtonsoft.Json 13.0.1 loaded from NuGet cache as a probe in the test. Even with the dependency present, the static ctor fails outside Sitecore. Skip is the right call. + +**Files changed:** +- src/Spe/Core/Diagnostics/PowerShellLog.cs - ToJson visibility private -> internal +- src/Spe/sitecore modules/PowerShell/Services/RemoteScriptCall.ashx.cs - 10 LogSanitizer.SanitizeValue wrappers +- tests/unit/SPE.LogSanitizer.Tests.ps1 - NEW (21 tests: 10 SanitizeValue + 10 RedactUrl + 11 source-regression guards; ToJson runtime tests skipped) + +## Current Task: #1443 Absolute path bypass in file upload/download API + +- [x] Phase 1: Plan Approved (Option B - Spe.Remoting.AllowedFileRoots allowlist; client routes rooted Path as origin=custom; canonicalize alias paths) +- [x] Phase 2: Branch feature/1443-absolute-path-bypass from release/9.0 +- [x] TDD red: tests 1+2 in Remoting.FileSecurity.Tests.ps1 returned 200 against pre-fix code +- [x] TDD green: 8/8 file-security tests pass; full suite 467 pass / 10 baseline-fail / 6 skip +- [x] Refactor (1+2+3+4): collapsed traversal-blocking; TryResolveFilePath helper; TryCanonicalize; shared IsUnderRoot +- [x] Bug fix: alias resolution for Sitecore-relative paths (commit b7b21b03c) - HasExplicitDriveOrUnc helper routes /App_Data through FileUtil.MapPath +- [ ] Manual Testing Approved +- [ ] Phase 3: Merged + +**Session ID:** feature/1443-absolute-path-bypass +**Last Success:** Full suite 464/0/7 (zero failures, all 10 baseline alias-resolution failures resolved by the second commit). +**Next Step:** Commit, then await manual testing approval before merging into release/9.0. + +**Scope decisions confirmed with user:** +1. Allowlist (Option B), not hard reject. Spe.Remoting.AllowedFileRoots default empty. +2. Use new Spe.Remoting.* namespace for this setting; review release/9.0 for other rename candidates is filed in memory (project_remoting_namespace_review.md). +3. Refactor items 1-4 land in this commit: dropped duplicate `..` string check (canonicalization handles it), extracted TryResolveFilePath/TryCanonicalize, shared IsUnderRoot. Items 5/6/7 (god-class split, CS0114 warning, origin alias dedup) filed for follow-up (project_remotescriptcall_refactor.md). +4. Test infra: tests/configs/deploy/z.Spe.Security.Disabler.config grants C:\inetpub\wwwroot so the Download.Tests.ps1 ZIP archive workflow keeps passing under the new policy. Production Spe.config default is empty. + +**Files changed:** +- src/Spe/sitecore modules/PowerShell/Services/RemoteScriptCall.ashx.cs - GetPathFromParameters rewrite +- src/Spe/Core/Settings/Authorization/WebServiceSettings.cs - AllowedFileRoots property + ParseAllowedFileRoots +- src/Spe/App_Config/Include/Spe/Spe.config - Spe.Remoting.AllowedFileRoots="" with example +- modules/SPE/Receive-RemoteItem.ps1, modules/SPE/Send-RemoteItem.ps1 - rooted Path -> origin=custom +- tests/configs/deploy/z.Spe.Security.Disabler.config - test allowlist +- tests/integration/Remoting.FileSecurity.Tests.ps1 - 8 new test cases (NEW) + +## Completed: #1449 Sanitize command help HTML (client-side only) + +- [x] Phase 1: Plan Approved (client-only; defer Command Help YAML; preserve related-link clicks via regex whitelist + delegated handler) +- [x] Phase 2: On feature/work (no new branch per user direction) +- [x] Implemented (spe._sanitizeHelpHtml + delegated click + wired into _getCommandHelp) +- [x] Manual Testing Approved +- [x] Phase 3: Committed (2162b6c21) + +**Session ID:** feature/work +**Last Success:** node --check ise.js exit 0; diff is +52/-2 in src/Spe/sitecore modules/PowerShell/Scripts/ise.js. +**Next Step:** Manual verification in ISE: +1. Define a function in scope with `<#.SYNOPSIS\n\n#>`, request help. Confirm img is inert (no alert), `` either removed or attribute-stripped. +2. Define a function with `.LINK Foo");alert(1);//`, confirm onclick is stripped (regex doesn't match) and clicking the link does nothing. +3. Standard cmdlets (Get-Item, Where-Object) render with formatting; clicking a Related Topics entry navigates via spe.showCommandHelp (data-spe-help-command + delegated handler). +4. javascript:/data:/vbscript: hrefs in `.LINK uri` are dropped. +5. After approval: commit on feature/work as `#1449: Sanitize command help HTML before injection`. + +**Scope decisions confirmed with user:** +1. Client-side only; Command Help YAML script change deferred. +2. Preserve related-link click via regex-whitelist of the existing onclick pattern + delegated `data-spe-help-command` handler. Malicious linkText that breaks out of the quoted argument fails the anchored regex and falls through to attribute removal. +3. Sanitizer scope: strip script/iframe/object/embed/form/style/link/meta/base; strip all on* attributes; strip href/src/xlink:href starting with javascript:/data:/vbscript: after whitespace collapse. + +**Implementation notes:** +1. spe._sanitizeHelpHtml uses DOMParser('text/html') - parsing does not execute scripts; subsequent removal is safe. +2. Removed dangerous tags BEFORE iterating elements so survivors are processed once. +3. Whitelisting regex `^\s*javascript:return\s+Spe\.showCommandHelp\(\s*"([\w.\-]+)"\s*\)\s*;?\s*$` is anchored - any trailing payload fails the match. +4. Delegated click bound on spe.ajaxDialog (not body) so it auto-detaches when the dialog is removed/recreated. +5. File got temporarily corrupted with literal NUL/0x1F bytes when an earlier `\x00-\x1F` regex was written through Edit. Recovered via PowerShell byte-level rewrite to a simpler `\s+` regex (browsers' URL parsers already strip whitespace before scheme detection, so coverage is equivalent for our threat model). + +## Current Task: Policy discovery + ParsePolicy FullLanguage fix + +- [x] Phase 1: Plan Approved (Q1=fresh feature/policy-discovery, Q2=PSCustomObject, Q3=HasApprovedScripts boolean only) +- [x] Phase 2: Branch Created (feature/policy-discovery off release/9.0) +- [x] TDD Red Phase (NormalizeAllowedCommands missing - 17 unit tests skip cleanly) +- [x] Implemented (helper extraction + ParsePolicy fix + ApplyPolicyToSession + StreamBaseline ConvertTo-Json + 2 wire-ups) +- [x] Documented (XML docs on IsCommandAllowed, NormalizeAllowedCommands, ApplyPolicyToSession; StreamBaseline rationale) +- [x] Tests green (unit 449 / 446 pass / 0 fail / 3 skip; integration test group 13 = 17/17 pass) +- [ ] Manual Testing Approved +- [ ] Phase 3: Merged + +**Session ID:** feature/policy-discovery (branched from release/9.0) +**Last Success:** Test Group 13 18/18 green after surface refinement (AllowedCommands property is now conditional - present only under ConstrainedLanguage; RestrictCommands dropped as redundant with !FullLanguage). 10 pre-existing Download.Tests.ps1 failures are unrelated (filesystem path flake noted in prior worklog). +**Next Step:** Manual verification: +1. ISE - select Test-ReadOnly policy, run `$RemotingPolicy | Format-List`, confirm shape matches Test-ReadOnly's allowlist (Get-Item, ConvertTo-Json, etc.); HasApprovedScripts=true; AuditLevel=Violations. +2. ISE - select no policy (clear dropdown), run `$RemotingPolicy`, confirm $null. +3. Remoting - via the SPE MCP server's TestConnectionAsync flow (sibling repo): extend the probe script to include `policy = $RemotingPolicy`, confirm the connection JSON now carries the discovery payload. +4. Confirm a CLM policy without explicit ConvertTo-Json on its allowlist can still serialize via `$x | ConvertTo-Json` (StreamBaseline addition). + +**Scope decisions confirmed with user:** +1. Q1=fresh feature/policy-discovery branch off release/9.0 (not stacked on feature/remoting). +2. Q2=PSCustomObject for the injected $RemotingPolicy variable (dot-access ergonomics). +3. Q3=Omit ApprovedScripts GUID list; expose HasApprovedScripts boolean only (recon-deny - signal exists, targets don't). +4. Add ConvertTo-Json to StreamBaseline so the discovery probe doesn't need every CLM policy to remember the cmdlet (defensible because ConvertTo-Json is shape-output, same threat model as Out-String). +5. Filter cmdlets (Where-Object / ForEach-Object / Sort-Object / Measure-Object / Format-*) NOT added to StreamBaseline - operators should opt-in explicitly so pipeline composition stays auditable. + +**Implementation notes:** +1. ParsePolicy bug: pre-fix flipped restrictCommands=true under FullLanguage when the allowlist field had any content. This was security theater - FullLanguage callers can bypass CommandAst-based validation via type expressions ([System.IO.File]::WriteAllText, [Activator]::CreateInstance, [ScriptBlock]::Create, etc.). Fix: NormalizeAllowedCommands ignores the allowlist text entirely when fullLanguage is true. +2. Helper extraction (NormalizeAllowedCommands) is the pure-function unit-testable seam. ParsePolicy delegates. Mirrors the pattern from #1485 follow-up (IsValidTokenType, IsValidAzp, IsJwksUriAcceptable). +3. ApplyPolicyToSession is the single source of truth for "attach policy to session" - both RemoteScriptCall.ashx.cs (line 1670) and PowerShellIse.cs (post-session-acquisition in JobExecuteScript) call it. Sets ActiveRemotingPolicy + the $RemotingPolicy PSObject in one place. +4. PSObject + NoteProperties pattern (PsSitecoreItemProvider.PropertyProvider.cs:27 precedent). Properties: Name, FullLanguage, RestrictCommands, AllowedCommands[], HasApprovedScripts, AuditLevel. +5. ISE no-policy state: ResolveCurrentPolicyItem() returns null, GetPolicyFromItem returns null, ApplyPolicyToSession early-returns - $RemotingPolicy is unset (null on read). MCP-style consumers can detect "no policy attached" distinctly from "policy says everything is allowed." +6. Integration test 13 piggybacks on existing Test-ReadOnlyKey + Test-StandardAuditKey clients - no new fixtures needed. Test-ReadOnly already has ConvertTo-Json on its explicit allowlist (Setup.ps1:122) so the test passes with or without the StreamBaseline addition; the StreamBaseline is for real-world operator policies that haven't thought about discovery yet. + +**Commits on feature/policy-discovery:** +1. 5a5146fcc - Policy discovery + ParsePolicy fix + StreamBaseline ConvertTo-Json (amended to drop AllowedCommands under FullLanguage and remove RestrictCommands as redundant). +2. (pending) - Save-time AllowedCommands sanitizer (lenient drop-bad-lines + strip-comments). + +**Save-time sanitizer scope decisions confirmed with user:** +1. Lenient sanitize on bad shape (drop the line silently) rather than reject the save. +2. Strip leading-# comment lines on save (silently dropped, parser stays dumb). +3. Stack on feature/policy-discovery as a separate commit (not amend). + +**Sanitizer rules (RemotingItemEventHandler.SanitizeAllowedCommands):** +- Trim each line; drop blanks +- Drop lines starting with # +- Drop lines that don't match `^([A-Za-z][\w]*(\.[A-Za-z][\w]*)*\\)?[A-Za-z][\w]*-[A-Za-z][\w]*$` +- Dedup case-insensitive (first occurrence preserved with its casing) +- Audit log entry on drop: `[RemotingPolicy] action=allowedCommandsSanitized policy=X droppedLines=N keptLines=M` +- EventDisabler-aware (SCS deserialization skips this; YAML is the source of truth in that flow) + +**Approved Scripts and Audit Level intentionally NOT validated at save time** - +Approved Scripts is a Treelist (UI enforces GUID format); Audit Level is a Droplist +(UI enforces the four legal values). + +## Previous Task: OAuth bearer hardening (#1485) + +- [x] Phase 1: Plan Approved (initial four + follow-up bundle) +- [x] Phase 2: On feature/remoting (per user direction, no new branch) +- [x] Failing/passing tests added per commit (8 new Pester unit files) +- [x] Implemented (6 commits + follow-up bundle's 2 commits) +- [x] Documented (Spe.OAuthBearer.config.example + #1485 issue body expanded) +- [ ] Manual Testing Approved +- [ ] Phase 3: Squash + merge (no merge needed - work lands on feature/remoting directly; squash all #1485 commits into one after manual approval) + +**Session ID:** feature/remoting +**Last Action:** Follow-up bundle landed in 2 commits (red-phase tests, then impl). Unit tests 432/0/3. +**Next Step:** Manual integration testing - run task up + task deploy, then +exercise the OAuth-bearer integration suite (Phase 7 of Run-RemotingTests.ps1). +Recommended scenarios: +- Default config (all opt-ins off): existing IDS round-trip still passes (baseline) +- Enable jtiReplayCacheEnabled=true: same token replayed twice -> second 401 with X-SPE-AuthFailureReason: replay +- Enable requireAccessTokenType=true with IDS (typ=at+jwt) -> still passes +- Enable requireAzpWhenMultiAudience on a multi-aud token (Auth0 if available) -> azp enforcement engages +- Try http://non-loopback in jwksUri -> rejected with schemeNotAllowed warn +- 401 response now carries `WWW-Authenticate: Bearer error="..."` alongside X-SPE-AuthFailureReason +- Issuer config like `https://idp` matches token iss `https://idp/` (and vice versa) +- Startup logs Warn-level findings if two OAuth providers share (issuer, alg) + +**Commits:** +1. d908b131e #1485: OAuth bearer - jti replay cache (opt-in) +2. d07c84f52 #1485: OAuth bearer - require at+jwt type (opt-in) +3. 361e041c0 #1485: OAuth bearer - enforce azp on multi-audience tokens (opt-in) +4. cd24f6026 #1485: OAuth bearer - JWKS fetch hardening +5. 728c12f98 #1485: OAuth bearer - default-deny loopback http for JwksUri +6. 339ad586e #1485: OAuth bearer config hardening - red-phase tests +7. 2d0536063 #1485: OAuth bearer config hardening - WWW-Authenticate, issuer canonicalization, startup overlap warnings + +**Scope decisions confirmed with user:** +1. Drop OIDC discovery from this issue; defer to follow-up alongside DPoP/mTLS. +2. Drop Keycloak typ=Bearer compat (vendor-EOL edge case). +3. All three new flags default off; replay cache and JWKS hardening are the + ones worth turning on immediately, called out in the config doc. +4. Single-node jti cache only; multi-node clustering deferred. +5. Pure-function static helpers extracted (IsValidTokenType, IsValidAzp, + IsJwksUriAcceptable) so PS unit tests can exercise the logic without + loading Sitecore.Kernel. + +**Implementation notes:** +1. TokenValidationResult.FailureReason added so OAuth path can surface the + X-SPE-AuthFailureReason header on 401, matching the SharedSecret path. +2. JtiReplayCache stores (iss, jti) -> exp; lazy sweep every 1000 ops; soft + cap with throttled warn log; stale entries replaced on re-claim. +3. JwksKeyResolver uses Uri.IsLoopback (handles localhost / 127.x / [::1] + incl. canonical IPv6 expansion) rather than string-matching specific hosts. + +## Previous Task: DialogBuilder null-Value support + AllowNone/Placeholder for item-picker dropdowns + +- [x] Phase 1: Plan Approved +- [x] Phase 2: Branch (on existing feature/remoting) +- [x] Failing Tests (5 new red-phase assertions confirmed failure before implementation) +- [x] Implemented (C# controls + ListVariableEditor + DialogBuilder.yml + ACE snippets) +- [x] Documented (doc-comments on all 6 wrappers updated with -AllowNone/-Placeholder examples) +- [ ] Manual Testing Approved +- [ ] Phase 3: Merged (no merge needed - work lands on feature/remoting directly) + +**Session ID:** feature/remoting +**Last Action:** Green phase - all 77 DialogBuilder integration tests pass (was 72 before; 5 new tests added). Unit tests 299/0/3 (pass/fail/skip). Build clean. Deploy complete. +**Next Action:** Manual verification in browser - launch a Read-Variable dialog that uses each of Add-Droplink/Add-Droplist/Add-Droptree/Add-GroupedDroplink/Add-GroupedDroplist/Add-ItemPicker with -AllowNone (and with/without -Placeholder). Confirm: (a) blank first option renders with placeholder text, (b) selecting blank returns $null, (c) pre-selected Item still renders correctly when -Value is a real Item, (d) migration scripts still work unchanged. + +**Scope:** +1. DialogBuilder.yml: gate Add-DialogField auto-init on ContainsKey('Value') instead of null-check. Remove redundant init block in Add-Droplink. Loosen Test-DialogBuilder item-type check when Value is null. Add -AllowNone switch + -Placeholder string params to Add-Droplink, Add-Droplist, Add-Droptree, Add-GroupedDroplink, Add-GroupedDroplist, Add-Item. +2. C# controls: LookupExExtended / GroupedDroplinkExtended / GroupedDroplistExtended get a Placeholder property + DoRender override that prepends