Skip to content

fix: Deep audit security, bug, and performance fixes across TreeWalker core#83

Open
InboraStudio wants to merge 1 commit into
microsoft:masterfrom
InboraStudio:master
Open

fix: Deep audit security, bug, and performance fixes across TreeWalker core#83
InboraStudio wants to merge 1 commit into
microsoft:masterfrom
InboraStudio:master

Conversation

@InboraStudio
Copy link
Copy Markdown

  • Security:

Added explicit TypeNameHandling.None in JSON deserialization to prevent type-injection attacks from modified DefaultSettings (TreeWalkerSession.cs)

  • Bug Fixes:

Replaced invalid Guid/CancellationToken == null checks with Guid.Empty validation since structs can’t be null (ActionContext, TreeNodeContext, TreeWalkerParameters)
Fixed CancellationTokenSource leak in ExecuteAction using proper using disposal for high-throughput retry cases (TreeWalkerSession.cs)
Implemented MissingResolver.Equals() and GetHashCode() correctly to prevent runtime crashes when used in collections (ExpressionExecutor.cs)

  • Performance:

Cached BaseAction.RunAction MethodInfo as static readonly to remove repeated reflection overhead (TreeWalkerSession.cs)
Replaced double serialize + parse (SerializeObject + JObject.Parse) with direct JObject.FromObject in schema validator (ForgeSchemaValidator.cs)

  • Code Quality:

Removed unused System.Threading.Tasks import (ActionDefinition.cs)
Fixed comment typos (timout → timeout, Attmpt → Attempt)

  • Build status:

Clean build with 0 warnings and 0 errors on both netstandard2.0 and net462.

…ker core

Comprehensive code audit of the Forge TreeWalker framework identifying
and remediating critical issues across 7 source files:

Security:
- Add explicit TypeNameHandling.None to JSON deserialization to prevent
  type-injection attacks via modified DefaultSettings (TreeWalkerSession.cs)

Bug Fixes:
- Replace dead Guid/CancellationToken null-checks with Guid.Empty validation;
  struct == null is always false in C# (ActionContext, TreeNodeContext,
  TreeWalkerParameters)
- Dispose CancellationTokenSource in ExecuteAction via using block to
  prevent OS handle leaks under high-throughput retry scenarios
  (TreeWalkerSession.cs)
- Implement MissingResolver.Equals/GetHashCode properly; previous
  NotImplementedException would crash at runtime if resolver was used
  in collections (ExpressionExecutor.cs)

Performance:
- Cache BaseAction.RunAction MethodInfo as static readonly to eliminate
  per-execution reflection overhead (TreeWalkerSession.cs)
- Replace double serialize-parse (SerializeObject + JObject.Parse) with
  direct JObject.FromObject in schema validator (ForgeSchemaValidator.cs)

Code Quality:
- Remove unused System.Threading.Tasks import (ActionDefinition.cs)
- Fix comment typos: timout -> timeout, Attmpt -> Attempt
  (TreeWalkerSession.cs)

Build verified: 0 warnings, 0 errors on both netstandard2.0 and net462.
Copilot AI review requested due to automatic review settings May 13, 2026 16:51
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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants