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();