Skip to content

Commit

Permalink
Introduce enforcement mode for task restrictions (#3329)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mjroghelia authored Mar 16, 2021
1 parent c6c241a commit 0cf2ef5
Show file tree
Hide file tree
Showing 15 changed files with 368 additions and 124 deletions.
13 changes: 13 additions & 0 deletions src/Agent.Sdk/Knob/AgentKnobs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion src/Agent.Worker/JobExtension.cs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ public async Task<List<IStep>> InitializeJob(IExecutionContext jobContext, Pipel
var restrictions = new List<TaskRestrictions>();
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))
{
Expand Down
36 changes: 5 additions & 31 deletions src/Agent.Worker/TaskCommandExtension.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -609,13 +608,11 @@ public void Execute(IExecutionContext context, Command command)
}
}

if (!context.Restrictions.All(restrictions => restrictions.SetVariableAllowed(name)))
var checker = context.GetHostContext().GetService<ITaskRestrictionsChecker>();
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);
}
}

Expand Down Expand Up @@ -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<ITaskRestrictionsChecker>();
if (!checker.CheckSettableVariable(context, Constants.PathVariable))
{
context.Warning(StringUtil.Loc("SetVariableNotAllowed", Constants.PathVariable));
return;
}

Expand Down Expand Up @@ -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;
}
}
}
9 changes: 9 additions & 0 deletions src/Agent.Worker/TaskManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand All @@ -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; }
Expand Down
147 changes: 147 additions & 0 deletions src/Agent.Worker/TaskRestrictionsChecker.cs
Original file line number Diff line number Diff line change
@@ -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<TaskRestrictions, bool> predicate,
Action enforcedWarn,
Action unenforcedWarn,
Action<TaskDefinitionRestrictions> 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<TaskDefinitionRestrictions>();

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<string, object>()
{
{ "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<string, object>()
{
{ "Variable", variable }
};
PublishTelemetry(context, restrictions, "TaskRestrictions_SetVariable", data);
}

private void PublishTelemetry(IExecutionContext context, TaskDefinitionRestrictions restrictions, string feature, Dictionary<string, object> 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);
}
}
}
76 changes: 76 additions & 0 deletions src/Agent.Worker/TaskRestrictionsExtension.cs
Original file line number Diff line number Diff line change
@@ -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; }
}
}
7 changes: 6 additions & 1 deletion src/Agent.Worker/Telemetry/TelemetryCommandExtension.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ public sealed class PublishTelemetryCommand: IWorkerCommand
public string Name => "publish";
public List<string> 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));
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 0cf2ef5

Please sign in to comment.