docs: add codebase-review remediation plan (HLPS + IS) by hegsie · Pull Request #765 · sefe/dorc

hegsie · 2026-07-06T05:55:57Z

Adds a High-Level Problem Statement and Implementation Sequence covering all
findings from the full-codebase review (authorization gaps, injection/RCE,
runner/monitor execution-engine safety, secret handling, error handling,
data-access correctness, and web frontend). Both documents are DRAFT pending
adversarial review and resolution of the blocking unknowns register.

Co-Authored-By: Claude Fable 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d

Adds a High-Level Problem Statement and Implementation Sequence covering all findings from the full-codebase review (authorization gaps, injection/RCE, runner/monitor execution-engine safety, secret handling, error handling, data-access correctness, and web frontend). Both documents are DRAFT pending adversarial review and resolution of the blocking unknowns register. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d

Terraform authz (S-001) mirrors CanModifyEnvironment; the fn: C# evaluator (S-002) is an actively-used feature, so the step becomes sandbox + permission-gate hardening rather than removal. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d

Three-reviewer panel (coverage, sequencing/risk, solution-correctness). Material corrections: - S-010: Terraform exit codes made command-aware (-detailed-exitcode 2 = success-with-changes; blanket nonzero=fail would break every changed plan) - S-010/S-012: declared a paired release; process-tree kill flagged as new work (no job object exists today) - S-014: re-scoped around environment-lock probing; reconciled with the already-delivered monitor-robustness S-004; U-7 promoted to blocking - S-004: account model corrected (env-specific deploy account + Monitor's real service identity, not literal SYSTEM); pipe-name parseability noted - S-002: ScriptOptions-imports sandbox corrected (not a boundary); safe expression-evaluator option added; AppDomain removed; timeout added - S-006: reframed (runner writes status directly to DB; DeployApiClient is dead code); authorize on deploymentResultId's owning request - S-011: transient-exception set enumerated; circuit-breaker/backoff - New Tier-1 steps: S-000 (A-4 server-side-enforcement spike), S-012a (busy-loop hotfix), S-021 XSS promoted from Tier 3 - Step Index dependency column populated; intro gate IDs corrected - Status: DRAFT -> REVISION Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d

Excludes quote and bracket characters from the login-name allow-list, embeds the name via an escaped quoted identifier and escaped string literal, and builds the connection with SqlConnectionStringBuilder so targetDbServer cannot inject connection-string keywords. Extracts IsValidLoginName / BuildResetLoginSql as testable statics with unit tests. The password-equals-username behaviour is retained intentionally: the UI documents it as a reset-to-default the user then changes. The defect was the injection, not the predictability (deviation recorded in the IS). Addresses finding B-1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d

Replaces the HasView/Confirm/DeclinePermission stubs (which all returned true) with real checks: the owning environment is resolved from the deployment result's request and gated on CanModifyEnvironment, mirroring the request-lifecycle endpoints. View is gated at the same modify level because the ACL model has no distinct read tier and plan content is sensitive infrastructure detail; fails closed when the environment cannot be resolved. Also switches the denial paths from Forbid(message) — which ASP.NET Core interprets as an auth scheme name and would throw once reachable — to StatusCode(403, message). Addresses finding A-1. Delivered with TerraformControllerTests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d

int.TryParse(out delay) set the delay to 0 whenever the key was missing or unparseable, discarding the 1000ms default and turning the processing loop into a busy spin (with a forced full GC every iteration). Now only a parsed value > 0 is used; otherwise the default floor applies. Adds the key to the shipped appsettings.json. Addresses finding C-7. Delivered with MonitorConfigurationTests cases. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d

Removes the catch-all that replaced a malformed IV/Key with a fresh random AES key, which produced undecryptable ciphertext and masked misconfiguration. Construction no longer throws (so deployments with no legacy v1 data are unaffected), but any actual use of a mis-initialised legacy encryptor now throws a clear InvalidOperationException rather than silently operating with a random key. Addresses finding D-1. Updates PropertyEncryptorTests (the prior test asserted the buggy fallback round-tripped). CLI dry-run audit for existing undecryptable values remains as a follow-up companion action. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d

RequestStatusesController.Patch, RawLog (Post) and GetLog performed no per-resource authorization, letting any authenticated user append to any deployment's log, repoint any request's log path to an arbitrary UNC share, or read any request's log by id. - Patch: authorizes on the environment owning the deployment result being mutated (resolved from deploymentResultId, not the caller-supplied requestId which the mutation ignores) via CanModifyEnvironment. - RawLog: validates the UNC path and authorizes via CanModifyEnvironment. - GetLog: only returns a log for a request the caller can resolve. These endpoints have no live client (the runner writes status/logs directly to the DB), so the lock-down does not affect deployments. Addresses finding A-2. Delivered with RequestStatusesControllerTests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d

ExceptionJsonConverter serialized the stack trace, full inner-exception chain, and fully-qualified type for every StatusCode(500, e) return, disclosing internal paths and dependency internals to any caller that triggered a handled exception. It now emits only a short, safe shape (type name + generic message + exception message), matching the sanitized DefaultExceptionHandler used for unhandled exceptions. Also fixes the one site that bypassed the converter by returning exception.ToString(). Addresses finding E-1 (E-5 validation-status/NRE cleanups remain as follow-up; their disclosure impact is now neutralized). Delivered with ExceptionJsonConverterTests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d

Both PowerShell runners serialized the full property value (which may be a decrypted secret) into the runner log file, stdout, and OpenSearch whenever SessionStateProxy.SetVariable threw. They now log only the variable name and its type. Addresses finding D-3; the Terraform-output masking and on-disk tfvars halves of D-2 are handled in S-010. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d

@click

Ten combo-box/grid renderers assigned backend-controlled strings (server, database, environment, script, build, user, bundle values) directly into root.innerHTML, so a value containing e.g. <img src=x onerror=...> executed in a viewing admin's session. All are converted to Lit's auto-escaping render(html`...`, root); the json-viewer post-render expand and a build-link click handler are preserved (the latter via a Lit @click binding). No unsafeHTML introduced; tsc --noEmit is clean. Addresses finding G-1. Adds tests/components/renderer-xss.test.ts (type-checks; executes in CI where the Playwright browser is installed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d

The dev-flag metadata changes were npm normalization unrelated to the XSS fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d

Frontier reviewers (Opus 4.8 + Fable 5) audited the Tier-1 diff. Accepted fixes applied: - S-007: the top-level exception Message is the common leak vector (e.g. SqlException login/DB names). Dropped it from both ExceptionJsonConverter and DefaultExceptionHandler (still logged server-side). Prior test only checked the inner message; added a top-level-message assertion. - S-006 GetLog: GetRequestForUser performs no permission filtering, so the existence check was not authorization. Now requires CanModifyEnvironment on the owning environment. - S-006 IsValidUncPath: reject '..' traversal, ':' (drive-qualified / NTFS ADS), and drive-letter admin shares (C$); named hidden shares still allowed. Added test rows. - S-009: dispose the partially-initialised Aes on init failure. All affected tests pass (Api 23, Core 33). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d

The processing loop caught only a bare SqlException; a RetryLimitExceededException (which EF throws wrapping a SqlException once retry-on-failure is exhausted) fell through to catch(Exception){throw}, stopping the entire Monitor service (default StopHost) until manually restarted. DeploymentEngine now classifies transient failures by walking the inner-exception chain (SqlException/TimeoutException/RetryLimitExceeded), logs and retries them with backoff, and continues — preserving in-flight _runningTasks state. Genuinely unexpected exceptions are handled by a consecutive-failure circuit breaker (capped exponential backoff, rethrow after 5 in a row so orchestration can restart) rather than an immediate crash or an infinite silent spin. Addresses finding C-5. Delivered with DeploymentEngineTransientTests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d

The ComponentProcessor switch had an empty default: case, so an unrecognised ComponentType left the status at StatusNotSet and the method returned 'not Failed' == true — the request completed as success with nothing deployed. The default now marks the result Failed and logs/records the reason. Addresses finding C-9 (the C-10 stale-Win32-error part is Windows-only, deferred). Delivered with ComponentProcessorFailClosedTests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d

… model EnvironmentsPersistentSource.AttachServerToEnv/DetachServerFromEnv caught every exception and returned an empty EnvironmentApiModel(), logging nothing and making the controller's ArgumentOutOfRangeException handler (which surfaces the 'Invalid or unknown server Id' message) unreachable dead code. Both now log and rethrow so the controller can translate the error. Addresses part of finding E-2. The deployment-hot-path swallows (PropertyEvaluator/VariableResolver) and the logger-less PropertyValues/ConfigValues removes (E-3) are deferred to a pass with integration coverage (see IS note). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d

…ware RunTerraformCommandAsync only treated exit code 1 as failure, so a crashed or OOM-killed 'apply' (exit code other than 0 or 1) was silently reported as success and the deployment marked Complete (finding C-3). Interpretation is now command-aware via IsTerraformCommandSuccessful: a plan run with -detailed-exitcode treats 0 and 2 as success (2 = succeeded-with-changes, the normal case) and 1/other as failure; every other command fails on any non-zero code. This avoids the regression a blanket 'non-zero = fail' would cause (marking every changed plan Failed). Cancellation/process-tree-kill (C-4), stdout drain (C-11) and tfvars cleanup (D-2) remain deferred to a Windows/integration pass (paired S-012). Delivered with TerraformExitCodeTests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d

SecurityObjectFilter.HasPrivilege returned false whenever ANY deny bit was set (denied <= 0 && allow), so a Deny of Write also blocked ReadSecrets and Owner — a user could be locked out of every access level by an unrelated deny (finding F-2). Deny is now scoped to the requested level via the extracted pure IsAllowed(allowed, denied, accessLevel), and the misleadingly-named IsBitSet mask helper is removed. The F-1 whole-table-load in AccessControlPersistentSource needs query-level verification and is deferred. Delivered with SecurityObjectFilterTests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d

fn: property values were handed straight to Roslyn CSharpScript, which executes arbitrary C# — anyone able to author a property value could run System.IO.File / System.Diagnostics.Process / reflection on the deployment host (finding B-2, RCE). U-2 resolved: fn: is used only for simple string and maths operations, so rather than sandboxing arbitrary C# (not a real boundary) the expression is now validated against a fail-closed Roslyn AST allow-list BEFORE execution. SafeExpressionValidator permits only literals, arithmetic/comparison/logical operators, and method/property access on string/number literals or the safe static types Math/Convert. Bare identifiers (how a type like File/Environment is referenced), typeof, new, lambdas, GetType (reflection entry point), and trailing statements are all refused. Arbitrary type resolution is impossible, so the RCE class is removed by construction, not merely mitigated. Adds a 5s evaluation timeout (runaway-expression DoS) and fixes the blocking .Result. Existing fn: expressions (e.g. "abc".ToUpper(), 2+3*4) still evaluate with identical semantics. Delivered with SafeExpressionValidatorTests (27 cases); existing VariableResolverTests still pass. Authorship permission-gating remains a defence-in-depth follow-up (RCE is already eliminated). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d

The frontier adversarial panel broke the first cut: "".GetType() with a Unicode-escaped GetType passed the deny-list because Identifier.Text returns the raw escaped spelling (!= "GetType") while the compiler still bound the real GetType(), reopening the entire reflection surface (empirically wrote a file to disk). Rewrite of SafeExpressionValidator: - compare Identifier.ValueText (the compiler-bound name), not .Text, so escaped spellings cannot slip past; - invert the member policy to an ALLOW-LIST — reflection entry points (GetType/Assembly/GetMethod/MakeGenericType/DynamicInvoke/CreateInstance) are simply absent, so owning any value can no longer reach a Type/Assembly; - reject generic member names (their type-argument list is attack surface); - exclude PadLeft/PadRight from the instance allow-list (a single call with a huge argument allocates multi-GB and the cooperative timeout can't stop it); - add the ternary conditional for compatibility. Both reviewers' full attack sets, including the weaponized reflection chains, are now rejected; legitimate expressions ("abc".ToUpper(), Math.Max, etc.) still evaluate. Predefined-type statics / interpolation remain rejected fail-safe (documented compat limitation). Regression tests added. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d

…nvert branch Round-3 frontier re-attack confirmed no HIGH/CRITICAL bypass survives, and found that Math/Convert expressions throw CS0103 at runtime because the scripting host runs with no imports (verified empirically: only string methods on literals and arithmetic operators actually execute). The blanket Math/Convert static-receiver branch was therefore dead at runtime while being the one place the validator granted name-blind member access. Removed it: every receiver must now be a value, and ALL bare identifiers are rejected. This matches what can actually run, loses no working behaviour, and eliminates the reviewer's concern that the safety guarantee rested on the runtime happening to lack a System import. Also adds the ternary to the allowed set. Core suite green (195). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d


-                var objCmd = new SqlCommand(sql, objConn);
-                var returnVal = objCmd.ExecuteScalar();
+                using var objCmd = new SqlCommand(sql, objConn);


+                    }
+
+                    using var cts = new CancellationTokenSource(EvaluationTimeout);
+                    var resolvedValue = CSharpScript.EvaluateAsync(exp, cancellationToken: cts.Token)


-                ConnectionString = "Data Source=" + targetDbServer +
-                                   ";Initial Catalog=master;Integrated Security=True;TrustServerCertificate=True"
+                var invalidMsg = $"Parameter username contains invalid value {username}";
+                _logger.LogError(invalidMsg);


+                    // fn:System.IO.File.ReadAllText("...") is refused rather than run.
+                    if (!SafeExpressionValidator.IsSafe(exp, out var reason))
+                    {
+                        _logger.LogWarning("Refusing to evaluate fn: expression '{Expression}': {Reason}", exp, reason);


+                    // fn:System.IO.File.ReadAllText("...") is refused rather than run.
+                    if (!SafeExpressionValidator.IsSafe(exp, out var reason))
+                    {
+                        _logger.LogWarning("Refusing to evaluate fn: expression '{Expression}': {Reason}", exp, reason);


+            foreach (var segment in segments)
+            {
+                if (segment == "..")
+                    return false;
+            }


+            foreach (var argument in invocation.ArgumentList.Arguments)
+            {
+                if (!IsSafeExpression(argument.Expression, out reason))
+                {
+                    return false;
+                }
+            }


+            catch (Exception ex)
+            {
+                caught = ex;
+            }


+                catch (Exception inner)
+                {
+                    throw new ApplicationException("outer failure", inner);
+                }


Copilot

Pull request overview

This PR adds draft remediation documentation (HLPS + Implementation Sequence) for the full-codebase review and simultaneously implements a first tranche of security/correctness hardening across the API, monitor/runner pipeline, core utilities, and web UI.

Changes:

Tightens authorization and reduces sensitive information disclosure in API endpoints and exception serialization.
Fixes multiple correctness/robustness issues in monitor/runner + terraform execution semantics, plus improved failure handling/logging.
Removes stored-XSS vectors in dorc-web renderers and adds regression tests; adds several new backend unit tests for the new behaviors.

Reviewed changes

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

Show a summary per file

File	Description
src/Dorc.TerraformRunner/TerraformProcessor.cs	Treats terraform exit codes correctly (plan `-detailed-exitcode` supports exit 2 as success).
src/Dorc.PowerShell/PowerShellScriptRunner.cs	Prevents logging/printing decrypted secret values on PS variable-set failures.
src/Dorc.PersistentData/Sources/EnvironmentsPersistentSource.cs	Stops swallowing attach/detach failures; logs and rethrows.
src/Dorc.PersistentData/SecurityObjectFilter.cs	Fixes allow/deny bitmask evaluation so deny is scoped to requested access.
src/Dorc.NetFramework.PowerShell/PowerShellScriptRunner.cs	Mirrors secret-safe logging behavior for .NET Framework runner.
src/Dorc.Monitor/MonitorConfiguration.cs	Ensures request-processing iteration delay never becomes 0 on bad/missing config.
src/Dorc.Monitor/DeploymentEngine.cs	Adds transient retry handling + circuit breaker/backoff for unexpected failures.
src/Dorc.Monitor/ComponentProcessor.cs	Fails closed on unknown component types (no longer “silent success”).
src/Dorc.Monitor/appsettings.json	Adds `requestProcessingIterationDelayMs` default config key.
src/Dorc.Monitor.Tests/MonitorConfigurationTests.cs	Unit tests for iteration delay defaulting/clamping behavior.
src/Dorc.Monitor.Tests/DeploymentEngineTransientTests.cs	Unit tests for transient exception classification logic.
src/Dorc.Monitor.Tests/ComponentProcessorFailClosedTests.cs	Regression test ensuring unknown component type marks failed.
src/Dorc.Core/VariableResolution/SafeExpressionValidator.cs	Adds allow-list validator for `fn:` expressions to prevent arbitrary code execution.
src/Dorc.Core/VariableResolution/PropertyExpressionEvaluator.cs	Enforces `fn:` validation + adds evaluation timeout to avoid runaway execution.
src/Dorc.Core/VariableResolution/PropertyEncryptor.cs	Removes silent random-key fallback; fails loudly when legacy encryptor is actually used.
src/Dorc.Core/SqlUserPasswordReset.cs	Tightens login-name allow-list + uses `SqlConnectionStringBuilder` to prevent conn-string injection.
src/Dorc.Core.Tests/TerraformExitCodeTests.cs	Tests terraform exit-code interpretation behavior.
src/Dorc.Core.Tests/SqlUserPasswordResetTests.cs	Tests SQL-login reset input validation and SQL string construction.
src/Dorc.Core.Tests/SecurityObjectFilterTests.cs	Tests corrected allow/deny logic and multi-flag semantics.
src/Dorc.Core.Tests/SafeExpressionValidatorTests.cs	Tests dangerous expression rejection + safe expression acceptance.
src/Dorc.Core.Tests/PropertyEncryptorTests.cs	Tests non-throwing construction + loud failure on use with invalid IV/key.
src/Dorc.Api/Services/ExceptionJsonConverter.cs	Stops leaking stack traces/inner exceptions/raw messages in HTTP serialization.
src/Dorc.Api/Services/DefaultExceptionHandler.cs	Keeps raw exception details server-side only (logs), returns safe client response.
src/Dorc.Api/Controllers/TerraformController.cs	Enforces plan view/confirm/decline authorization via environment modify privilege.
src/Dorc.Api/Controllers/RequestStatusesController.cs	Adds authz checks for log read/mutation + UNC-path validation; reduces 500-detail leakage.
src/Dorc.Api.Tests/ExceptionJsonConverterTests.cs	Tests that exception serialization does not leak sensitive details.
src/Dorc.Api.Tests/Controllers/TerraformControllerTests.cs	Tests TerraformController authorization behavior (fail closed on missing env).
src/Dorc.Api.Tests/Controllers/RequestStatusesControllerTests.cs	Tests new authz + UNC validation + IDOR protection on Patch/RawLog/GetLog.
src/dorc-web/tests/components/renderer-xss.test.ts	Regression test for XSS-safe rendering in combo/grid renderers.
src/dorc-web/src/pages/page-scripts-list.ts	Replaces `innerHTML` with Lit `render()` to prevent XSS.
src/dorc-web/src/pages/page-project-components.ts	Replaces `innerHTML` with Lit `render()` and uses Lit click binding.
src/dorc-web/src/pages/page-project-bundles.ts	Replaces `innerHTML` with Lit `render()` to prevent XSS.
src/dorc-web/src/pages/page-deploy.ts	Replaces `innerHTML` with Lit `render()` to prevent XSS.
src/dorc-web/src/components/edit-database-permissions.ts	Replaces `innerHTML` with Lit `render()` to prevent XSS.
src/dorc-web/src/components/attach-server.ts	Replaces `innerHTML` with Lit `render()` to prevent XSS.
src/dorc-web/src/components/attach-database.ts	Replaces `innerHTML` with Lit `render()` to prevent XSS.
src/dorc-web/src/components/add-user-or-group/utilities/addUserOrGroupTemplateHelper.ts	Uses Lit templating instead of string concatenation into `innerHTML`.
src/dorc-web/src/components/add-sql-port.ts	Replaces `innerHTML` with Lit `render()` to prevent XSS.
src/dorc-web/src/components/add-edit-database.ts	Replaces `innerHTML` with Lit `render()` to prevent XSS.
docs/codebase-review-remediation/IS-codebase-review-remediation.md	Adds the (draft) implementation sequence for codebase-review remediation.
docs/codebase-review-remediation/HLPS-codebase-review-remediation.md	Adds the (draft) high-level problem statement for remediation.

+        /// Returns true when the supplied login name contains only characters
+        /// that are safe to embed in a quoted identifier and string literal.
+        /// </summary>
+        public static bool IsValidLoginName(string username)


+            "CompareTo", "Equals"
+        };
+
+        public static bool IsSafe(string expression, out string reason)


            _log.LogDebug($"Calling Append to log method {log}");
            _requestsStatusPersistentSource.AppendLogToJob(deploymentResultId, log);
            _log.LogDebug($"Calling Append to log method {log}...Done");


+        /// server and share, and must not contain characters invalid in a path.
+        /// Prevents repointing a request's log at an arbitrary local path or URL.
+        /// </summary>
+        public static bool IsValidUncPath(string uncLogPath)


+# IS: Codebase Review Remediation — Implementation Sequence
+
+| Field       | Value                                            |
+|-------------|--------------------------------------------------|
+| **Status**  | REVISION (round-1 adversarial review applied)    |
+| **Author**  | Agent                                            |
+| **Date**    | 2026-07-05                                        |
+| **HLPS**    | HLPS-codebase-review-remediation.md (REVISION)   |


+                objCmd.ExecuteScalar();
                return new ApiBoolResult { Result = true };
            }
            catch (Exception e)


github-actions · 2026-07-06T06:07:01Z

Test Results

465 tests +87 465 ✅ +87 10s ⏱️ +3s
3 suites ± 0 0 💤 ± 0
3 files ± 0 0 ❌ ± 0

Results for commit 7c11a87. ± Comparison against base commit a537bdc.

This pull request removes 1 and adds 88 tests. Note that renamed tests count towards both.

Dorc.Core.Tests.PropertyEncryptorTests ‑ Constructor_HandlesFallbackOnException

Dorc.Api.Tests.Controllers.RequestStatusesControllerTests ‑ GetLog_UnknownRequest_ReturnsNotFound_AndDoesNotReadLog
Dorc.Api.Tests.Controllers.RequestStatusesControllerTests ‑ GetLog_WithModifyRights_ReturnsLog
Dorc.Api.Tests.Controllers.RequestStatusesControllerTests ‑ GetLog_WithoutModifyRights_ReturnsForbidden_AndDoesNotReadLog
Dorc.Api.Tests.Controllers.RequestStatusesControllerTests ‑ IsValidUncPath_ValidatesAsExpected ("",False)
Dorc.Api.Tests.Controllers.RequestStatusesControllerTests ‑ IsValidUncPath_ValidatesAsExpected ("C:\local\path",False)
Dorc.Api.Tests.Controllers.RequestStatusesControllerTests ‑ IsValidUncPath_ValidatesAsExpected ("\\server",False)
Dorc.Api.Tests.Controllers.RequestStatusesControllerTests ‑ IsValidUncPath_ValidatesAsExpected ("\\server\C$\Windows\Temp\x",False)
Dorc.Api.Tests.Controllers.RequestStatusesControllerTests ‑ IsValidUncPath_ValidatesAsExpected ("\\server\share",True)
Dorc.Api.Tests.Controllers.RequestStatusesControllerTests ‑ IsValidUncPath_ValidatesAsExpected ("\\server\share\..\..\..\other",False)
Dorc.Api.Tests.Controllers.RequestStatusesControllerTests ‑ IsValidUncPath_ValidatesAsExpected ("\\server\share\file.log",True)
…

claude added 21 commits July 5, 2026 22:48

chore: revert incidental package-lock.json churn from local npm install

8703c06

The dev-flag metadata changes were npm normalization unrelated to the XSS fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bUuv6qiuLSjHf3jnnaT9d

benhegartysefe requested a review from Copilot July 6, 2026 05:57

Copilot started reviewing on behalf of benhegartysefe July 6, 2026 05:58 View session

github-advanced-security AI found potential problems Jul 6, 2026

View reviewed changes

github-code-quality Bot found potential problems Jul 6, 2026

View reviewed changes

Copilot AI reviewed Jul 6, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

docs: add codebase-review remediation plan (HLPS + IS)#765

docs: add codebase-review remediation plan (HLPS + IS)#765
hegsie wants to merge 21 commits into
mainfrom
claude/codebase-review-25f9w7

hegsie commented Jul 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

hegsie commented Jul 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

github-actions Bot commented Jul 6, 2026

Test Results

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants