Skip to content

Add Microsoft Defender for AI real-time protection on agent-hooks - #266

Open
Krishnadheeraj (DheerajPannala) wants to merge 16 commits into
mainfrom
kpannala-microsoft-defender-rtp-agent-hooks
Open

Krishnadheeraj (DheerajPannala) wants to merge 16 commits into
mainfrom
kpannala-microsoft-defender-rtp-agent-hooks

Conversation

@DheerajPannala

@DheerajPannala Krishnadheeraj (DheerajPannala) commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds opt-in real-time protection from Microsoft Defender for AI ("Defender RTP") to the Agent 365 .NET SDK, on the Responsible AI agent-hooks control contract (AGENT-HOOKS-0.1). The host emits an agent-hooks context at each interception point, an interceptor posts it to Defender's prevention endpoint, and Defender's verdict decides allow or deny.

This is the .NET part of a three-SDK change; the Node.js and Python parts are microsoft/Agent365-nodejs#310 and microsoft/Agent365-python#278.

  • Microsoft.Agents.A365.Tooling: new namespace Microsoft.Agents.A365.Tooling.Protection.Defender
    • DefenderRtpClient: EvaluateHookContextAsync sends an agent-hooks/0.1 context to POST .../v1/protection/evaluate and returns a DefenderRtpEvaluationResult; PrefetchAccessTokenAsync warms the token cache.
    • DefenderRtpOptions (environment-driven configuration), DefenderRtpAgentContext, DefenderRtpTokenResolver, and DefenderRtpTokenResolvers.FromAgenticConnection (agent identity token).
  • Microsoft.Agents.A365.Tooling.Extensions.AgentHooks (new package, preview): A365DefenderInterceptor, an IInterceptor for ResponsibleAI.AgentHooks 0.1.0-beta.1, plus A365AgentHooks.CreateProtectionEmitter (enforce mode, parallel/strictest) and AddA365Defender.
  • New test project Microsoft.Agents.A365.Tooling.Extensions.AgentHooks.Tests; both new projects are added to the solution.
  • Docs: package README, CHANGELOG ([Unreleased] → Added), docs/design.md, and the Tooling design doc.

Scope: Defender RTP and the agent-hooks interceptor only. No Purview, no samples.

How it works

Interception points. Defender evaluates four points; any other point (agent_startup, model calls, agent_shutdown) is allowed locally with no call.

Point When On deny
input the user's message, before the agent runs the agent does not run
pre_tool_call a tool call, before it runs the tool does not run
post_tool_call a tool result, before the agent uses it the result is withheld
output the reply, before it is sent the reply is replaced

Request. Defender receives a copy of the host's agent-hooks AgentContext, built to meet its request validation; the host's context is not modified.

  • The copy keeps the session, sequence and tool-call ids.
  • spec = agent-hooks/0.1, timestamp in UTC, sequence ≥ 0. When the host omits sequence, the client numbers it per session; a session it stopped tracking resumes above every sequence it handed out, so a session's sequence never repeats or goes back.
  • agent.id is the Entra agent identity, agent.framework is sanitized to ^[a-z0-9_-]+$, and session.id is required.
  • The envelope (session, agent, tenant, actor, model, trace, request_id) is rebuilt from its spec fields only, so nothing a host adds to an envelope object escapes the content budget.
  • target equals the point's field, and tool_call/tool_result carry only spec members.
  • tenant.id is always the agent's tenant, since it must equal the token's tid.
  • Optional members of another shape, such as a string model or a non-object extension, are repaired or dropped instead of failing the evaluation.

Content size. Each content string is clamped to MaxContentCharacters (default 20000), and the copy carries at most four times that in all. Names, keys and nulls count toward it too, so a context padded with many empty or null items stays bounded, and content nested more than 32 levels deep is cut the same way. Ids, names and roles are not truncated.

  • The content under decision (the point's field, mirrored in target) is fitted first. At pre_tool_call and post_tool_call, the called tool's declaration comes next. A name-only scan of the first 10,000 declarations finds it; its name is sent whole and its description and schema use the budget. Without any declarations, the a365 extension's description is used. Then come the post_tool_call arguments, the other tool declarations in the host's order, the message history (newest first, so the oldest messages are dropped first), extensions and host-added members. Only as many tool declarations, messages and extension namespaces as the budget can hold are read, so the work is bounded too.
  • If the content under decision had to be cut, Defender saw only part of what its verdict would authorize, so the result is marked Truncated. At a tool point the same applies when the called tool's description or schema had to be cut, or the list is longer than 10,000 and the called tool wasn't among the first 10,000. A tool absent from a list scanned to the end is declared from its name and isn't a cut. Error names the cause: TruncatedContentError, TruncatedToolDeclarationError or UnseenToolDeclarationError. A deny stands, and so does a transform, which blocks. An allow isn't authoritative and follows the fail mode, so padding can't push a payload, or the tool's real declaration, past what Defender inspects.
  • Lone UTF-16 surrogates in any string or property name are replaced with U+FFFD, since Defender can't parse them. A cut never splits a surrogate pair, and a renamed key that would collide with another gets a numbered suffix, so no value is dropped.

Correlation. Every call sends a unique x-ms-correlation-id (GUID). Defender echoes it and logs the evaluation under it; it is returned as DefenderRtpEvaluationResult.CorrelationId.

Verdict.

  • allow proceeds, passing Defender's warnings and result_labels through to the agent-hooks verdict. A warning reason in the host_error: namespace, which agent-hooks reserves for host failures, is reported as defender:warning, so it can't invalidate the verdict.
  • deny blocks with Defender's message, and the agent-hooks verdict carries a correlation evidence pointer.
  • transform also blocks, because this version doesn't apply rewrites.

Authentication. Defender is always called app-only as the agent identity. That covers user turns, autonomous runs, A2A and startup prefetch, where there's no user token.

  1. The Agents SDK connection returns the agent identity's assertion via IAgenticTokenProvider.GetAgenticApplicationTokenAsync(tenantId, agentAppInstanceId). This is the same authority Observability S2S export uses.
  2. FromAgenticConnection exchanges it at https://login.microsoftonline.com/{tenantId}/oauth2/v2.0/token (client_credentials, client_id = agent identity, jwt-bearer client assertion) for api://86a21212-634e-4553-b3d6-e477e4c9d9ec/.default. The token carries the app role RealtimeProtection.Evaluate.All.
  3. Tokens are cached per tenant, agent and scope (kept as separate parts of the key), at most 100 at a time:
    • Within 5 minutes of expiry, the token is refreshed in the background. If the refresh fails, the cached token is kept until it actually expires.
    • One acquisition per key runs at a time. The timeout bounds the resolver's task itself, so a resolver that ignores cancellation can't hold it. It leaves the in-flight map as it completes, so a cancelled caller can't strand it and a failed acquisition is never reused.

The endpoint and the token authority must be absolute HTTPS URLs. Redirects are never followed: a 307 or 308 would replay the agent's context, or the client assertion, to another host. The built-in client doesn't follow them; an injected HttpClient must not either, and a request it redirects anyway is treated as a failure.

Deadline and fail mode. One deadline, Timeout, covers token acquisition and the request. A timeout, token failure, transport exception (including a resilience handler's, such as an open circuit) or non-2xx response marks the result not evaluated (Evaluated = false, with Error and HttpStatus).

  • Fail open (the default) allows the action with an agent-hooks defender:unverified warning.
  • A365_DEFENDER_RTP_FAIL_MODE=closed denies with runtime_error:defender_unverified, which is never reported as a detection.
  • A 400 surfaces the failed rules from Defender's diagnostics.validationErrors in Error.
  • In the interceptor, the per-call factory (A365DefenderCall) isn't invoked when Defender is disabled or the point isn't evaluated. If it resolves no agent identity (returns null) or throws, the result follows the fail mode. The result and verdict record only the exception's type, never its message; the exception goes to the optional ILogger.
  • The onEvaluated callback sees every result and never changes the verdict. It runs on the thread pool once the verdict is decided, outside the emitter's interceptor timeout, so a slow callback can't turn the verdict into a host error. The verdict doesn't share data with the result, Warnings and ResultLabels are read-only, and an exception the callback throws is logged (optional ILogger) and ignored.

Configuration

Variable Meaning
ENABLE_A365_DEFENDER_RTP true to call Defender; false or unset leaves it off (1/0, yes/no and on/off also work). Any other value fails startup, so a typo can't quietly turn protection off
A365_DEFENDER_RTP_ENDPOINT the prevention endpoint, an absolute HTTPS URL: https://<host>/v1/protection/evaluate (required when enabled)
A365_DEFENDER_RTP_FAIL_MODE open (default) or closed, which blocks when no verdict is obtained. Any other value fails startup, so a typo can't quietly select fail-open
A365_DEFENDER_RTP_TIMEOUT_MILLISECONDS the deadline for one evaluation, including token acquisition (default 10000)
A365_DEFENDER_RTP_AUTHENTICATION_SCOPE overrides the Defender API scope
A365_DEFENDER_RTP_MAX_CONTENT_CHARACTERS clamps each content string sent (default 20000); a request carries at most four times this in all. Raise it for agents that handle long content

Prerequisites.

  • The agent blueprint has RealtimeProtection.Evaluate.All on the Defender API (86a21212-634e-4553-b3d6-e477e4c9d9ec) as an application permission with admin consent. Add Defender permissions part of "a365 setup all" Agent365-devTools#485 adds it to a365 setup all; until that ships, the package README documents the manual grant.
  • The agent's tenant is onboarded to Microsoft Defender for AI.

Without these, Defender answers 403, which follows the fail mode.

Emitter timeout. The emitter's per-interceptor timeout must be longer than Timeout; otherwise agent-hooks records a fail-closed host error before the client can apply its fail mode. A365AgentHooks.CreateProtectionEmitter uses Timeout + 2 s. If you register the interceptor on another emitter (agent-hooks defaults to 5 s), raise that emitter's timeout or lower Timeout.

var defender = new DefenderRtpClient(DefenderRtpOptions.FromEnvironment(), httpClient);
var tokens = DefenderRtpTokenResolvers.FromAgenticConnection(
    (IAgenticTokenProvider)connections.GetDefaultConnection(), httpClient);

var emitter = A365AgentHooks.CreateProtectionEmitter(defender: defender.Options)
    .AddA365Defender(new A365DefenderInterceptor(
        defender,
        context => new A365DefenderCall(
            new DefenderRtpAgentContext
            {
                AgentId = turnContext.Activity.Recipient.AgenticAppId, // the agent identity
                TenantId = turnContext.Activity.Recipient.TenantId,     // the agent's tenant
            },
            tokens)));

Testing

  • dotnet build (Release): 0 warnings, 0 errors.
  • dotnet test: all 13 test assemblies pass. The Observability integration tests skip 9 cases that need Azure OpenAI settings.
  • dotnet pack: succeeds, and the AgentHooks package depends on ResponsibleAI.AgentHooks 0.1.0-beta.1.
  • 99 DefenderRtpClientTests, each request checked against the endpoint's request validation rules. They cover:
    • forwarding, the correlation id and the agent identity;
    • context fitting and clamping, including the envelope rebuilt from its spec fields and optional members or verdict metadata of another shape;
    • the total budget: the content under decision first, the oldest history dropped first, a context padded with tens of thousands of empty or null items staying bounded, and content nested too deeply cut without deep recursion;
    • the called tool's declaration: copied first past a padded list; marked truncated, with its own error, when it is cut (including the a365 description used without declarations) or when the list outruns the scan (10,000 entries not truncated, 10,001 truncated); declared from its name when absent;
    • truncation authority: a deny on a truncated copy stands, and an allow follows the fail mode;
    • lone surrogates replaced without dropping a value, even when many keys collide on one name, and surrogate pairs never split;
    • deny, allow with warnings, transform-as-block, and read-only verdict lists;
    • fail open and closed on HTTP errors, transport exceptions and token failures;
    • 400 validation rules and timeouts;
    • one shared deadline for the token and the request;
    • HTTPS-only endpoints, redirected requests treated as failures, and enable and fail-mode settings that reject unknown values;
    • token caching, prefetch, background refresh with fallback to a still-valid token, a cache that stays bounded under concurrent acquisitions, a resolver that ignores the deadline, and in-flight acquisitions that are never reused after cancellation, failure or expiry;
    • sequences that keep increasing for a session the client stopped tracking.
  • 17 DefenderRtpTokenResolversTests, using a handler-based fake token endpoint. They cover:
    • the token URL and a custom authority, and the exact form fields;
    • assertion handling;
    • non-2xx responses without echoing the body, malformed JSON, a non-object response and a missing access_token;
    • an HTTPS-only authority, redirects (never followed by the shared client, and failing when an injected client follows them), and cancellation.
  • 26 A365DefenderInterceptorTests, run under the real native agent-hooks emitter. They cover:
    • verdict mapping (including a warning reason in the reserved host_error: namespace), fail open and fail closed, and points Defender doesn't evaluate;
    • no factory call when Defender is disabled or the point isn't evaluated;
    • a factory that throws or resolves no agent identity, and an allow of truncated content, each following the fail mode, with only the exception's type recorded;
    • the fail mode applying before the emitter's interceptor timeout;
    • an evaluation callback that throws, tries to change the result or blocks, leaving the verdict unchanged.
  • ResponsibleAI.AgentHooks 0.1.0-alpha.5 → 0.1.0-beta.1 doesn't change the agent-hooks/0.1 wire contract; the .NET wrapper only adds InterceptionEmitter.RecordHostFailure.
  • Verified live against the Defender prevention endpoint, fail-closed, authenticating as an agent identity through FromAgenticConnection:
    • clean traffic was allowed at all four points;
    • a known-malicious URL was denied at pre_tool_call (defender:block:prevention_blocked), and the tool did not run.

Known limitations and follow-ups

  • transform is treated as a block. This version doesn't apply Defender rewrites (TransformPath is surfaced on the verdict).
  • Long content isn't evaluated in full. Content under decision beyond MaxContentCharacters, or beyond its share of the request's total budget, follows the fail mode unless Defender denies the part it saw. Raise A365_DEFENDER_RTP_MAX_CONTENT_CHARACTERS for agents that handle long content; evaluating long content in chunks is a follow-up.
  • Prerelease dependency. ResponsibleAI.AgentHooks is still prerelease (0.1.0-beta.1), so NU5104 is suppressed for the new package. Its native core ships for linux-x64, win-x64, osx-x64 and osx-arm64 only.
  • Not yet wired into the hosting pipeline. Hosts register the interceptor on their own agent-hooks emitter. Wiring into the Agents SDK / Agent Framework hosting pipeline is a follow-up; for Agent Framework, Microsoft.Agents.AI.AgentHooks can register the same interceptor.
  • The Defender permission needs a manual grant for now. Until Add Defender permissions part of "a365 setup all" Agent365-devTools#485 ships, grant it as the README describes.

Related work

Copilot AI balanced review requested due to automatic review settings October 8, 2026 11:38
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown

⚠️ Deprecation Warning: The deny-licenses option is deprecated for possible removal in the next major release. For more information, see issue 997.

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails
nuget/FluentAssertions >= 0 UnknownUnknown
nuget/Microsoft.NET.Test.Sdk >= 0 UnknownUnknown
nuget/coverlet.collector >= 0 UnknownUnknown
nuget/xunit >= 0 UnknownUnknown
nuget/xunit.runner.visualstudio >= 0 UnknownUnknown
nuget/Microsoft.SourceLink.GitHub >= 0 UnknownUnknown
nuget/ResponsibleAI.AgentHooks >= 0 UnknownUnknown

Scanned Files

  • src/Tests/Microsoft.Agents.A365.Tooling.Extensions.AgentHooks.Tests/Microsoft.Agents.A365.Tooling.Extensions.AgentHooks.Tests.csproj
  • src/Tooling/Extensions/AgentHooks/Microsoft.Agents.A365.Tooling.Extensions.AgentHooks.csproj

Tooling (Microsoft.Agents.A365.Tooling):
- DefenderRtpClient sends agent-hooks/0.1 contexts to the Defender prevention
  endpoint (POST .../v1/protection/evaluate) at the four points Defender
  evaluates (input, pre_tool_call, post_tool_call, output) and returns its
  verdict; deny and transform block. Contexts are fitted to Defender's
  request validation (target equals the point field, spec-only tool_call and
  tool_result members, UTC timestamp, sanitized framework and extensions)
  without modifying the host's context.
- Calls carry the agent identity's app-only token for the Defender API
  (RealtimeProtection.Evaluate.All), resolved by a DefenderRtpTokenResolver
  and cached per agent and tenant. FromAgenticConnection uses the agent's
  IAgenticTokenProvider connection, the authority Observability S2S uses.
- A unique x-ms-correlation-id per call. Failures follow
  A365_DEFENDER_RTP_FAIL_MODE; a 400 reports the failed validation rule.

New preview package Microsoft.Agents.A365.Tooling.Extensions.AgentHooks:
- A365DefenderInterceptor, an IInterceptor for ResponsibleAI.AgentHooks
  0.1.0-beta.1, and CreateProtectionEmitter (parallel/strictest).

Docs: package README (`a365 setup all` grants the Defender role through
microsoft/Agent365-devTools#485; the agent's tenant must be onboarded to
Microsoft Defender for AI, otherwise 403, which follows the fail mode),
CHANGELOG, docs/design.md and the Tooling design doc.

Tests: 25 client tests, checked against the endpoint's request validation
rules, and 8 interceptor tests under the real native emitter.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 21c5fda0-eb1d-4d28-8fe0-f54c61e0fe61
Copilot-Session: fdebe82a-4ddd-45f5-9970-ee89289ca92b
@DheerajPannala
Krishnadheeraj (DheerajPannala) force-pushed the kpannala-microsoft-defender-rtp-agent-hooks branch from 31964d5 to 31289b9 Compare October 8, 2026 11:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Transport security, timeout handling, and token-acquisition cleanup issues must be resolved.

9 open findings
What changed in this PR

Adds opt-in Microsoft Defender for AI real-time protection through the agent-hooks contract.

Changes:

  • Adds Defender evaluation, authentication, caching, configuration, and verdict handling.
  • Introduces the AgentHooks interceptor extension package.
  • Adds tests, package registration, and documentation.
File Description
src/​Tooling/​Extensions/​AgentHooks/​README.md Documents setup and usage.
src/​Tooling/​Extensions/​AgentHooks/​Microsoft.Agents.A365.Tooling.Extensions.AgentHooks.csproj Defines the new package.
src/​Tooling/​Extensions/​AgentHooks/​A365DefenderInterceptor.cs Integrates Defender with agent-hooks.
src/​Tooling/​Core/​Protection/​Defender/​DefenderRtpTokenResolvers.cs Implements agent token exchange.
src/​Tooling/​Core/​Protection/​Defender/​DefenderRtpTokenResolver.cs Defines identity and token contracts.
src/​Tooling/​Core/​Protection/​Defender/​DefenderRtpOptions.cs Adds environment-driven configuration.
src/​Tooling/​Core/​Protection/​Defender/​DefenderRtpEvaluationResult.cs Defines evaluation results.
src/​Tooling/​Core/​Protection/​Defender/​DefenderRtpClient.cs Implements request preparation, transport, and caching.
src/​Tooling/​Core/​docs/​design.md Documents Defender architecture.
src/​Tests/​Microsoft.Agents.A365.Tooling.Extensions.AgentHooks.Tests/​Microsoft.Agents.A365.Tooling.Extensions.AgentHooks.Tests.csproj Defines interceptor tests.
src/​Tests/​Microsoft.Agents.A365.Tooling.Extensions.AgentHooks.Tests/​A365DefenderInterceptorTests.cs Tests interceptor behavior.
src/​Tests/​Microsoft.Agents.A365.Tooling.Core.Tests/​Protection/​DefenderRtpClientTests.cs Tests client behavior.
src/​Microsoft.Agents.A365.Sdk.sln Registers new projects.
src/​Directory.Packages.props Pins the agent-hooks dependency.
docs/​design.md Updates repository architecture documentation.
CHANGELOG.md Records the new feature.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Copilot AI balanced review requested due to automatic review settings October 8, 2026 11:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- One deadline (DefenderRtpOptions.Timeout) now covers token acquisition and
  the request, so the fail mode applies before an agent-hooks interceptor
  timeout; the README explains how to size the emitter's timeout.
- Token acquisitions leave the in-flight map as they complete, so a cancelled
  caller cannot strand one and a failed or stale acquisition is never reused.
  Within five minutes of expiry the cached token is refreshed in the
  background and kept until it actually expires if the refresh fails.
- Any transport exception (for example from a resilience handler) is a
  not-evaluated result that follows the fail mode.
- The endpoint and the token authority must be absolute HTTPS URLs.
- MaxContentCharacters applies to every content string (messages, tool
  declarations, extensions and host-added members); ids, names and roles are
  not truncated.
- FromAgenticConnection reports a malformed token response clearly; new tests
  cover its URL, form fields, assertion, errors and cancellation.
- Docs: Defender receives a fitted copy of the context; until
  microsoft/Agent365-devTools#485 ships, grant RealtimeProtection.Evaluate.All
  manually.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fdebe82a-4ddd-45f5-9970-ee89289ca92b
Copilot AI balanced review requested due to automatic review settings October 8, 2026 12:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The onEvaluated callback is for logging and telemetry. An exception it threw
escaped InterceptAsync, and agent-hooks turned it into a fail-closed host
error even in fail-open mode. It is now caught, logged to an optional
ILogger, and never changes the verdict, matching the Node.js and Python SDKs.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fdebe82a-4ddd-45f5-9970-ee89289ca92b
Copilot AI balanced review requested due to automatic review settings October 8, 2026 12:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Payload bounds, endpoint validation, malformed-response handling, and callback timeout behavior have unresolved correctness and security issues.

6 open findings
1 resolved since last review
Previously missed (2)

In code that hasn't changed since last review

Medium severity Handle non-object diagnostics without escaping evaluation

src/​Tooling/​Core/​Protection/​Defender/​DefenderRtpClient.cs:729

A non-object diagnostics value (for example "diagnostics": [] or a serialized "[]") makes the string indexer throw here. That lets an unexpected non-2xx response escape EvaluateHookContextAsync instead of producing the documented not-evaluated result and applying the fail mode. Type-check the parsed node before reading validationErrors, then fall back to the normal error detail fields.

Medium severity Prevent telemetry callback from causing interceptor timeouts

src/​Tooling/​Extensions/​AgentHooks/​A365DefenderInterceptor.cs:106

The telemetry callback runs synchronously inside the interceptor timeout. If it blocks long enough—especially after an evaluation consumed most of the Defender deadline—the emitter returns host_error:interceptor_timeout, so an allow or fail-open result becomes a deny despite the callback's documented guarantee that it never changes the verdict. Move telemetry off the verdict-critical path or explicitly enforce a non-blocking callback contract.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

- The interceptor resolves the per-call identity only when Defender RTP is
  enabled and Defender evaluates the point, and a failure to resolve it
  follows the fail mode instead of becoming a fail-closed host error.
- tenant.id is always the agent's tenant, which the token is acquired for.
- Truncated content stays within MaxContentCharacters, marker included.
- trace is an envelope member and is not clamped.
- A shared HttpClient replaces the undisposed fallback client.
- README: the blueprint-level manual grant of RealtimeProtection.Evaluate.All
  (inheritable by the blueprint's agent identities) until
  microsoft/Agent365-devTools#485 ships; the usage snippet imports
  Microsoft.Agents.Authentication.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fdebe82a-4ddd-45f5-9970-ee89289ca92b
Copilot AI balanced review requested due to automatic review settings October 8, 2026 12:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Endpoint mutation can bypass HTTPS validation, and additional parsing, Unicode, and cache-concurrency defects remain.

2 open findings
6 resolved since last review
Previously missed (2)

In code that hasn't changed since last review

Medium severity Handle non-object diagnostics JSON without throwing

src/​Tooling/​Core/​Protection/​Defender/​DefenderRtpClient.cs:725

A valid JSON diagnostics value is not necessarily an object. If Defender returns an array or scalar here, the string indexer throws InvalidOperationException, so a non-2xx response escapes instead of producing the documented not-evaluated result with its HTTP status. Cast to JsonObject before reading validationErrors and fall back to the normal detail fields otherwise.

Medium severity Synchronize cache insertion and enforce the maximum size

src/​Tooling/​Core/​Protection/​Defender/​DefenderRtpClient.cs:828

The capacity check happens before insertion and is not coordinated across acquisitions. If many distinct tenants finish acquisition concurrently while the cache is below 100, they can all skip eviction and then insert; the oversized cache remains oversized because later calls evict only one entry before adding one. Insert and then trim in a loop, or synchronize the prune/add section, so MaxCachedTokens is an actual retained-memory bound.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Identity attribution, callback timeout behavior, and non-cooperative token acquisition can undermine protection guarantees.

2 open findings
3 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Host-supplied agent identity can override authenticated Entra identity

src/​Tooling/​Core/​Protection/​Defender/​DefenderRtpClient.cs:373

The authenticated identity can be replaced by the host envelope here: agent-hooks only requires agent.id to be a non-empty host identifier, so a generic host may provide a logical or spoofed value unrelated to the Entra identity whose token is acquired from agent.AgentId. This contradicts the stated invariant that Defender receives the Entra agent identity and can misattribute the evaluation. Build this field only from DefenderRtpAgentContext (AgentObjectId, then AgentId).

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

…safe

- Bound the token resolver's task by the timeout as well as asking it to
  stop, and observe a task left behind, so a resolver that ignores
  cancellation cannot hold the identity's acquisition, and every later
  evaluation, forever.
- Run the evaluation callback on the thread pool once the verdict is
  decided, outside the emitter's interceptor timeout, so a slow callback
  cannot turn the verdict into a fail-closed host error.
- Give a session the client stops tracking a sequence above every
  sequence it handed out for a dropped session, so a session that comes
  back never repeats or goes back.
- Read only as many tool declarations and extension namespaces as the
  budget can hold, charging skipped entries too; a called tool beyond
  them is declared from its name.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fdebe82a-4ddd-45f5-9970-ee89289ca92b
Copilot AI balanced review requested due to automatic review settings October 8, 2026 15:02
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fdebe82a-4ddd-45f5-9970-ee89289ca92b

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Authority URI handling and called-tool metadata prioritization need correction before approval.

1 open finding
2 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Reject authority query or fragment before appending token path

src/​Tooling/​Core/​Protection/​Defender/​DefenderRtpTokenResolvers.cs:52

Validation accepts an authority containing a query or fragment, but the token path is later appended to AbsoluteUri as text. For example, https://login.example.test?x=1 turns the tenant/token path into part of the query and sends the request to /; a fragment similarly hides the appended path. Reject these components (or construct the URI by components) so every accepted authority produces the documented token endpoint.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Copilot AI balanced review requested due to automatic review settings October 8, 2026 15:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Automatic redirects can replay protected content or client assertions to another host.

3 open findings
Previously missed (1)

In code that hasn't changed since last review

Low severity Specify the exact Defender permission setup command

src/​Tooling/​Extensions/​AgentHooks/​README.md:36

The linked DevTools change grants this permission only through a365 setup all, not the generic a365 setup wording used here. Name the exact command so users do not assume another setup path provisions Defender access.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

At a tool point, a name-only scan of the first 10,000 tool declarations
finds the called tool's declaration, which is copied first, ahead of the
rest of the context. If the scan stops before finding it, or its
description or schema had to be cut, the result is marked Truncated, so
Defender's allow follows the fail mode: it did not see the full
declaration of the tool under decision. A tool absent from a list scanned
to the end is declared from its name and is not a cut. The other
declarations then fill the remaining budget in the host's order.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fdebe82a-4ddd-45f5-9970-ee89289ca92b
Copilot AI balanced review requested due to automatic review settings October 8, 2026 15:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Token-cache key collisions and quadratic surrogate-key handling create unresolved security risks.

3 open findings
1 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Key collision handling causes quadratic work and budget overrun

src/​Tooling/​Core/​Protection/​Defender/​DefenderRtpClient.cs:1292

Collision resolution restarts at suffix 2 for every renamed key. An input can contain many distinct lone-surrogate property names that all normalize to the same name, making the nth key probe every prior suffix (quadratic CPU work before Defender is called). The generated suffixes are also added after ContentBudget accounting, so they can exceed the documented total-size cap. Sanitize keys during the budgeted copy and keep a per-base next-suffix counter.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

…arts

- The shared HTTP client no longer follows redirects: a 307 or 308 would
  replay the agent's context, or the token exchange's client assertion,
  to another host. A 3xx is a failure that follows the fail mode, and a
  request an injected client redirected anyway is treated as a failure.
- Key cached and in-flight tokens by tenant, agent and scope as separate
  parts, so identities whose ids would join alike never share a token.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fdebe82a-4ddd-45f5-9970-ee89289ca92b
Copilot AI balanced review requested due to automatic review settings October 8, 2026 15:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unsupported ConcurrentDictionary overloads block the net8.0 build, and collision normalization can violate CPU and request-size bounds.

1 open finding
3 resolved since last review
Previously missed (2)

In code that hasn't changed since last review

Medium severity Prevent quadratic key normalization and request budget overruns

src/​Tooling/​Core/​Protection/​Defender/​DefenderRtpClient.cs:1312

Distinct malformed keys can all normalize to the same name. Restarting the suffix search at 2 for each key makes this loop quadratic, and every added ~N suffix is created after Clamp has spent the budget, so a context with many collisions can both consume excessive CPU and exceed the promised total request bound. Normalize keys while copying/clamping, track the next suffix per normalized base name, and charge the final key length to the budget.

Medium severity Reject authorities containing query or fragment components

src/​Tooling/​Core/​Protection/​Defender/​DefenderRtpTokenResolvers.cs:54

An authority with a query or fragment passes this validation, but the string concatenation below appends the tenant and token path inside that query/fragment (for example, https://host/#x sends the assertion to https://host/). Reject these components so every accepted authority produces the documented token endpoint.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

- The called tool's name is its identity: it is copied whole and not
  charged; its description and schema use the budget.
- The other declarations skip only the called tool's entry.
- The a365 extension's description declares the called tool only when
  there are no tool declarations at all; a cut there is a truncation.
- Each truncation cause has its own Error: TruncatedContentError,
  TruncatedToolDeclarationError (the declaration was cut) and
  UnseenToolDeclarationError (the list is longer than 10,000 and the
  called tool was not among the first 10,000).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fdebe82a-4ddd-45f5-9970-ee89289ca92b
Copilot AI balanced review requested due to automatic review settings October 8, 2026 15:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Redirect detection currently fails with standard auto-redirecting HTTP clients, potentially accepting responses after sensitive requests are replayed to another host.

1 open finding
1 resolved since last review
Previously missed (2)

In code that hasn't changed since last review

Medium severity Evict cache entries only for new keys

src/​Tooling/​Core/​Protection/​Defender/​DefenderRtpClient.cs:1134

At capacity this also runs when refreshing an existing cache key. Because assigning that key does not add an entry, the eviction unnecessarily drops another identity's still-valid token and causes an avoidable acquisition later. Only evict when the incoming key is new.

Medium severity Bound collision suffixes within the content budget

src/​Tooling/​Core/​Protection/​Defender/​DefenderRtpClient.cs:1344

Collision suffixes are appended after ContentBudget has already charged the original key. Many distinct lone-surrogate keys can normalize to the same name and each gain an uncharged ~N suffix, allowing the serialized request to exceed the documented four-times content cap. Account for the normalized unique key before finalizing the fitted copy, or use a bounded collision scheme.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

- ENABLE_A365_DEFENDER_RTP accepts true or false (and 1/0, yes/no,
  on/off); any other value fails startup instead of quietly leaving
  protection off, as FromEnvironment documents.
- Keys renamed after replacing lone surrogates remember their next
  suffix per name, so many keys colliding on one name cost linear work
  rather than quadratic.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fdebe82a-4ddd-45f5-9970-ee89289ca92b
Copilot AI balanced review requested due to automatic review settings October 8, 2026 16:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Truncated blocking results lose diagnostic errors, and mixed supplied/generated sequences can regress within a session.

0 open findings

1 resolved since last review
Previously missed (2)

In code that hasn't changed since last review

Medium severity Record host-provided sequences to prevent sequence regression

src/​Tooling/​Core/​Protection/​Defender/​DefenderRtpClient.cs:436

A valid host-provided sequence is copied without updating _sequences. If the next context for that session omits sequence, NextSequence starts from the cache floor (typically 1), so a session can emit 100 followed by 1, violating the agent-hooks requirement that sequence be strictly increasing within a session and the PR's no-repeat/no-regression guarantee. Record accepted host sequences in the same atomic counter used by NextSequence before generating later values.

Medium severity Preserve truncation reason in Defender evaluation results

src/​Tooling/​Core/​Protection/​Defender/​DefenderRtpClient.cs:863

A truncated deny or transform reaches this branch with Truncated = true, but the truncation reason is discarded. This contradicts DefenderRtpEvaluationResult's contract that Error identifies TruncatedContentError, TruncatedToolDeclarationError, or UnseenToolDeclarationError, leaving callbacks unable to diagnose what Defender did not inspect. Preserve truncation here as the allow branch does.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

@slreznit

Copy link
Copy Markdown

[P2] Keep typed optional fields valid when fitting the content budget

Location: DefenderRtpClient.cs, lines 403-407.

response and summary are recognized agent-hooks contract fields, but they fall through this generic content-clamping path. When little budget remains after copying history, clamping can remove required members or truncate enum values. Defender still deserializes those optional fields and rejects the whole request before evaluating the content under decision.

Reproduced with the default MaxContentCharacters = 20000: an input containing "hello", four user history messages of 19,974 characters each, and a valid prior model response is fitted to response: {"content":"prior"}, dropping the required tool_calls and finish_reason. This produces HTTP 400 even though the input itself fits. Fail-open then allows without inspection, while fail-closed blocks unnecessarily. A partially fitted summary.reason can likewise become an invalid enum value.

Please fit these typed optional fields schema-aware, or omit the entire nonessential field when it cannot remain valid, and add regression coverage for the required-member and enum constraints.

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.

4 participants