From fa85936d757613305aa796a46b7b0698f5082fdd Mon Sep 17 00:00:00 2001 From: Konstantin Tyukalov Date: Wed, 10 Apr 2024 19:27:17 +0400 Subject: [PATCH 1/3] Add ShellTasksValidation audit action data to issues --- .../Handlers/ProcessHandler/ProcessHandlerHelper.cs | 9 ++++++++- .../Handlers/ProcessHandler/ProcessHandlerV2.cs | 9 ++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/Agent.Worker/Handlers/ProcessHandler/ProcessHandlerHelper.cs b/src/Agent.Worker/Handlers/ProcessHandler/ProcessHandlerHelper.cs index 3315f45c39..fed6e68f4a 100644 --- a/src/Agent.Worker/Handlers/ProcessHandler/ProcessHandlerHelper.cs +++ b/src/Agent.Worker/Handlers/ProcessHandler/ProcessHandlerHelper.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using Agent.Sdk.Knob; +using Microsoft.TeamFoundation.DistributedTask.WebApi; using Microsoft.VisualStudio.Services.Agent.Util; using Microsoft.VisualStudio.Services.Common; @@ -132,7 +133,13 @@ public static (bool, Dictionary) ValidateInputArguments( { if (enableAudit && !enableValidation) { - context.Warning(StringUtil.Loc("ProcessHandlerInvalidScriptArgs")); + var issue = new Issue + { + Type = IssueType.Warning, + Message = StringUtil.Loc("ProcessHandlerInvalidScriptArgs"), + }; + issue.Data.Add("auditAction", "1"); // ShellTasksValidation = 1 + context.AddIssue(issue); } if (enableValidation) { diff --git a/src/Agent.Worker/Handlers/ProcessHandler/ProcessHandlerV2.cs b/src/Agent.Worker/Handlers/ProcessHandler/ProcessHandlerV2.cs index cc317c6c6e..1ab60f7aee 100644 --- a/src/Agent.Worker/Handlers/ProcessHandler/ProcessHandlerV2.cs +++ b/src/Agent.Worker/Handlers/ProcessHandler/ProcessHandlerV2.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using Agent.Sdk.Knob; +using Microsoft.TeamFoundation.DistributedTask.WebApi; using Microsoft.VisualStudio.Services.Agent.Util; using Newtonsoft.Json; using System; @@ -280,7 +281,13 @@ private void ValidateScriptArgs( if (!shouldThrow && enableSecureArgumentsAudit) { - ExecutionContext.Warning(StringUtil.Loc("ProcessHandlerInvalidScriptArgs")); + var issue = new Issue + { + Type = IssueType.Warning, + Message = StringUtil.Loc("ProcessHandlerInvalidScriptArgs"), + }; + issue.Data.Add("auditAction", "1"); // ShellTasksValidation = 1 + ExecutionContext.AddIssue(issue); } } if (enableTelemetry && telemetry != null) From a64030e8335956882ed821e46369764b27e03c11 Mon Sep 17 00:00:00 2001 From: Konstantin Tyukalov Date: Mon, 29 Apr 2024 16:46:48 +0400 Subject: [PATCH 2/3] Add validation for issues by knob --- src/Agent.Sdk/Knob/AgentKnobs.cs | 6 ++++++ src/Agent.Worker/TaskCommandExtension.cs | 3 ++- src/Agent.Worker/TaskRunner.cs | 3 ++- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Agent.Sdk/Knob/AgentKnobs.cs b/src/Agent.Sdk/Knob/AgentKnobs.cs index a45c26c290..9af13bcda0 100644 --- a/src/Agent.Sdk/Knob/AgentKnobs.cs +++ b/src/Agent.Sdk/Knob/AgentKnobs.cs @@ -347,6 +347,12 @@ public class AgentKnobs new BuiltInDefaultKnobSource("0")); // Misc + public static readonly Knob EnableTaskIssueAudit = new Knob( + nameof(EnableTaskIssueAudit), + "When true, enable issue source validation for the task.issue command to prevent extra audit events.", + new RuntimeKnobSource("AZP_ENABLE_TASK_AUDIT_ISSUES"), + new BuiltInDefaultKnobSource("false")); + public static readonly Knob EnableIssueSourceValidation = new Knob( nameof(EnableIssueSourceValidation), "When true, enable issue source validation for the task.issue command.", diff --git a/src/Agent.Worker/TaskCommandExtension.cs b/src/Agent.Worker/TaskCommandExtension.cs index 38e949941a..71fbf9f255 100644 --- a/src/Agent.Worker/TaskCommandExtension.cs +++ b/src/Agent.Worker/TaskCommandExtension.cs @@ -371,7 +371,8 @@ public void Execute(IExecutionContext context, Command command) var eventProperties = command.Properties; var data = command.Data; - if (AgentKnobs.EnableIssueSourceValidation.GetValue(context).AsBoolean()) + if (AgentKnobs.EnableIssueSourceValidation.GetValue(context).AsBoolean() + || AgentKnobs.EnableTaskIssueAudit.GetValue(context).AsBoolean()) { ProcessIssueSource(context, command); } diff --git a/src/Agent.Worker/TaskRunner.cs b/src/Agent.Worker/TaskRunner.cs index 36ab3a7965..eceac36e99 100644 --- a/src/Agent.Worker/TaskRunner.cs +++ b/src/Agent.Worker/TaskRunner.cs @@ -412,7 +412,8 @@ private async Task RunAsyncInternal() runtimeVariables, taskDirectory: definition.Directory); - if (AgentKnobs.EnableIssueSourceValidation.GetValue(ExecutionContext).AsBoolean()) + if (AgentKnobs.EnableIssueSourceValidation.GetValue(ExecutionContext).AsBoolean() + || AgentKnobs.EnableTaskIssueAudit.GetValue(ExecutionContext).AsBoolean()) { if (Task.IsServerOwned.HasValue && Task.IsServerOwned.Value && IsCorrelationIdRequired(handler, definition)) { From 86fd3ed1a7d1685c9b8434607e9cee5a0cc988f3 Mon Sep 17 00:00:00 2001 From: Konstantin Tyukalov Date: Tue, 16 Jul 2024 16:12:24 +0400 Subject: [PATCH 3/3] Remove knob --- src/Agent.Sdk/Knob/AgentKnobs.cs | 6 ------ src/Agent.Worker/TaskCommandExtension.cs | 3 +-- src/Agent.Worker/TaskRunner.cs | 3 +-- 3 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/Agent.Sdk/Knob/AgentKnobs.cs b/src/Agent.Sdk/Knob/AgentKnobs.cs index 2ed1264af3..5e469fba9a 100644 --- a/src/Agent.Sdk/Knob/AgentKnobs.cs +++ b/src/Agent.Sdk/Knob/AgentKnobs.cs @@ -360,12 +360,6 @@ public class AgentKnobs new BuiltInDefaultKnobSource("0")); // Misc - public static readonly Knob EnableTaskIssueAudit = new Knob( - nameof(EnableTaskIssueAudit), - "When true, enable issue source validation for the task.issue command to prevent extra audit events.", - new RuntimeKnobSource("AZP_ENABLE_TASK_AUDIT_ISSUES"), - new BuiltInDefaultKnobSource("false")); - public static readonly Knob EnableIssueSourceValidation = new Knob( nameof(EnableIssueSourceValidation), "When true, enable issue source validation for the task.issue command.", diff --git a/src/Agent.Worker/TaskCommandExtension.cs b/src/Agent.Worker/TaskCommandExtension.cs index 71fbf9f255..38e949941a 100644 --- a/src/Agent.Worker/TaskCommandExtension.cs +++ b/src/Agent.Worker/TaskCommandExtension.cs @@ -371,8 +371,7 @@ public void Execute(IExecutionContext context, Command command) var eventProperties = command.Properties; var data = command.Data; - if (AgentKnobs.EnableIssueSourceValidation.GetValue(context).AsBoolean() - || AgentKnobs.EnableTaskIssueAudit.GetValue(context).AsBoolean()) + if (AgentKnobs.EnableIssueSourceValidation.GetValue(context).AsBoolean()) { ProcessIssueSource(context, command); } diff --git a/src/Agent.Worker/TaskRunner.cs b/src/Agent.Worker/TaskRunner.cs index faf8a00049..6f0fdf0401 100644 --- a/src/Agent.Worker/TaskRunner.cs +++ b/src/Agent.Worker/TaskRunner.cs @@ -399,8 +399,7 @@ private async Task RunAsyncInternal() runtimeVariables, taskDirectory: definition.Directory); - if (AgentKnobs.EnableIssueSourceValidation.GetValue(ExecutionContext).AsBoolean() - || AgentKnobs.EnableTaskIssueAudit.GetValue(ExecutionContext).AsBoolean()) + if (AgentKnobs.EnableIssueSourceValidation.GetValue(ExecutionContext).AsBoolean()) { if (Task.IsServerOwned.HasValue && Task.IsServerOwned.Value && IsCorrelationIdRequired(handler, definition)) {