From 0cf2ef5602d1c08733622f0f655e7b8db6755971 Mon Sep 17 00:00:00 2001 From: Mark Roghelia Date: Tue, 16 Mar 2021 14:56:38 -0400 Subject: [PATCH] Introduce enforcement mode for task restrictions (#3329) Introduce enforcement mode for task restrictions Based on a feature variable, don't enforce restrictions from task.json, or just warn, or warn and enforce normally. Also support collecting telemetry on matching task restrictions, so we can see where they'd be applied before fully enabling them. --- src/Agent.Sdk/Knob/AgentKnobs.cs | 13 ++ src/Agent.Worker/JobExtension.cs | 2 +- src/Agent.Worker/TaskCommandExtension.cs | 36 +---- src/Agent.Worker/TaskManager.cs | 9 ++ src/Agent.Worker/TaskRestrictionsChecker.cs | 147 ++++++++++++++++++ src/Agent.Worker/TaskRestrictionsExtension.cs | 76 +++++++++ .../Telemetry/TelemetryCommandExtension.cs | 7 +- src/Agent.Worker/WorkerCommandManager.cs | 61 +------- src/Misc/layoutbin/en-US/strings.json | 2 + src/Test/L0/ServiceInterfacesL0.cs | 2 +- .../CodeCoverageCommandExtensionTests.cs | 25 +-- .../L0/Worker/SetVariableRestrictionsL0.cs | 64 ++++++++ src/Test/L0/Worker/TaskCommandExtensionL0.cs | 22 +-- .../TelemetryCommandExtensionTests.cs | 17 +- .../ResultsCommandExtensionTests.cs | 9 +- 15 files changed, 368 insertions(+), 124 deletions(-) create mode 100644 src/Agent.Worker/TaskRestrictionsChecker.cs create mode 100644 src/Agent.Worker/TaskRestrictionsExtension.cs diff --git a/src/Agent.Sdk/Knob/AgentKnobs.cs b/src/Agent.Sdk/Knob/AgentKnobs.cs index 968dffc3b9..27f57912b6 100644 --- a/src/Agent.Sdk/Knob/AgentKnobs.cs +++ b/src/Agent.Sdk/Knob/AgentKnobs.cs @@ -185,6 +185,19 @@ public class AgentKnobs new EnvironmentKnobSource("AZP_USE_CREDSCAN_REGEXES"), new BuiltInDefaultKnobSource("false")); + // Task restrictions + public static readonly Knob TaskRestrictionsEnforcementMode = new Knob( + nameof(TaskRestrictionsEnforcementMode), + "The enforcement mode for command or variable restrictions defined in tasks. Values are Enabled, WarningOnly, Disabled.", + new RuntimeKnobSource("agent.taskRestrictionsEnforcementMode"), + new BuiltInDefaultKnobSource("WarningOnly")); + + public static readonly Knob EnableTaskRestrictionsTelemetry = new Knob( + nameof(EnableTaskRestrictionsTelemetry), + "Enable capturing telemetry on the enforcement of command or variable restrictions defined in tasks.", + new RuntimeKnobSource("agent.enableTaskRestrictionsTelemetry"), + new BuiltInDefaultKnobSource("false")); + // Misc public static readonly Knob DisableAgentDowngrade = new Knob( nameof(DisableAgentDowngrade), diff --git a/src/Agent.Worker/JobExtension.cs b/src/Agent.Worker/JobExtension.cs index e7c57bac9c..003f1e8a32 100644 --- a/src/Agent.Worker/JobExtension.cs +++ b/src/Agent.Worker/JobExtension.cs @@ -205,7 +205,7 @@ public async Task> InitializeJob(IExecutionContext jobContext, Pipel var restrictions = new List(); if (taskDefinition.Data.Restrictions != null) { - restrictions.Add(taskDefinition.Data.Restrictions); + restrictions.Add(new TaskDefinitionRestrictions(taskDefinition.Data)); } if (string.Equals(WellKnownStepTargetStrings.Restricted, task.Target?.Commands, StringComparison.OrdinalIgnoreCase)) { diff --git a/src/Agent.Worker/TaskCommandExtension.cs b/src/Agent.Worker/TaskCommandExtension.cs index f9568fdb01..397c15e89b 100644 --- a/src/Agent.Worker/TaskCommandExtension.cs +++ b/src/Agent.Worker/TaskCommandExtension.cs @@ -4,7 +4,6 @@ using Agent.Sdk.Knob; using Microsoft.TeamFoundation.DistributedTask.WebApi; using Microsoft.VisualStudio.Services.Agent.Util; -using Minimatch; using System; using System.Collections.Generic; using System.Globalization; @@ -609,13 +608,11 @@ public void Execute(IExecutionContext context, Command command) } } - if (!context.Restrictions.All(restrictions => restrictions.SetVariableAllowed(name))) + var checker = context.GetHostContext().GetService(); + if (checker.CheckSettableVariable(context, name)) { - context.Warning(StringUtil.Loc("SetVariableNotAllowed", name)); - return; + context.SetVariable(name, data, isSecret: isSecret, isOutput: isOutput, isReadOnly: isReadOnly); } - - context.SetVariable(name, data, isSecret: isSecret, isOutput: isOutput, isReadOnly: isReadOnly); } } @@ -789,9 +786,9 @@ public void Execute(IExecutionContext context, Command command) ArgUtil.NotNull(context, nameof(context)); ArgUtil.NotNull(command, nameof(command)); - if (!context.Restrictions.All(restrictions => restrictions.SetVariableAllowed(Constants.PathVariable))) + var checker = context.GetHostContext().GetService(); + if (!checker.CheckSettableVariable(context, Constants.PathVariable)) { - context.Warning(StringUtil.Loc("SetVariableNotAllowed", Constants.PathVariable)); return; } @@ -863,27 +860,4 @@ internal static class TaskSetEndpointEventProperties public static readonly String Field = "field"; public static readonly String Key = "key"; } - - internal static class TaskRestrictionExtension - { - public static Boolean SetVariableAllowed(this TaskRestrictions restrictions, String variable) - { - var allowedList = restrictions.SettableVariables?.Allowed; - if (allowedList == null) - { - return true; - } - - var opts = new Options() { IgnoreCase = true }; - foreach (String pattern in allowedList) - { - if (Minimatcher.Check(variable, pattern, opts)) - { - return true; - } - } - - return false; - } - } } diff --git a/src/Agent.Worker/TaskManager.cs b/src/Agent.Worker/TaskManager.cs index 9682264a55..c3e16b84ca 100644 --- a/src/Agent.Worker/TaskManager.cs +++ b/src/Agent.Worker/TaskManager.cs @@ -326,6 +326,8 @@ public sealed class Definition public sealed class DefinitionData { + public DefinitionVersion Version { get; set; } + public string Name { get; set; } public string FriendlyName { get; set; } public string Description { get; set; } public string HelpMarkDown { get; set; } @@ -339,6 +341,13 @@ public sealed class DefinitionData public TaskRestrictions Restrictions { get; set; } } + public sealed class DefinitionVersion + { + public int Major { get; set; } + public int Minor { get; set; } + public int Patch { get; set; } + } + public sealed class OutputVariable { public string Name { get; set; } diff --git a/src/Agent.Worker/TaskRestrictionsChecker.cs b/src/Agent.Worker/TaskRestrictionsChecker.cs new file mode 100644 index 0000000000..78d58dcc8f --- /dev/null +++ b/src/Agent.Worker/TaskRestrictionsChecker.cs @@ -0,0 +1,147 @@ +using Agent.Sdk.Knob; +using Microsoft.TeamFoundation.Common; +using Microsoft.TeamFoundation.DistributedTask.WebApi; +using Microsoft.VisualStudio.Services.Agent.Util; +using Microsoft.VisualStudio.Services.Agent.Worker.Telemetry; +using Microsoft.VisualStudio.Services.WebPlatform; +using Newtonsoft.Json; +using System; +using System.Collections.Generic; +using System.Linq; + +namespace Microsoft.VisualStudio.Services.Agent.Worker +{ + [ServiceLocator(Default = typeof(TaskRestrictionsChecker))] + public interface ITaskRestrictionsChecker : IAgentService + { + bool CheckCommand(IExecutionContext context, IWorkerCommand workerCommand, Command command); + bool CheckSettableVariable(IExecutionContext context, string variable); + } + + public sealed class TaskRestrictionsChecker : AgentService, ITaskRestrictionsChecker + { + public bool CheckCommand(IExecutionContext context, IWorkerCommand workerCommand, Command command) + { + ArgUtil.NotNull(context, nameof(context)); + ArgUtil.NotNull(workerCommand, nameof(workerCommand)); + ArgUtil.NotNull(command, nameof(command)); + + return Check( + context, + (TaskRestrictions restrictions) => restrictions.IsCommandAllowed(workerCommand), + () => context.Warning(StringUtil.Loc("CommandNotAllowed", command.Area, command.Event)), + () => context.Warning(StringUtil.Loc("CommandNotAllowedWarnOnly", command.Area, command.Event)), + (TaskDefinitionRestrictions restrictions) => PublishCommandTelemetry(context, restrictions, command)); + } + + public bool CheckSettableVariable(IExecutionContext context, string variable) + { + ArgUtil.NotNull(context, nameof(context)); + ArgUtil.NotNull(variable, nameof(variable)); + + return Check( + context, + (TaskRestrictions restrictions) => restrictions.IsSetVariableAllowed(variable), + () => context.Warning(StringUtil.Loc("SetVariableNotAllowed", variable)), + () => context.Warning(StringUtil.Loc("SetVariableNotAllowedWarnOnly", variable)), + (TaskDefinitionRestrictions restrictions) => PublishVariableTelemetry(context, restrictions, variable)); + } + + private bool Check( + IExecutionContext context, + Func predicate, + Action enforcedWarn, + Action unenforcedWarn, + Action publishTelemetry) + { + ArgUtil.NotNull(context, nameof(context)); + ArgUtil.NotNull(predicate, nameof(predicate)); + ArgUtil.NotNull(enforcedWarn, nameof(enforcedWarn)); + ArgUtil.NotNull(unenforcedWarn, nameof(unenforcedWarn)); + ArgUtil.NotNull(publishTelemetry, nameof(publishTelemetry)); + + var failedMatches = context.Restrictions?.Where(restrictions => !predicate(restrictions)); + + if (failedMatches.IsNullOrEmpty()) + { + return true; + } + else + { + var taskMatches = failedMatches.Where(restrictions => restrictions is TaskDefinitionRestrictions).Cast(); + + if(AgentKnobs.EnableTaskRestrictionsTelemetry.GetValue(context).AsBoolean()) + { + foreach(var match in taskMatches) + { + publishTelemetry(match); + } + } + + string mode = AgentKnobs.TaskRestrictionsEnforcementMode.GetValue(context).AsString(); + + if (String.Equals(mode, "Enabled", StringComparison.OrdinalIgnoreCase) || taskMatches.Count() != failedMatches.Count()) + { + // we are enforcing restrictions from tasks, or we matched restrictions from the pipeline, which we always enforce + enforcedWarn(); + return false; + } + else + { + if (!String.Equals(mode, "Disabled", StringComparison.OrdinalIgnoreCase)) + { + unenforcedWarn(); + } + return true; + } + } + } + + private void PublishCommandTelemetry(IExecutionContext context, TaskDefinitionRestrictions restrictions, Command command) + { + ArgUtil.NotNull(context, nameof(context)); + ArgUtil.NotNull(restrictions, nameof(restrictions)); + ArgUtil.NotNull(command, nameof(command)); + + var data = new Dictionary() + { + { "Command", $"{command.Area}.{command.Event}" } + }; + PublishTelemetry(context, restrictions, "TaskRestrictions_Command", data); + } + + private void PublishVariableTelemetry(IExecutionContext context, TaskDefinitionRestrictions restrictions, string variable) + { + ArgUtil.NotNull(context, nameof(context)); + ArgUtil.NotNull(restrictions, nameof(restrictions)); + ArgUtil.NotNull(variable, nameof(variable)); + + var data = new Dictionary() + { + { "Variable", variable } + }; + PublishTelemetry(context, restrictions, "TaskRestrictions_SetVariable", data); + } + + private void PublishTelemetry(IExecutionContext context, TaskDefinitionRestrictions restrictions, string feature, Dictionary data) + { + ArgUtil.NotNull(context, nameof(context)); + ArgUtil.NotNull(restrictions, nameof(restrictions)); + ArgUtil.NotNull(feature, nameof(feature)); + ArgUtil.NotNull(data, nameof(data)); + + data.Add("TaskName", restrictions.Definition.Name); + data.Add("TaskVersion", restrictions.Definition.Version); + + CustomerIntelligenceEvent ciEvent = new CustomerIntelligenceEvent() + { + Area = "AzurePipelinesAgent", + Feature = feature, + Properties = data + }; + + var publishCommand = new PublishTelemetryCommand(); + publishCommand.PublishEvent(context, ciEvent); + } + } +} diff --git a/src/Agent.Worker/TaskRestrictionsExtension.cs b/src/Agent.Worker/TaskRestrictionsExtension.cs new file mode 100644 index 0000000000..c65c8f5d07 --- /dev/null +++ b/src/Agent.Worker/TaskRestrictionsExtension.cs @@ -0,0 +1,76 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.TeamFoundation.DistributedTask.WebApi; +using Microsoft.VisualStudio.Services.Agent.Util; +using Minimatch; +using System; + +namespace Microsoft.VisualStudio.Services.Agent.Worker +{ + [ AttributeUsage(AttributeTargets.Class, Inherited = false, AllowMultiple = false)] + public sealed class CommandRestrictionAttribute : Attribute + { + public bool AllowedInRestrictedMode { get; set; } + } + + public static class TaskRestrictionExtension + { + public static Boolean IsCommandAllowed(this TaskRestrictions restrictions, IWorkerCommand command) + { + ArgUtil.NotNull(command, nameof(command)); + + if (restrictions.Commands?.Mode == TaskCommandMode.Restricted) + { + foreach (var attr in command.GetType().GetCustomAttributes(typeof(CommandRestrictionAttribute), false)) + { + var cra = attr as CommandRestrictionAttribute; + if (cra.AllowedInRestrictedMode) + { + return true; + } + } + + return false; + } + else + { + return true; + } + } + + public static Boolean IsSetVariableAllowed(this TaskRestrictions restrictions, String variable) + { + ArgUtil.NotNull(variable, nameof(variable)); + + var allowedList = restrictions.SettableVariables?.Allowed; + if (allowedList == null) + { + return true; + } + + var opts = new Options() { IgnoreCase = true }; + foreach (String pattern in allowedList) + { + if (Minimatcher.Check(variable, pattern, opts)) + { + return true; + } + } + + return false; + } + } + + public sealed class TaskDefinitionRestrictions : TaskRestrictions + { + public TaskDefinitionRestrictions(DefinitionData definition) : base() + { + Definition = definition; + Commands = definition.Restrictions?.Commands; + SettableVariables = definition.Restrictions?.SettableVariables; + } + + public DefinitionData Definition { get; } + } +} diff --git a/src/Agent.Worker/Telemetry/TelemetryCommandExtension.cs b/src/Agent.Worker/Telemetry/TelemetryCommandExtension.cs index d20f71e830..18e0d76123 100644 --- a/src/Agent.Worker/Telemetry/TelemetryCommandExtension.cs +++ b/src/Agent.Worker/Telemetry/TelemetryCommandExtension.cs @@ -27,7 +27,6 @@ public sealed class PublishTelemetryCommand: IWorkerCommand public string Name => "publish"; public List Aliases => null; - [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Maintainability", "CA2000:Dispose objects before losing scope", MessageId = "GetVssConnection")] public void Execute(IExecutionContext context, Command command) { ArgUtil.NotNull(context, nameof(context)); @@ -67,6 +66,12 @@ public void Execute(IExecutionContext context, Command command) throw new ArgumentException(StringUtil.Loc("TelemetryCommandDataError", data, ex.Message)); } + PublishEvent(context, ciEvent); + } + + [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Maintainability", "CA2000:Dispose objects before losing scope", MessageId = "GetVssConnection")] + public void PublishEvent(IExecutionContext context, CustomerIntelligenceEvent ciEvent) + { ICustomerIntelligenceServer ciService; VssConnection vssConnection; try diff --git a/src/Agent.Worker/WorkerCommandManager.cs b/src/Agent.Worker/WorkerCommandManager.cs index 1785711fdf..55bad557c8 100644 --- a/src/Agent.Worker/WorkerCommandManager.cs +++ b/src/Agent.Worker/WorkerCommandManager.cs @@ -100,22 +100,12 @@ public bool TryProcessCommand(IExecutionContext context, string input) return false; } - IWorkerCommandRestrictionPolicy restrictionPolicy; - if (context.Restrictions.Any(restrictions => restrictions.Commands?.Mode == TaskCommandMode.Restricted)) - { - restrictionPolicy = new AttributeBasedWorkerCommandRestrictionPolicy(); - } - else - { - restrictionPolicy = new UnrestricedWorkerCommandRestrictionPolicy(); - } - // process logging command in serialize oreder. lock (_commandSerializeLock) { try { - extension.ProcessCommand(context, command, restrictionPolicy); + extension.ProcessCommand(context, command); } catch (Exception ex) { @@ -155,7 +145,7 @@ public interface IWorkerCommandExtension : IExtension HostTypes SupportedHostTypes { get; } - void ProcessCommand(IExecutionContext context, Command command, IWorkerCommandRestrictionPolicy policy); + void ProcessCommand(IExecutionContext context, Command command); } public interface IWorkerCommand @@ -167,42 +157,6 @@ public interface IWorkerCommand void Execute(IExecutionContext context, Command command); } - public interface IWorkerCommandRestrictionPolicy - { - bool isCommandAllowed(IWorkerCommand command); - } - - public class UnrestricedWorkerCommandRestrictionPolicy: IWorkerCommandRestrictionPolicy - { - public bool isCommandAllowed(IWorkerCommand command) - { - return true; - } - } - - [ AttributeUsage(AttributeTargets.Class, Inherited = false, AllowMultiple = false)] - public sealed class CommandRestrictionAttribute : Attribute - { - public bool AllowedInRestrictedMode { get; set; } - } - - public class AttributeBasedWorkerCommandRestrictionPolicy: IWorkerCommandRestrictionPolicy - { - public bool isCommandAllowed(IWorkerCommand command) - { - ArgUtil.NotNull(command, nameof(command)); - foreach (var attr in command.GetType().GetCustomAttributes(typeof(CommandRestrictionAttribute), false)) - { - var cra = attr as CommandRestrictionAttribute; - if (cra.AllowedInRestrictedMode) - { - return true; - } - } - return false; - } - } - public abstract class BaseWorkerCommandExtension: AgentService, IWorkerCommandExtension { @@ -242,25 +196,22 @@ public IWorkerCommand GetWorkerCommand(String name) return commandExecutor; } - public void ProcessCommand(IExecutionContext context, Command command, IWorkerCommandRestrictionPolicy restrictionPolicy) + public void ProcessCommand(IExecutionContext context, Command command) { ArgUtil.NotNull(context, nameof(context)); ArgUtil.NotNull(command, nameof(command)); - ArgUtil.NotNull(restrictionPolicy, nameof(restrictionPolicy)); var commandExecutor = GetWorkerCommand(command.Event); if (commandExecutor == null) { throw new Exception(StringUtil.Loc("CommandNotFound2", CommandArea.ToLowerInvariant(), command.Event, CommandArea)); } - if (restrictionPolicy.isCommandAllowed(commandExecutor)) + + var checker = context.GetHostContext().GetService(); + if (checker.CheckCommand(context, commandExecutor, command)) { commandExecutor.Execute(context, command); } - else - { - context.Warning(StringUtil.Loc("CommandNotAllowed", command.Area, command.Event)); - } } } diff --git a/src/Misc/layoutbin/en-US/strings.json b/src/Misc/layoutbin/en-US/strings.json index 5af006c626..f019f6edeb 100644 --- a/src/Misc/layoutbin/en-US/strings.json +++ b/src/Misc/layoutbin/en-US/strings.json @@ -214,6 +214,7 @@ ".{0}config.{1} remove --unattended --auth negotiate --username myDomain\\myUserName --password myPassword" ], "CommandNotAllowed": "##vso[{0}.{1}] is not allowed in this step due to policy restrictions. Please reference documentation (http://go.microsoft.com/fwlink/?LinkId=817296)", + "CommandNotAllowedWarnOnly": "##vso[{0}.{1}] has been declared as disallowed due to policy restrictions. This command will be suppressed at a future date. Please reference documentation (http://go.microsoft.com/fwlink/?LinkId=817296)", "CommandNotFound": "Cannot find command extension for ##vso[{0}.command]. Please reference documentation (http://go.microsoft.com/fwlink/?LinkId=817296)", "CommandNotFound2": "##vso[{0}.{1}] is not a recognized command for {2} command extension. Please reference documentation (http://go.microsoft.com/fwlink/?LinkId=817296).", "CommandNotSupported": "{0} commands are not supported for {1} flow. Please reference documentation (http://go.microsoft.com/fwlink/?LinkId=817296)", @@ -560,6 +561,7 @@ "SetBuildVars": "Set build variables.", "SetEnvVar": "Setting environment variable {0}", "SetVariableNotAllowed": "Setting variable '{0}' has been disabled by the task or build definition.", + "SetVariableNotAllowedWarnOnly": "Setting variable '{0}' has been declared as disallowed by the task or build definition. This command will be suppressed at a future date.", "ShallowCheckoutFail": "Git checkout failed on shallow repository, this might because of git fetch with depth '{0}' doesn't include the checkout commit '{1}'. Please reference documentation (http://go.microsoft.com/fwlink/?LinkId=829603)", "ShallowLfsFetchFail": "Git lfs fetch failed on shallow repository, this might because of git fetch with depth '{0}' doesn't include the lfs fetch commit '{1}'. Please reference documentation (http://go.microsoft.com/fwlink/?LinkId=829603)", "ShutdownMessage": "Restarting the machine in order to launch agent in interactive mode.", diff --git a/src/Test/L0/ServiceInterfacesL0.cs b/src/Test/L0/ServiceInterfacesL0.cs index 9f4b60a0e8..9a5f44d112 100644 --- a/src/Test/L0/ServiceInterfacesL0.cs +++ b/src/Test/L0/ServiceInterfacesL0.cs @@ -97,7 +97,7 @@ public void WorkerInterfacesSpecifyDefaultImplementation() typeof(IResultReader), typeof(INUnitResultsXmlReader), typeof(IWorkerCommand), - typeof(IWorkerCommandRestrictionPolicy) + typeof(ITaskRestrictionsChecker) }; Validate( assembly: typeof(IStepsRunner).GetTypeInfo().Assembly, diff --git a/src/Test/L0/Worker/CodeCoverage/CodeCoverageCommandExtensionTests.cs b/src/Test/L0/Worker/CodeCoverage/CodeCoverageCommandExtensionTests.cs index 8d138d4f3c..2913e95214 100644 --- a/src/Test/L0/Worker/CodeCoverage/CodeCoverageCommandExtensionTests.cs +++ b/src/Test/L0/Worker/CodeCoverage/CodeCoverageCommandExtensionTests.cs @@ -26,7 +26,6 @@ public class CodeCoverageCommandExtensionTests private Mock _mockCommandContext; private List _codeCoverageStatistics; private Variables _variables; - private IWorkerCommandRestrictionPolicy _policy = new UnrestricedWorkerCommandRestrictionPolicy(); #region publish code coverage tests [Fact] @@ -40,7 +39,7 @@ public void PublishCodeCoverageWithNoCCTool() publishCCCommand.Initialize(_hc); var command = new Command("codecoverage", "publish"); command.Properties.Add("summaryfile", "a.xml"); - Assert.Throws(() => publishCCCommand.ProcessCommand(_ec.Object, command, _policy)); + Assert.Throws(() => publishCCCommand.ProcessCommand(_ec.Object, command)); } } @@ -55,7 +54,7 @@ public void PublishCodeCoverageWithRelease() publishCCCommand.Initialize(_hc); var command = new Command("codecoverage", "publish"); _variables.Set("system.hostType", "release"); - publishCCCommand.ProcessCommand(_ec.Object, command, _policy); + publishCCCommand.ProcessCommand(_ec.Object, command); Assert.Equal(1, _warnings.Count); Assert.Equal(0, _errors.Count); } @@ -71,7 +70,7 @@ public void PublishCodeCoverageWithNoSummaryFileInput() var publishCCCommand = new CodeCoverageCommandExtension(); publishCCCommand.Initialize(_hc); var command = new Command("codecoverage", "publish"); - Assert.Throws(() => publishCCCommand.ProcessCommand(_ec.Object, command, _policy)); + Assert.Throws(() => publishCCCommand.ProcessCommand(_ec.Object, command)); } } @@ -87,7 +86,7 @@ public void PublishCodeCoverageWithInvalidCCTool() var command = new Command("codecoverage", "publish"); command.Properties.Add("codecoveragetool", "InvalidTool"); command.Properties.Add("summaryfile", "a.xml"); - Assert.Throws(() => publishCCCommand.ProcessCommand(_ec.Object, command, _policy)); + Assert.Throws(() => publishCCCommand.ProcessCommand(_ec.Object, command)); } } @@ -113,7 +112,7 @@ public void Publish_CoberturaNewIndexFile() command.Properties.Add("codecoveragetool", "cobertura"); command.Properties.Add("summaryfile", coberturaXml); command.Properties.Add("reportdirectory", reportDirectory); - publishCCCommand.ProcessCommand(_ec.Object, command, _policy); + publishCCCommand.ProcessCommand(_ec.Object, command); Assert.Equal(0, _warnings.Count); Assert.Equal(0, _errors.Count); _mockCodeCoveragePublisher.Verify(x => x.PublishCodeCoverageSummaryAsync(It.IsAny(), It.IsAny>(), It.IsAny(), It.IsAny())); @@ -149,7 +148,7 @@ public void Publish_WithIndexHtmFileinReportDirectory() command.Properties.Add("codecoveragetool", "mockCCTool"); command.Properties.Add("summaryfile", summaryFile); command.Properties.Add("reportdirectory", reportDirectory); - publishCCCommand.ProcessCommand(_ec.Object, command, _policy); + publishCCCommand.ProcessCommand(_ec.Object, command); Assert.Equal(0, _warnings.Count); Assert.Equal(0, _errors.Count); _mockCodeCoveragePublisher.Verify(x => x.PublishCodeCoverageSummaryAsync(It.IsAny(), It.IsAny>(), It.IsAny(), It.IsAny())); @@ -185,7 +184,7 @@ public void Publish_WithIndexHtmAndHtmlFileInReportDirectory() command.Properties.Add("codecoveragetool", "mockCCTool"); command.Properties.Add("summaryfile", summaryFile); command.Properties.Add("reportdirectory", reportDirectory); - publishCCCommand.ProcessCommand(_ec.Object, command, _policy); + publishCCCommand.ProcessCommand(_ec.Object, command); Assert.Equal(0, _warnings.Count); Assert.Equal(0, _errors.Count); _mockCodeCoveragePublisher.Verify(x => x.PublishCodeCoverageSummaryAsync(It.IsAny(), It.IsAny>(), It.IsAny(), It.IsAny())); @@ -218,7 +217,7 @@ public void PublishesCCFilesWhenCodeCoverageDataIsNull() command.Properties.Add("summaryfile", summaryFile); _mocksummaryReader.Setup(x => x.GetCodeCoverageSummary(It.IsAny(), It.IsAny())) .Returns((List)null); - publishCCCommand.ProcessCommand(_ec.Object, command, _policy); + publishCCCommand.ProcessCommand(_ec.Object, command); Assert.Equal(1, _warnings.Count); Assert.Equal(0, _errors.Count); _mockCodeCoveragePublisher.Verify(x => x.PublishCodeCoverageFilesAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>>(), It.IsAny(), It.IsAny())); @@ -249,7 +248,7 @@ public void PublishCCFilesWithOnlyReportDirectoryInput() command.Properties.Add("codecoveragetool", "mockCCTool"); command.Properties.Add("summaryfile", summaryFile); command.Properties.Add("reportdirectory", reportDirectory); - publishCCCommand.ProcessCommand(_ec.Object, command, _policy); + publishCCCommand.ProcessCommand(_ec.Object, command); Assert.Equal(0, _warnings.Count); Assert.Equal(0, _errors.Count); _mockCodeCoveragePublisher.Verify(x => x.PublishCodeCoverageSummaryAsync(It.IsAny(), It.IsAny>(), It.IsAny(), It.IsAny())); @@ -280,7 +279,7 @@ public void PublishCCFilesWithOnlyAdditionalFilesInput() command.Properties.Add("codecoveragetool", "mockCCTool"); command.Properties.Add("summaryfile", summaryFile); command.Properties.Add("additionalcodecoveragefiles", summaryFile); - publishCCCommand.ProcessCommand(_ec.Object, command, _policy); + publishCCCommand.ProcessCommand(_ec.Object, command); Assert.Equal(0, _warnings.Count); Assert.Equal(0, _errors.Count); _mockCodeCoveragePublisher.Verify(x => x.PublishCodeCoverageSummaryAsync(It.IsAny(), It.IsAny>(), It.IsAny(), It.IsAny())); @@ -314,7 +313,7 @@ public void PublishCCWithBothReportDirectoryAndAdditioanlFilesInputs() command.Properties.Add("summaryfile", summaryFile); command.Properties.Add("reportdirectory", reportDirectory); command.Properties.Add("additionalcodecoveragefiles", summaryFile); - publishCCCommand.ProcessCommand(_ec.Object, command, _policy); + publishCCCommand.ProcessCommand(_ec.Object, command); Assert.Equal(0, _warnings.Count); Assert.Equal(0, _errors.Count); _mockCodeCoveragePublisher.Verify(x => x.PublishCodeCoverageSummaryAsync(It.IsAny(), It.IsAny>(), It.IsAny(), It.IsAny())); @@ -332,6 +331,7 @@ public void PublishCCWithBothReportDirectoryAndAdditioanlFilesInputs() private TestHostContext SetupMocks([CallerMemberName] string name = "") { var _hc = new TestHostContext(this, name); + _hc.SetSingleton(new TaskRestrictionsChecker() as ITaskRestrictionsChecker); _codeCoverageStatistics = new List { new CodeCoverageStatistics { Label = "label", Covered = 10, Total = 10, Position = 1 } }; _mocksummaryReader = new Mock(); if (String.Equals(name, "Publish_CoberturaNewIndexFile")) @@ -366,6 +366,7 @@ private TestHostContext SetupMocks([CallerMemberName] string name = "") endpointAuthorization.Parameters[EndpointAuthorizationParameters.AccessToken] = "accesstoken"; _ec = new Mock(); + _ec.Setup(x => x.Restrictions).Returns(new List()); _ec.Setup(x => x.Endpoints).Returns(new List { new ServiceEndpoint { Url = new Uri("http://dummyurl"), Name = WellKnownServiceEndpointNames.SystemVssConnection, Authorization = endpointAuthorization } }); _ec.Setup(x => x.Variables).Returns(_variables); _ec.Setup(x => x.TranslateToHostPath(It.IsAny())).Returns( (string x) => x ); diff --git a/src/Test/L0/Worker/SetVariableRestrictionsL0.cs b/src/Test/L0/Worker/SetVariableRestrictionsL0.cs index a5328a527a..46ab4ea7b3 100644 --- a/src/Test/L0/Worker/SetVariableRestrictionsL0.cs +++ b/src/Test/L0/Worker/SetVariableRestrictionsL0.cs @@ -248,9 +248,73 @@ public void PrependPathAllowed() } } + [Theory] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + [InlineData("Enabled", false, true)] + [InlineData("enabled", false, true)] + [InlineData("WarningOnly", true, true)] + [InlineData("Disabled", true, false)] + [InlineData("disAbled", true, false)] + [InlineData("Unexpected", true, true)] + public void TaskDefinitionRestrictionsEnforcementMode(string mode, bool variableExpected, bool warningExpected) + { + using (TestHostContext hc = CreateTestContext()) + { + _ec.Setup(x => x.GetVariableValueOrDefault("agent.taskRestrictionsEnforcementMode")).Returns(mode); + var definition = new DefinitionData { Name = "TestTask", Version = new DefinitionVersion { Major = 2, Minor = 7, Patch = 1 } }; + // allow no variables + var restrictions = new TaskDefinitionRestrictions(definition) { SettableVariables = new TaskVariableRestrictions() }; + _ec.Object.Restrictions.Add(restrictions); + var variable = "myVar"; + var value = "myValue"; + Command command = SetVariableCommand(variable, value); + TaskSetVariableCommand setVariable = new TaskSetVariableCommand(); + setVariable.Execute(_ec.Object, command); + Assert.Equal((variableExpected ? value : null), _variables.Get(variable)); + if (warningExpected) + { + Assert.Equal(1, _warnings.Count); + if (variableExpected) + { + Assert.EndsWith("SetVariableNotAllowedWarnOnly", _warnings[0]); + } + else + { + Assert.EndsWith("SetVariableNotAllowed", _warnings[0]); + } + } + else + { + Assert.Equal(0, _warnings.Count); + } + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EnforcementModeDoesNotImpactPipelineRestrictions() + { + using (TestHostContext hc = CreateTestContext()) + { + // This does nothing because this isn't using a TaskDefinitionRestrictions + _ec.Setup(x => x.GetVariableValueOrDefault("agent.taskRestrictionsEnforcementMode")).Returns("Disabled"); + _ec.Object.Restrictions.Add(new TaskRestrictions() { SettableVariables = new TaskVariableRestrictions() }); + var variable = "myVar"; + var setVariable = new TaskSetVariableCommand(); + var command = SetVariableCommand(variable, "myVal"); + setVariable.Execute(_ec.Object, command); + Assert.Equal(null, _variables.Get(variable)); + Assert.Equal(1, _warnings.Count); + Assert.Contains("SetVariableNotAllowed", _warnings[0]); + } + } + private TestHostContext CreateTestContext([CallerMemberName] String testName = "") { var hc = new TestHostContext(this, testName); + hc.SetSingleton(new TaskRestrictionsChecker() as ITaskRestrictionsChecker); _variables = new Variables( hostContext: hc, diff --git a/src/Test/L0/Worker/TaskCommandExtensionL0.cs b/src/Test/L0/Worker/TaskCommandExtensionL0.cs index eeefa45a6d..8b96b8e5e6 100644 --- a/src/Test/L0/Worker/TaskCommandExtensionL0.cs +++ b/src/Test/L0/Worker/TaskCommandExtensionL0.cs @@ -15,7 +15,6 @@ public sealed class TaskCommandExtensionL0 { private Mock _ec; private ServiceEndpoint _endpoint; - private IWorkerCommandRestrictionPolicy _policy = new UnrestricedWorkerCommandRestrictionPolicy(); [Fact] [Trait("Level", "L0")] @@ -32,7 +31,7 @@ public void SetEndpointAuthParameter() cmd.Properties.Add("id", Guid.Empty.ToString()); cmd.Properties.Add("key", "test"); - commandExtension.ProcessCommand(_ec.Object, cmd, _policy); + commandExtension.ProcessCommand(_ec.Object, cmd); Assert.Equal(_endpoint.Authorization.Parameters["test"], "blah"); } @@ -52,7 +51,7 @@ public void SetEndpointDataParameter() cmd.Properties.Add("id", Guid.Empty.ToString()); cmd.Properties.Add("key", "test"); - commandExtension.ProcessCommand(_ec.Object, cmd, _policy); + commandExtension.ProcessCommand(_ec.Object, cmd); Assert.Equal(_endpoint.Data["test"], "blah"); } @@ -71,7 +70,7 @@ public void SetEndpointUrlParameter() cmd.Properties.Add("field", "url"); cmd.Properties.Add("id", Guid.Empty.ToString()); - commandExtension.ProcessCommand(_ec.Object, cmd, _policy); + commandExtension.ProcessCommand(_ec.Object, cmd); Assert.Equal(_endpoint.Url.ToString(), cmd.Data); } @@ -86,7 +85,7 @@ public void SetEndpointWithoutValue() { TaskCommandExtension commandExtension = new TaskCommandExtension(); var cmd = new Command("task", "setEndpoint"); - Assert.Throws(() => commandExtension.ProcessCommand(_ec.Object, cmd, _policy)); + Assert.Throws(() => commandExtension.ProcessCommand(_ec.Object, cmd)); } } @@ -100,7 +99,7 @@ public void SetEndpointWithoutEndpointField() TaskCommandExtension commandExtension = new TaskCommandExtension(); var cmd = new Command("task", "setEndpoint"); - Assert.Throws(() => commandExtension.ProcessCommand(_ec.Object, cmd, _policy)); + Assert.Throws(() => commandExtension.ProcessCommand(_ec.Object, cmd)); } } @@ -115,7 +114,7 @@ public void SetEndpointInvalidEndpointField() var cmd = new Command("task", "setEndpoint"); cmd.Properties.Add("field", "blah"); - Assert.Throws(() => commandExtension.ProcessCommand(_ec.Object, cmd, _policy)); + Assert.Throws(() => commandExtension.ProcessCommand(_ec.Object, cmd)); } } @@ -130,7 +129,7 @@ public void SetEndpointWithoutEndpointId() var cmd = new Command("task", "setEndpoint"); cmd.Properties.Add("field", "url"); - Assert.Throws(() => commandExtension.ProcessCommand(_ec.Object, cmd, _policy)); + Assert.Throws(() => commandExtension.ProcessCommand(_ec.Object, cmd)); } } @@ -146,7 +145,7 @@ public void SetEndpointInvalidEndpointId() cmd.Properties.Add("field", "url"); cmd.Properties.Add("id", "blah"); - Assert.Throws(() => commandExtension.ProcessCommand(_ec.Object, cmd, _policy)); + Assert.Throws(() => commandExtension.ProcessCommand(_ec.Object, cmd)); } } @@ -162,7 +161,7 @@ public void SetEndpointIdWithoutEndpointKey() cmd.Properties.Add("field", "authParameter"); cmd.Properties.Add("id", Guid.Empty.ToString()); - Assert.Throws(() => commandExtension.ProcessCommand(_ec.Object, cmd, _policy)); + Assert.Throws(() => commandExtension.ProcessCommand(_ec.Object, cmd)); } } @@ -179,13 +178,14 @@ public void SetEndpointUrlWithInvalidValue() cmd.Properties.Add("field", "url"); cmd.Properties.Add("id", Guid.Empty.ToString()); - Assert.Throws(() => commandExtension.ProcessCommand(_ec.Object, cmd, _policy)); + Assert.Throws(() => commandExtension.ProcessCommand(_ec.Object, cmd)); } } private TestHostContext SetupMocks([CallerMemberName] string name = "") { var _hc = new TestHostContext(this, name); + _hc.SetSingleton(new TaskRestrictionsChecker() as ITaskRestrictionsChecker); _ec = new Mock(); _endpoint = new ServiceEndpoint() diff --git a/src/Test/L0/Worker/Telemetry/TelemetryCommandExtensionTests.cs b/src/Test/L0/Worker/Telemetry/TelemetryCommandExtensionTests.cs index 516933def0..daf3d64cb9 100644 --- a/src/Test/L0/Worker/Telemetry/TelemetryCommandExtensionTests.cs +++ b/src/Test/L0/Worker/Telemetry/TelemetryCommandExtensionTests.cs @@ -21,7 +21,6 @@ public class TelemetryCommandExtensionTests private List _errors = new List(); private Mock _mockCiService; private Mock _mockCommandContext; - private IWorkerCommandRestrictionPolicy _policy = new UnrestricedWorkerCommandRestrictionPolicy(); [Fact] [Trait("Level", "L0")] @@ -46,7 +45,7 @@ public void PublishTelemetryCommandWithCiProps() cmd.Properties.Add("area", "Test"); cmd.Properties.Add("feature", "Task"); - publishTelemetryCmd.ProcessCommand(_ec.Object, cmd, _policy); + publishTelemetryCmd.ProcessCommand(_ec.Object, cmd); _mockCiService.Verify(s => s.PublishEventsAsync(It.Is(e => VerifyEvent(e, data))), Times.Once()); } } @@ -72,7 +71,7 @@ public void PublishTelemetryCommandWithSpecialCiProps() cmd.Properties.Add("area", "Test"); cmd.Properties.Add("feature", "Task"); - publishTelemetryCmd.ProcessCommand(_ec.Object, cmd, _policy); + publishTelemetryCmd.ProcessCommand(_ec.Object, cmd); _mockCiService.Verify(s => s.PublishEventsAsync(It.Is(e => VerifyEvent(e, data))), Times.Once()); } } @@ -91,7 +90,7 @@ public void PublishTelemetryWithoutArea() cmd.Data = "key1=value1;key2=value2"; cmd.Properties.Add("feature", "Task"); - Assert.Throws(() => publishTelemetryCmd.ProcessCommand(_ec.Object, cmd, _policy)); + Assert.Throws(() => publishTelemetryCmd.ProcessCommand(_ec.Object, cmd)); } } @@ -109,7 +108,7 @@ public void PublishTelemetryWithoutFeature() cmd.Data = "key1=value1;key2=value2"; cmd.Properties.Add("area", "Test"); - Assert.Throws(() => publishTelemetryCmd.ProcessCommand(_ec.Object, cmd, _policy)); + Assert.Throws(() => publishTelemetryCmd.ProcessCommand(_ec.Object, cmd)); } } @@ -127,7 +126,7 @@ public void PublishTelemetryWithoutCiData() cmd.Properties.Add("area", "Test"); cmd.Properties.Add("feature", "Task"); - Assert.Throws(() => publishTelemetryCmd.ProcessCommand(_ec.Object, cmd, _policy)); + Assert.Throws(() => publishTelemetryCmd.ProcessCommand(_ec.Object, cmd)); } } @@ -145,7 +144,7 @@ public void PublishTelemetryWithoutCommandEvent() cmd.Properties.Add("area", "Test"); cmd.Properties.Add("feature", "Task"); - var ex = Assert.Throws(() => publishTelemetryCmd.ProcessCommand(_ec.Object, cmd, _policy)); + var ex = Assert.Throws(() => publishTelemetryCmd.ProcessCommand(_ec.Object, cmd)); } } @@ -174,7 +173,7 @@ public void PublishTelemetryCommandWithExceptionFromServer() cmd.Properties.Add("area", "Test"); cmd.Properties.Add("feature", "Task"); - publishTelemetryCmd.ProcessCommand(_ec.Object, cmd, _policy); + publishTelemetryCmd.ProcessCommand(_ec.Object, cmd); _mockCiService.Verify(s => s.PublishEventsAsync(It.Is(e => VerifyEvent(e, data))), Times.Once()); Assert.True(_warnings.Count > 0); } @@ -183,6 +182,7 @@ public void PublishTelemetryCommandWithExceptionFromServer() private TestHostContext SetupMocks([CallerMemberName] string name = "") { var _hc = new TestHostContext(this, name); + _hc.SetSingleton(new TaskRestrictionsChecker() as ITaskRestrictionsChecker); _mockCiService = new Mock(); _hc.SetSingleton(_mockCiService.Object); @@ -199,6 +199,7 @@ private TestHostContext SetupMocks([CallerMemberName] string name = "") endpointAuthorization.Parameters[EndpointAuthorizationParameters.AccessToken] = "accesstoken"; _ec = new Mock(); + _ec.Setup(x => x.Restrictions).Returns(new List()); _ec.Setup(x => x.Endpoints).Returns(new List { new ServiceEndpoint { Url = new Uri("http://dummyurl"), Name = WellKnownServiceEndpointNames.SystemVssConnection, Authorization = endpointAuthorization } }); _ec.Setup(x => x.Variables).Returns(variables); var asyncCommands = new List(); diff --git a/src/Test/L0/Worker/TestResults/ResultsCommandExtensionTests.cs b/src/Test/L0/Worker/TestResults/ResultsCommandExtensionTests.cs index 4660769ebd..674e782b12 100644 --- a/src/Test/L0/Worker/TestResults/ResultsCommandExtensionTests.cs +++ b/src/Test/L0/Worker/TestResults/ResultsCommandExtensionTests.cs @@ -31,7 +31,6 @@ public sealed class ResultsCommandTests private Mock _mockCustomerIntelligenceServer; private Mock _mockFeatureFlagService; private Variables _variables; - private IWorkerCommandRestrictionPolicy _policy = new UnrestricedWorkerCommandRestrictionPolicy(); public ResultsCommandTests() { @@ -62,7 +61,7 @@ public void Publish_NullTestRunner() var command = new Command("results", "publish"); command.Properties.Add("resultFiles", "ResultFile.txt"); - Assert.Throws(() => resultCommand.ProcessCommand(_ec.Object, command, _policy)); + Assert.Throws(() => resultCommand.ProcessCommand(_ec.Object, command)); } } @@ -76,7 +75,7 @@ public void Publish_NullTestResultFiles() var resultCommand = new ResultsCommandExtension(); resultCommand.Initialize(_hc); var command = new Command("results", "publish"); - Assert.Throws(() => resultCommand.ProcessCommand(_ec.Object, command, _policy)); + Assert.Throws(() => resultCommand.ProcessCommand(_ec.Object, command)); } } @@ -92,7 +91,7 @@ public void Publish_DataIsHonoredWhenTestResultsFieldIsNotSpecified() var command = new Command("results", "publish"); command.Properties.Add("type", "mockResults"); command.Data = "testfile1,testfile2"; - resultCommand.ProcessCommand(_ec.Object, command, _policy); + resultCommand.ProcessCommand(_ec.Object, command); Assert.Equal(0, _errors.Count()); } @@ -143,6 +142,7 @@ private TestDataProvider MockParserData() private TestHostContext SetupMocks([CallerMemberName] string name = "", bool includePipelineVariables = false) { var _hc = new TestHostContext(this, name); + _hc.SetSingleton(new TaskRestrictionsChecker() as ITaskRestrictionsChecker); _hc.SetSingleton(_mockTestRunDataPublisher.Object); _hc.SetSingleton(_mockParser.Object); @@ -176,6 +176,7 @@ private TestHostContext SetupMocks([CallerMemberName] string name = "", bool inc endpointAuthorization.Parameters[EndpointAuthorizationParameters.AccessToken] = "accesstoken"; _ec = new Mock(); + _ec.Setup(x => x.Restrictions).Returns(new List()); _ec.Setup(x => x.Endpoints).Returns(new List { new ServiceEndpoint { Url = new Uri("http://dummyurl"), Name = WellKnownServiceEndpointNames.SystemVssConnection, Authorization = endpointAuthorization } }); _ec.Setup(x => x.Variables).Returns(_variables); var asyncCommands = new List();