Skip to content

Refactor RemoteScriptCall.ashx.cs into per-route processor classes #1491

@michaellwest

Description

@michaellwest

Problem

src/Spe/sitecore modules/PowerShell/Services/RemoteScriptCall.ashx.cs reached 2723 lines covering the entire SPE remoting HTTP surface in a single class:

  • Authentication pipeline (Bearer SharedSecret HMAC + OAuth, Basic, query-string, session)
  • Throttling and impersonation
  • Dispatch on apiVersion
  • Three ProcessScript overloads (inline, by-Item, main 460-line v3 with policy validation, stream capture, output formatting, error serialization, session lifetime)
  • File upload + download with path canonicalization
  • Media upload + download with zip extraction
  • Handle-based one-shot download
  • Async long-poll wait endpoint with HMAC-signed cursors
  • Audit / CORS / error-response shaping
  • API-script discovery + cache

The size made the file slow to read, the change history hard to follow, and the security-relevant sections (auth dispatch, path resolution, policy enforcement) hard to isolate when reviewing. Project memory project_remotescriptcall_refactor.md flagged the split as backlog from the #1443 fix.

Proposed solution

Split the handler into per-concern static classes under src/Spe/sitecore modules/PowerShell/Services/Processors/ (namespace Spe.sitecore_modules.PowerShell.Services.Processors). The .ashx keeps the entry points, the dispatcher, parameter constants, ApiVersionToServiceMapping, and the v2 ApiScripts cache (used only by the dispatcher) — everything else moves out.

Class Concern
AuthProcessor AuthenticateRequest, CheckServiceEnabled, CheckServiceAuthentication, CheckIsUserAuthorized, RejectAuthenticationMethod, OAuth dispatch warning gates
WaitProcessor ProcessWaitAsync long-poll, AppendStreamsJson, TryProbeSitecoreJob, TryProbeScriptSession, TryEnforceSessionOwnership
ScriptProcessor three ProcessScript overloads + ReadRequestBody
FileProcessor ProcessFile / Upload / Download + private TryResolveFilePath
MediaProcessor ProcessMedia / Upload / Download + the GuidRegex
HandleProcessor ProcessHandle
BuiltinActionProcessor ProcessCleanup, ProcessTest
RemotingRequestContext GetIp, ResolveIp, GetRequestId, ComputeScriptHash, GetActivePolicy, GetRequestOwnershipIdentity
RemotingAuditFormat FormatRemotingClientId/Kid, FormatOAuthClientId, ResolveAuthKindForAudit, TryReadAuditSessionId
ErrorResponseWriter JsonEscape, SetErrorResponse
HttpResponseHelpers AddCorsHeaders, IsOriginAllowed, AddThrottleHeaders
ContentDispositionHelpers SanitizeFilename, AddContentHeaders
FilePathResolver GetPathFromParameters, TryCanonicalize, IsUnderRoot, ResolveUnderRoot, ResolveAgainstAllowlist
OriginAliasFolders Lazy<IReadOnlyDictionary> of the 9 origin aliases (closes the alias-dedup item from project_remotescriptcall_refactor.md)
RemotingScriptBlocks the four PowerShell const string blocks (StreamCaptureBootstrap, StructuredErrorScript, SanitizedStructuredErrorScript, FlatErrorScript)
RemotingHttpContextKeys the five HttpContext.Items key constants
LimitedReadStream / RequestBodyTooLargeException / ApiScript / ApiScriptCollection promoted from inner classes to top-level files

The .ashx uses using static directives so call sites stay verbatim — every audit log line, every field name, every <config> / <none> sentinel preserved byte-for-byte. The handler's IsReusable, async wait dispatch, and IRequiresSessionState contract are unchanged; the .ashx <%@ WebHandler %> directive still resolves to Spe.sitecore_modules.PowerShell.Services.RemoteScriptCall.

Result

  • RemoteScriptCall.ashx.cs: 2723 → 316 lines (−88%).
  • 19 new files under Processors/.
  • Build: 0 errors, 5 baseline warnings (unchanged).
  • 622 unit tests pass; 534 integration tests pass.
  • Memory item project_remotescriptcall_refactor.md: god-class split + origin-alias dedup both resolved.

Co-located test fix

SPE.LogSanitizer.Tests.ps1 and SPE.RemoteErrorVerbosity.Tests.ps1 are static-analysis source-regression sweeps that grep the handler file for LogSanitizer.SanitizeValue(...) call sites and DetailedErrors gates. The grep target widened to scan the .ashx + every .cs under Processors/. The StructuredErrorScript : SanitizedStructuredErrorScript ternary regex now allows an optional class qualifier (the constants moved into RemotingScriptBlocks).

Out of scope

  • Interface extractions / DI for processors (project rule: no abstractions beyond the task).
  • Splitting AuthenticateRequest further into per-method classes (cohesive decision tree).
  • Replacing HttpContext.Items with an explicit context object.
  • Touching RewriteUrl.cs (upstream URL rewriter, unrelated).
  • Modifying the integration test suite (it's the regression net).
  • Converting Spe.csproj to SDK-style.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions