diff --git a/Public/Src/Engine/Processes/FileAccessReportingContext.cs b/Public/Src/Engine/Processes/FileAccessReportingContext.cs index b047dbbb74..2e79caa61a 100644 --- a/Public/Src/Engine/Processes/FileAccessReportingContext.cs +++ b/Public/Src/Engine/Processes/FileAccessReportingContext.cs @@ -80,32 +80,6 @@ public void ReportFileAccessDeniedByManifest(ReportedFileAccess unexpectedFileAc MatchAndReportUnexpectedFileAccess(unexpectedFileAccess); } - /// - /// Matches an instance of with allow list entries. - /// - public FileAccessAllowlist.MatchType MatchReportedFileAccess(ReportedFileAccess fileAccess) => - m_fileAccessAllowlist?.HasEntries == true - ? m_fileAccessAllowlist.Matches(m_loggingContext, fileAccess, m_pip) - : FileAccessAllowlist.MatchType.NoMatch; - - /// - /// Matches an instance of with allow list entries. - /// - public (FileAccessAllowlist.MatchType aggregateMatchType, (ReportedFileAccess, FileAccessAllowlist.MatchType)[] reportedMatchTypes) MatchObservedFileAccess(ObservedFileAccess observedFileAccess) - { - var aggregateMatch = FileAccessAllowlist.MatchType.MatchesAndCacheable; - var rfas = new (ReportedFileAccess, FileAccessAllowlist.MatchType)[observedFileAccess.Accesses.Count]; - int index = 0; - foreach (ReportedFileAccess reportedAccess in observedFileAccess.Accesses) - { - FileAccessAllowlist.MatchType thisMatch = MatchReportedFileAccess(reportedAccess); - rfas[index++] = (reportedAccess, thisMatch); - aggregateMatch = AggregateMatchType(aggregateMatch, thisMatch); - } - - return (aggregateMatch, rfas); - } - /// /// For an unexpected (which is actually an aggregation of es to /// a single path), reports each constituent access and computes an aggregate allowlist match type (the least permissive of any @@ -117,32 +91,26 @@ public FileAccessAllowlist.MatchType MatchAndReportUnexpectedObservedFileAccess( foreach (ReportedFileAccess reportedAccess in unexpectedObservedFileAccess.Accesses) { FileAccessAllowlist.MatchType thisMatch = MatchAndReportUnexpectedFileAccess(reportedAccess); - aggregateMatch = AggregateMatchType(aggregateMatch, thisMatch); - } - return aggregateMatch; - } - - private static FileAccessAllowlist.MatchType AggregateMatchType(FileAccessAllowlist.MatchType aggregateType, FileAccessAllowlist.MatchType currentType) - { - switch (currentType) - { - case FileAccessAllowlist.MatchType.NoMatch: - aggregateType = FileAccessAllowlist.MatchType.NoMatch; - break; - case FileAccessAllowlist.MatchType.MatchesButNotCacheable: - if (aggregateType == FileAccessAllowlist.MatchType.MatchesAndCacheable) - { - aggregateType = FileAccessAllowlist.MatchType.MatchesButNotCacheable; - } - - break; - default: - Contract.Assert(currentType == FileAccessAllowlist.MatchType.MatchesAndCacheable); - break; + switch (thisMatch) + { + case FileAccessAllowlist.MatchType.NoMatch: + aggregateMatch = FileAccessAllowlist.MatchType.NoMatch; + break; + case FileAccessAllowlist.MatchType.MatchesButNotCacheable: + if (aggregateMatch == FileAccessAllowlist.MatchType.MatchesAndCacheable) + { + aggregateMatch = FileAccessAllowlist.MatchType.MatchesButNotCacheable; + } + + break; + default: + Contract.Assert(thisMatch == FileAccessAllowlist.MatchType.MatchesAndCacheable); + break; + } } - return aggregateType; + return aggregateMatch; } /// @@ -150,50 +118,49 @@ private static FileAccessAllowlist.MatchType AggregateMatchType(FileAccessAllowl /// private FileAccessAllowlist.MatchType MatchAndReportUnexpectedFileAccess(ReportedFileAccess unexpectedFileAccess) { - FileAccessAllowlist.MatchType matchType = FileAccessAllowlist.MatchType.NoMatch; - if (m_fileAccessAllowlist != null && m_fileAccessAllowlist.HasEntries) { Contract.Assert( m_config.FailUnexpectedFileAccesses == false, "Having a file-access allowlist requires that Detours failure injection is off."); - matchType = m_fileAccessAllowlist.Matches(m_loggingContext, unexpectedFileAccess, m_pip); - } - - ReportFileAccess(unexpectedFileAccess, matchType); - return matchType; - } + FileAccessAllowlist.MatchType matchType = m_fileAccessAllowlist.Matches(m_loggingContext, unexpectedFileAccess, m_pip); + switch (matchType) + { + case FileAccessAllowlist.MatchType.NoMatch: + AddUnexpectedFileAccessNotAllowlisted(unexpectedFileAccess); + ReportUnexpectedFileAccessNotAllowlisted(unexpectedFileAccess); + break; + case FileAccessAllowlist.MatchType.MatchesButNotCacheable: + AddUnexpectedFileAccessAllowlisted(unexpectedFileAccess); + m_numAllowlistedButNotCacheableFileAccessViolations++; + ReportAllowlistedFileAccessNonCacheable(unexpectedFileAccess); + break; + case FileAccessAllowlist.MatchType.MatchesAndCacheable: + AddUnexpectedFileAccessAllowlisted(unexpectedFileAccess); + m_numAllowlistedAndCacheableFileAccessViolations++; + ReportAllowlistedFileAccessCacheable(unexpectedFileAccess); + break; + default: + throw Contract.AssertFailure("Unknown allowlist-match type."); + } - /// - /// Reports file access to this reporting context. - /// - public void ReportFileAccess(ReportedFileAccess fileAccess, FileAccessAllowlist.MatchType matchType) - { - switch (matchType) + return matchType; + } + else { - case FileAccessAllowlist.MatchType.NoMatch: - AddUnexpectedFileAccessNotAllowlisted(fileAccess); - ReportUnexpectedFileAccessNotAllowlisted(fileAccess); - break; - case FileAccessAllowlist.MatchType.MatchesButNotCacheable: - AddUnexpectedFileAccessAllowlisted(fileAccess); - m_numAllowlistedButNotCacheableFileAccessViolations++; - ReportAllowlistedFileAccessNonCacheable(fileAccess); - break; - case FileAccessAllowlist.MatchType.MatchesAndCacheable: - AddUnexpectedFileAccessAllowlisted(fileAccess); - m_numAllowlistedAndCacheableFileAccessViolations++; - ReportAllowlistedFileAccessCacheable(fileAccess); - break; - default: - throw Contract.AssertFailure("Unknown allowlist-match type."); + AddUnexpectedFileAccessNotAllowlisted(unexpectedFileAccess); + ReportUnexpectedFileAccessNotAllowlisted(unexpectedFileAccess); + return FileAccessAllowlist.MatchType.NoMatch; } } private void AddUnexpectedFileAccessNotAllowlisted(ReportedFileAccess reportedFileAccess) { - m_violations ??= new List(); + if (m_violations == null) + { + m_violations = new List(); + } if (reportedFileAccess.Operation != ReportedFileOperation.NtCreateFile || m_config.UnsafeSandboxConfiguration.MonitorNtCreateFile) { @@ -208,7 +175,10 @@ private void AddUnexpectedFileAccessNotAllowlisted(ReportedFileAccess reportedFi private void AddUnexpectedFileAccessAllowlisted(ReportedFileAccess reportedFileAccess) { - m_allowlistedAccesses ??= new List(); + if (m_allowlistedAccesses == null) + { + m_allowlistedAccesses = new List(); + } if (reportedFileAccess.Operation != ReportedFileOperation.NtCreateFile || m_config.UnsafeSandboxConfiguration.MonitorNtCreateFile) { @@ -223,7 +193,7 @@ private void ReportUnexpectedFileAccessNotAllowlisted(ReportedFileAccess reporte if (path.StartsWith(PipEnvironment.RestrictedTemp, OperatingSystemHelper.PathComparison)) { - Tracing.Logger.Log.PipProcessDisallowedTempFileAccess( + BuildXL.Processes.Tracing.Logger.Log.PipProcessDisallowedTempFileAccess( m_loggingContext, m_pip.SemiStableHash, m_pip.GetDescription(m_context), @@ -232,7 +202,7 @@ private void ReportUnexpectedFileAccessNotAllowlisted(ReportedFileAccess reporte } else { - Tracing.Logger.Log.PipProcessDisallowedFileAccess( + BuildXL.Processes.Tracing.Logger.Log.PipProcessDisallowedFileAccess( m_loggingContext, m_pip.SemiStableHash, m_pip.GetDescription(m_context), @@ -242,13 +212,13 @@ private void ReportUnexpectedFileAccessNotAllowlisted(ReportedFileAccess reporte path); if (reportedFileAccess.Operation == ReportedFileOperation.NtCreateFile && - !m_config.UnsafeSandboxConfiguration.MonitorNtCreateFile) + !m_config.UnsafeSandboxConfiguration.MonitorNtCreateFile) { // If the unsafe_IgnoreNtCreate is set, disallowed ntCreateFile accesses are not marked as violations. // Since there will be no error or warning for the ignored NtCreateFile violations in the FileMonitoringViolationAnalyzer, // this is the only place for us to log a warning for those. // We also need to emit a dx09 verbose above for those violations due to WrapItUp. - Tracing.Logger.Log.PipProcessDisallowedNtCreateFileAccessWarning( + BuildXL.Processes.Tracing.Logger.Log.PipProcessDisallowedNtCreateFileAccessWarning( m_loggingContext, m_pip.SemiStableHash, m_pip.GetDescription(m_context), @@ -267,7 +237,7 @@ private void ReportAllowlistedFileAccessNonCacheable(ReportedFileAccess reported if (m_reportAllowlistedAccesses) { - Tracing.Logger.Log.PipProcessUncacheableAllowlistNotAllowedInDistributedBuilds( + BuildXL.Processes.Tracing.Logger.Log.PipProcessUncacheableAllowlistNotAllowedInDistributedBuilds( m_loggingContext, m_pip.SemiStableHash, m_pip.GetDescription(m_context), @@ -278,7 +248,7 @@ private void ReportAllowlistedFileAccessNonCacheable(ReportedFileAccess reported } else { - Tracing.Logger.Log.PipProcessDisallowedFileAccessAllowlistedNonCacheable( + BuildXL.Processes.Tracing.Logger.Log.PipProcessDisallowedFileAccessAllowlistedNonCacheable( m_loggingContext, m_pip.SemiStableHash, m_pip.GetDescription(m_context), @@ -292,7 +262,7 @@ private void ReportAllowlistedFileAccessCacheable(ReportedFileAccess reportedFil string path = reportedFileAccess.GetPath(m_context.PathTable); string description = reportedFileAccess.Describe(); - Tracing.Logger.Log.PipProcessDisallowedFileAccessAllowlistedCacheable( + BuildXL.Processes.Tracing.Logger.Log.PipProcessDisallowedFileAccessAllowlistedCacheable( m_loggingContext, m_pip.SemiStableHash, m_pip.GetDescription(m_context), diff --git a/Public/Src/Engine/Processes/SandboxedProcessPipExecutor.cs b/Public/Src/Engine/Processes/SandboxedProcessPipExecutor.cs index b79e9535ad..5fb6efcc38 100644 --- a/Public/Src/Engine/Processes/SandboxedProcessPipExecutor.cs +++ b/Public/Src/Engine/Processes/SandboxedProcessPipExecutor.cs @@ -1686,24 +1686,14 @@ private async Task ProcessSandboxedProcessRe JobObject.AccountingInformation? jobAccounting = result.JobAccountingInformation; var stopwatch = System.Diagnostics.Stopwatch.StartNew(); - - var fileAccessReportingContext = new FileAccessReportingContext( - loggingContext, - m_context, - m_sandboxConfig, - m_pip, - m_validateDistribution, - m_fileAccessAllowlist); - // If this operation fails, error was logged already bool sharedOpaqueProcessingSuccess = TryGetObservedFileAccesses( - fileAccessReportingContext, - result, - allInputPathsUnderSharedOpaques, - out var unobservedOutputs, - out var sharedDynamicDirectoryWriteAccesses, - out SortedReadOnlyArray observed, - out IReadOnlySet createdDirectories); + result, + allInputPathsUnderSharedOpaques, + out var unobservedOutputs, + out var sharedDynamicDirectoryWriteAccesses, + out SortedReadOnlyArray observed, + out IReadOnlySet createdDirectories); LogSubPhaseDuration(m_loggingContext, m_pip, SandboxedProcessCounters.SandboxedPipExecutorPhaseGettingObservedFileAccesses, stopwatch.Elapsed, $"(count: {observed.Length})"); @@ -1818,6 +1808,14 @@ private async Task ProcessSandboxedProcessRe stopwatch.Restart(); + var fileAccessReportingContext = new FileAccessReportingContext( + loggingContext, + m_context, + m_sandboxConfig, + m_pip, + m_validateDistribution, + m_fileAccessAllowlist); + // Note that when MonitorFileAccesses == false, we should not assume the various reported-access sets are non-null. if (MonitorFileAccesses) { @@ -2758,13 +2756,7 @@ private void AddDirectoryAncestorsToManifest(AbsolutePath path, HashSet } } - private void AddStaticFileDependencyToFileAccessManifest(FileArtifact dependency, HashSet allInputPathsUnderSharedOpaques) + private void AddStaticFileDependencyToFileAccessManifest(FileArtifact dependency, HashSet allInputPaths) { // TODO:22476: We allow inputs to not exist. Is that the right thing to do? // We mask 'report' here, since inputs are expected read and should never fail observed-access validation (directory dependency, etc.) @@ -2854,14 +2840,15 @@ private void AddStaticFileDependencyToFileAccessManifest(FileArtifact dependency FileAccessPolicy.MaskNothing)); m_remoteSbDataBuilder?.AddFileDependency(dependency.Path); + allInputPaths.Add(path); + // If the file artifact is under the root of a shared opaque we make sure all the directories // walking that path upwards get added to the manifest explicitly, so timestamp faking happens for them // We need the outmost matching root in case shared opaques are nested within each other: timestamp faking // needs to happen for all directories under all shared opaques if (pathIsUnderSharedOpaque) { - allInputPathsUnderSharedOpaques.Add(path); - AddDirectoryAncestorsToManifest(path, allInputPathsUnderSharedOpaques, sharedOpaqueRoot); + AddDirectoryAncestorsToManifest(path, allInputPaths, sharedOpaqueRoot); } } @@ -3905,7 +3892,6 @@ private IEnumerable GetEnumeratedFileAccessesForIncrementalT /// /// Whether the operation succeeded. This operation may fail only in regards to shared dynamic write access processing. private bool TryGetObservedFileAccesses( - FileAccessReportingContext fileAccessReportingContext, SandboxedProcessResult result, HashSet allInputPathsUnderSharedOpaques, out List unobservedOutputs, @@ -4103,23 +4089,13 @@ private bool TryGetObservedFileAccesses( // if the path is still a candidate to be part of a shared opaque, that means there was at least a write to that path. If the path is then // in the cone of a shared opaque, then it is a dynamic write access bool? isAccessUnderASharedOpaque = null; - if (isPathCandidateToBeOwnedByASharedOpaque && - IsAccessUnderASharedOpaque(firstAccess, dynamicWriteAccesses, out AbsolutePath sharedDynamicDirectoryRoot)) + if (isPathCandidateToBeOwnedByASharedOpaque && IsAccessUnderASharedOpaque( + firstAccess, + dynamicWriteAccesses, + out AbsolutePath sharedDynamicDirectoryRoot)) { - bool shouldBeConsideredAsOutput = ShouldBeConsideredSharedOpaqueOutput(fileAccessReportingContext, firstAccess, out FileAccessAllowlist.MatchType matchType); - - if (matchType != FileAccessAllowlist.MatchType.NoMatch) - { - // If the match is cacheable/uncacheable, report the access so that pip executor knows if the pip can be cached or not. - fileAccessReportingContext.ReportFileAccess(firstAccess, matchType); - } - - if (shouldBeConsideredAsOutput) - { - dynamicWriteAccesses[sharedDynamicDirectoryRoot].Add(entry.Key); - isAccessUnderASharedOpaque = true; - } - + dynamicWriteAccesses[sharedDynamicDirectoryRoot].Add(entry.Key); + isAccessUnderASharedOpaque = true; // This is a known output, so don't store it continue; } @@ -4399,6 +4375,7 @@ private bool IsAccessUnderASharedOpaque( // TODO: consider adding a cache from manifest paths to containing shared opaques. It is likely // that many writes for a given pip happens under the same cones. + foreach (var currentNode in m_context.PathTable.EnumerateHierarchyBottomUp(initialNode)) { var currentPath = new AbsolutePath(currentNode); @@ -4413,29 +4390,6 @@ private bool IsAccessUnderASharedOpaque( return false; } - /// - /// Checks whether a reported file access should be considered as an output under a shared opaque. - /// - private bool ShouldBeConsideredSharedOpaqueOutput( - FileAccessReportingContext fileAccessReportingContext, - ReportedFileAccess access, - out FileAccessAllowlist.MatchType matchType) - { - // Given a file access f under a shared opaque. - // - NoMatch => true - // - MatchCacheable / NotCacheable - // - case 1: f is static source/output => true - // - case 2: f is under exclusive opaque => true - // - otherwise: false - // - // In case 1 & 2 above, f is considered an output so that the pip executor can detect for double writes. - matchType = fileAccessReportingContext.MatchReportedFileAccess(access); - return matchType == FileAccessAllowlist.MatchType.NoMatch - || (access.TryParseAbsolutePath(m_context, m_loggingContext, m_pip, out AbsolutePath accessPath) - && (m_pipGraphFileSystemView.TryGetLatestFileArtifactForPath(accessPath).IsValid - || (m_pipGraphFileSystemView.IsPathUnderOutputDirectory(accessPath, out bool isSharedOpaque) && !isSharedOpaque))); - } - /// /// Checks if a path starts with a prefix, given the fact that the path may start with "\??\" or "\\?\". /// diff --git a/Public/Src/Engine/Scheduler/Fingerprints/ObservedInputProcessor.cs b/Public/Src/Engine/Scheduler/Fingerprints/ObservedInputProcessor.cs index f2ecbaeb27..ea80ef81f2 100644 --- a/Public/Src/Engine/Scheduler/Fingerprints/ObservedInputProcessor.cs +++ b/Public/Src/Engine/Scheduler/Fingerprints/ObservedInputProcessor.cs @@ -439,18 +439,18 @@ internal static async Task ProcessInternalAsync ProcessInternalAsync /// the access being allowed but we want to report it differently. ObservedInputAccessCheckFailureAction OnAccessCheckFailure(TObservation observation, bool fromTopLevelDirectory); - /// - /// Actions to perform when BuildXL allows undeclared access. - /// - ObservedInputAccessCheckFailureAction OnAllowingUndeclaredAccessCheck(TObservation observation); - void ReportUnexpectedAccess(TObservation observation, ObservedInputType observedInputType); bool IsReportableUnexpectedAccess(AbsolutePath path); @@ -1836,7 +1820,7 @@ internal DirectoryEnumerationMode DetermineEnumerationModeAndRule(AbsolutePath d /// internal bool MayDetermineMinimalGraphWithAlienFiles(bool allowUndeclaredSourceReads) { - return m_env.Configuration.Sandbox.FileSystemMode == FileSystemMode.AlwaysMinimalGraph || allowUndeclaredSourceReads; + return m_env.Configuration.Sandbox.FileSystemMode == BuildXL.Utilities.Configuration.FileSystemMode.AlwaysMinimalGraph || allowUndeclaredSourceReads; } /// diff --git a/Public/Src/Engine/Scheduler/PipExecutor.cs b/Public/Src/Engine/Scheduler/PipExecutor.cs index 1fc5b5025b..054f21f0cd 100644 --- a/Public/Src/Engine/Scheduler/PipExecutor.cs +++ b/Public/Src/Engine/Scheduler/PipExecutor.cs @@ -3823,7 +3823,10 @@ public TwoPhasePathSetValidationTarget(IPipExecutionEnvironment environment, Ope public string Description => m_pipDescription; - public AbsolutePath GetPathOfObservation(ObservedPathEntry assertion) => assertion.Path; + public AbsolutePath GetPathOfObservation(ObservedPathEntry assertion) + { + return assertion.Path; + } public ObservationFlags GetObservationFlags(ObservedPathEntry assertion) { @@ -3847,19 +3850,28 @@ public ObservedInputAccessCheckFailureAction OnAccessCheckFailure(ObservedPathEn public void CheckProposedObservedInput(ObservedPathEntry assertion, ObservedInput proposedObservedInput) { + return; } - public bool IsSearchPathEnumeration(ObservedPathEntry directoryEnumeration) => directoryEnumeration.IsSearchPath; + public bool IsSearchPathEnumeration(ObservedPathEntry directoryEnumeration) + { + return directoryEnumeration.IsSearchPath; + } - public string GetEnumeratePatternRegex(ObservedPathEntry directoryEnumeration) => directoryEnumeration.EnumeratePatternRegex; + public string GetEnumeratePatternRegex(ObservedPathEntry directoryEnumeration) + { + return directoryEnumeration.EnumeratePatternRegex; + } public void ReportUnexpectedAccess(ObservedPathEntry assertion, ObservedInputType observedInputType) { + // noop } - public bool IsReportableUnexpectedAccess(AbsolutePath path) => false; - - public ObservedInputAccessCheckFailureAction OnAllowingUndeclaredAccessCheck(ObservedPathEntry observation) => ObservedInputAccessCheckFailureAction.Fail; + public bool IsReportableUnexpectedAccess(AbsolutePath path) + { + return false; + } } private readonly struct ObservedFileAccessValidationTarget : IObservedInputProcessingTarget @@ -3886,19 +3898,30 @@ public ObservedFileAccessValidationTarget( m_state = state; } - public ObservationFlags GetObservationFlags(ObservedFileAccess observation) => observation.ObservationFlags; + public ObservationFlags GetObservationFlags(ObservedFileAccess observation) + { + return observation.ObservationFlags; + } - public AbsolutePath GetPathOfObservation(ObservedFileAccess observation) => observation.Path; + public AbsolutePath GetPathOfObservation(ObservedFileAccess observation) + { + return observation.Path; + } public void CheckProposedObservedInput(ObservedFileAccess observation, ObservedInput proposedObservedInput) { + return; } public void ReportUnexpectedAccess(ObservedFileAccess observation, ObservedInputType observedInputType) { + // noop } - public bool IsReportableUnexpectedAccess(AbsolutePath path) => false; + public bool IsReportableUnexpectedAccess(AbsolutePath path) + { + return false; + } public ObservedInputAccessCheckFailureAction OnAccessCheckFailure(ObservedFileAccess observation, bool fromTopLevelDirectory) { @@ -3982,23 +4005,6 @@ public string GetEnumeratePatternRegex(ObservedFileAccess directoryEnumeration) return RegexDirectoryMembershipFilter.ConvertWildcardsToRegex(set.OrderBy(m => m, StringComparer.OrdinalIgnoreCase).ToArray()); } } - - public ObservedInputAccessCheckFailureAction OnAllowingUndeclaredAccessCheck(ObservedFileAccess observation) - { - (FileAccessAllowlist.MatchType aggregateMatchType, (ReportedFileAccess, FileAccessAllowlist.MatchType)[] reportedMatchTypes) = m_fileAccessReportingContext.MatchObservedFileAccess(observation); - if (aggregateMatchType != FileAccessAllowlist.MatchType.NoMatch) - { - // Report cacheable/uncachable so that pip executor can decide if the pip is cacheable or uncacheable. - foreach ((ReportedFileAccess fa, FileAccessAllowlist.MatchType matchType) in reportedMatchTypes.Where(t => t.Item2 != FileAccessAllowlist.MatchType.NoMatch)) - { - m_fileAccessReportingContext.ReportFileAccess(fa, matchType); - } - - return ObservedInputAccessCheckFailureAction.SuppressAndIgnorePath; - } - - return ObservedInputAccessCheckFailureAction.Fail; - } } /// diff --git a/Public/Src/Engine/UnitTests/Scheduler.IntegrationTest/AllowedUndeclaredReadsTests.cs b/Public/Src/Engine/UnitTests/Scheduler.IntegrationTest/AllowedUndeclaredReadsTests.cs index bc6415b2d3..7503fdda09 100644 --- a/Public/Src/Engine/UnitTests/Scheduler.IntegrationTest/AllowedUndeclaredReadsTests.cs +++ b/Public/Src/Engine/UnitTests/Scheduler.IntegrationTest/AllowedUndeclaredReadsTests.cs @@ -67,70 +67,6 @@ public void UndeclaredReadsCachingBehavior() RunScheduler().AssertCacheHit(pip.Process.PipId); } - public enum AllowListKind - { - None, - Cacheable, - UnCacheable - } - - [Theory] - [InlineData(AllowListKind.None)] - [InlineData(AllowListKind.Cacheable)] - [InlineData(AllowListKind.UnCacheable)] - public void UndeclaredReadsWithAllowList(AllowListKind allowListKind) - { - FileArtifact source = CreateSourceFile(); - if (allowListKind != AllowListKind.None) - { - var entry = new FileAccessAllowlistEntry - { - Name = nameof(UndeclaredReadsWithAllowList), - ToolPath = new DiscriminatingUnion(TestProcessExecutable.Path.GetName(Context.PathTable)), - PathFragment = ArtifactToString(source), - }; - - if (allowListKind == AllowListKind.Cacheable) - { - Configuration.CacheableFileAccessAllowlist.Add(entry); - } - else - { - Configuration.FileAccessAllowList.Add(entry); - } - } - - ProcessWithOutputs pip = ScheduleProcessWithUndeclaredReads(source); - RunScheduler().AssertSuccess(); - ScheduleRunResult secondRun = RunScheduler().AssertSuccess(); - - if (allowListKind != AllowListKind.UnCacheable) - { - secondRun.AssertCacheHit(pip.Process.PipId); - } - else - { - secondRun.AssertCacheMiss(pip.Process.PipId); - } - - WriteSourceFile(source); - ScheduleRunResult thirdRun = RunScheduler().AssertSuccess(); - - if (allowListKind == AllowListKind.Cacheable) - { - thirdRun.AssertCacheHit(pip.Process.PipId); - } - else - { - thirdRun.AssertCacheMiss(pip.Process.PipId); - } - - if (allowListKind == AllowListKind.UnCacheable) - { - AssertWarningEventLogged(LogEventId.ProcessNotStoredToCacheDueToFileMonitoringViolations, allowMore: true); - } - } - [Fact] public void PresentProbeCachingBehavior() { @@ -511,7 +447,8 @@ public void WritingUntrackedUndeclaredInputsUnderSharedOpaquesAreAllowed() // TODO: fix this test case for Linux where writes under a shared opaque are DFAs. Work item #1984802 [TheoryIfSupported(requiresWindowsBasedOperatingSystem: true)] - [MemberData(nameof(TruthTable.GetTable), 1, MemberType = typeof(TruthTable))] + [InlineData(true)] + [InlineData(false)] public void WritingToExistentFileProducedBySamePipIsAllowed(bool varyPath) { // Run a pip that writes into a file twice: the second time, the file will exist. However, this should be allowed. diff --git a/Public/Src/Engine/UnitTests/Scheduler.IntegrationTest/SharedOpaqueDirectoryTests.cs b/Public/Src/Engine/UnitTests/Scheduler.IntegrationTest/SharedOpaqueDirectoryTests.cs index dc708e3278..1c17ce9a51 100644 --- a/Public/Src/Engine/UnitTests/Scheduler.IntegrationTest/SharedOpaqueDirectoryTests.cs +++ b/Public/Src/Engine/UnitTests/Scheduler.IntegrationTest/SharedOpaqueDirectoryTests.cs @@ -2887,149 +2887,6 @@ public void DirectoryProbeWithNoProducedFilesUnderneathReplayConsistently() RunScheduler().AssertSuccess().AssertCacheHit(proberPip.Process.PipId, dirCreatorPip.Process.PipId); } - public enum AllowListKind - { - None, - Cacheable, - UnCacheable - } - - [Theory] - [InlineData(AllowListKind.None)] - [InlineData(AllowListKind.Cacheable)] - [InlineData(AllowListKind.UnCacheable)] - public void DoubleWritesWithAllowList(AllowListKind allowListKind) - { - string sharedOpaqueDir = Path.Combine(ObjectRoot, "sod"); - AbsolutePath sharedOpaqueDirPath = AbsolutePath.Create(Context.PathTable, sharedOpaqueDir); - FileArtifact sharedOutput = FileArtifact.CreateOutputFile(sharedOpaqueDirPath.Combine(Context.PathTable, "shared.out")); - FileArtifact source = CreateSourceFile(ObjectRoot); - WriteSourceFile(source); - FileArtifact outputP = CreateOutputFileArtifact(sharedOpaqueDir); - FileArtifact outputQ = CreateOutputFileArtifact(sharedOpaqueDir); - - if (allowListKind != AllowListKind.None) - { - var entry = new global::BuildXL.Utilities.Configuration.Mutable.FileAccessAllowlistEntry - { - Name = nameof(DoubleWritesWithAllowList), - ToolPath = new DiscriminatingUnion(TestProcessExecutable.Path.GetName(Context.PathTable)), - PathFragment = ArtifactToString(sharedOutput), - }; - - if (allowListKind == AllowListKind.Cacheable) - { - Configuration.CacheableFileAccessAllowlist.Add(entry); - } - else - { - Configuration.FileAccessAllowList.Add(entry); - } - } - - ProcessBuilder pBuilder = CreatePipBuilder(new Operation[] - { - Operation.ReadFile(source), - Operation.WriteFile(outputP, doNotInfer: true), - Operation.WriteFile(sharedOutput, doNotInfer: true) - }); - pBuilder.AddOutputDirectory(sharedOpaqueDirPath, SealDirectoryKind.SharedOpaque); - pBuilder.RewritePolicy |= RewritePolicy.DefaultSafe; - ProcessWithOutputs p = SchedulePipBuilder(pBuilder); - - ProcessBuilder qBuilder = CreatePipBuilder(new Operation[] - { - Operation.ReadFile(outputP, doNotInfer: true), - Operation.WriteFile(outputQ, doNotInfer: true), - Operation.WriteFile(sharedOutput, doNotInfer: true) - }); - qBuilder.AddInputDirectory(p.ProcessOutputs.GetOpaqueDirectory(sharedOpaqueDirPath)); - qBuilder.AddOutputDirectory(sharedOpaqueDirPath, SealDirectoryKind.SharedOpaque); - qBuilder.RewritePolicy |= RewritePolicy.DefaultSafe; - ProcessWithOutputs q = SchedulePipBuilder(qBuilder); - - if (allowListKind == AllowListKind.None) - { - RunScheduler().AssertFailure(); - AssertVerboseEventLogged(LogEventId.DependencyViolationDoubleWrite); - AssertErrorEventLogged(LogEventId.FileMonitoringError); - } - else - { - RunScheduler().AssertSuccess(); - File.Delete(ArtifactToString(sharedOutput)); - - var secondRunResult = RunScheduler().AssertSuccess(); - if (allowListKind == AllowListKind.Cacheable) - { - secondRunResult.AssertCacheHit(p.Process.PipId, q.Process.PipId); - XAssert.IsFalse(File.Exists(ArtifactToString(sharedOutput))); - } - else - { - secondRunResult.AssertCacheMiss(p.Process.PipId, q.Process.PipId); - XAssert.IsTrue(File.Exists(ArtifactToString(sharedOutput))); - AssertWarningEventLogged(LogEventId.ProcessNotStoredToCacheDueToFileMonitoringViolations, allowMore: true); - } - } - } - - [Theory] - [MemberData(nameof(TruthTable.GetTable), 1, MemberType = typeof(TruthTable))] - public void DoubleWritesOnStaticallyDeclaredOutputsWithAllowList(bool onExclusiveOpaque) - { - string sharedOpaqueDir = Path.Combine(ObjectRoot, "sod"); - AbsolutePath sharedOpaqueDirPath = AbsolutePath.Create(Context.PathTable, sharedOpaqueDir); - - AbsolutePath nestedUnderSharedOpaqueDirPath = sharedOpaqueDirPath.Combine(Context.PathTable, "nested"); - FileArtifact sharedOutput = FileArtifact.CreateOutputFile(nestedUnderSharedOpaqueDirPath.Combine(Context.PathTable, "shared.out")); - FileArtifact source = CreateSourceFile(ObjectRoot); - WriteSourceFile(source); - - FileArtifact outputP = CreateOutputFileArtifact(sharedOpaqueDir); - FileArtifact outputQ = CreateOutputFileArtifact(sharedOpaqueDir); - - Configuration.CacheableFileAccessAllowlist.Add(new global::BuildXL.Utilities.Configuration.Mutable.FileAccessAllowlistEntry - { - Name = nameof(DoubleWritesOnStaticallyDeclaredOutputsWithAllowList), - ToolPath = new DiscriminatingUnion(TestProcessExecutable.Path.GetName(Context.PathTable)), - PathFragment = ArtifactToString(sharedOutput), - }); - - ProcessBuilder pBuilder = CreatePipBuilder(new Operation[] - { - Operation.ReadFile(source), - Operation.WriteFile(outputP), - Operation.WriteFile(sharedOutput, doNotInfer: onExclusiveOpaque) - }); - if (onExclusiveOpaque) - { - pBuilder.AddOutputDirectory(nestedUnderSharedOpaqueDirPath, SealDirectoryKind.Opaque); - } - - pBuilder.RewritePolicy |= RewritePolicy.DefaultSafe; - ProcessWithOutputs p = SchedulePipBuilder(pBuilder); - - ProcessBuilder qBuilder = CreatePipBuilder(new Operation[] - { - Operation.ReadFile(outputP), - Operation.WriteFile(outputQ), - Operation.WriteFile(sharedOutput, doNotInfer: true) - }); - qBuilder.AddOutputDirectory(sharedOpaqueDirPath, SealDirectoryKind.SharedOpaque); - qBuilder.RewritePolicy |= RewritePolicy.DefaultSafe; - ProcessWithOutputs q = SchedulePipBuilder(qBuilder); - - RunScheduler().AssertFailure(); - AssertVerboseEventLogged(LogEventId.DependencyViolationDoubleWrite); - AssertErrorEventLogged(LogEventId.FileMonitoringError); - - if (!onExclusiveOpaque) - { - AssertWarningEventLogged(LogEventId.AllowSameContentPolicyNotAvailableForStaticallyDeclaredOutputs); - } - } - private string ToString(AbsolutePath path) => path.ToString(Context.PathTable); } } diff --git a/Public/Src/Engine/UnitTests/Scheduler/ObservedInputProcessorTests.cs b/Public/Src/Engine/UnitTests/Scheduler/ObservedInputProcessorTests.cs index 2b791786bc..1b919fb768 100644 --- a/Public/Src/Engine/UnitTests/Scheduler/ObservedInputProcessorTests.cs +++ b/Public/Src/Engine/UnitTests/Scheduler/ObservedInputProcessorTests.cs @@ -244,6 +244,7 @@ public AbsolutePath GetPathOfObservation(TestObservation observation) public ObservationFlags GetObservationFlags(TestObservation observation) { + var str = observation.Path.ToString(Context.PathTable); if (observation.IsDirectoryEnumeration) { return ObservationFlags.Enumeration; @@ -357,11 +358,6 @@ public bool IsReportableUnexpectedAccess(AbsolutePath path) { return true; } - - public ObservedInputAccessCheckFailureAction OnAllowingUndeclaredAccessCheck(TestObservation observation) - { - return ObservedInputAccessCheckFailureAction.Fail; - } } private sealed class Harness