Add check to not private builds to public channels

eng/promote-build.yml

@@ -51,6 +51,14 @@ parameters:
   type: string
   default: ' '
 
+# TODO https://github.com/dotnet/arcade/issues/14167 
+# The default value of this parameter is temporarely being set to true to avoid breaking builds.
+# In the future, the value of this parameter will be provided by darc, and will be set to true for only builds that are not really internal, like wpf-int
+- name: SkipInternalAssetToPublicFeedCheck
+  displayName: Skip the check that ensures internal assets are not published to public feeds
+  type: boolean
+  default: true
+
 trigger: none
 resources:
   repositories:
@@ -78,4 +86,5 @@ extends:
         PromoteToChannelIds: ${{ parameters.PromoteToChannelIds }}
         BARBuildId: ${{ parameters.BARBuildId }}
         symbolPublishingAdditionalParameters: ${{ parameters.SymbolPublishingAdditionalParameters }}
-        artifactsPublishingAdditionalParameters: ${{ parameters.ArtifactsPublishingAdditionalParameters }}
+        artifactsPublishingAdditionalParameters: ${{ parameters.ArtifactsPublishingAdditionalParameters }}
+        SkipInternalAssetToPublicFeedCheck: ${{ parameters.SkipInternalAssetToPublicFeedCheck }}

eng/publishing/v3/publish.yml

@@ -4,6 +4,8 @@ parameters:
   BARBuildId: ''
   symbolPublishingAdditionalParameters: ''
   buildQuality: 'daily'
+  SkipInternalAssetToPublicFeedCheck: true
+
 stages:
 - stage: publish
   displayName: Publishing
@@ -97,7 +99,7 @@ stages:
           /p:PublishingInfraVersion=3
           /p:BARBuildId=${{ parameters.BARBuildId }}
           /p:TargetChannels='${{ parameters.PromoteToChannelIds }}'
-          /p:IsInternalBuild=${{ contains(variables['AzDOBranch'], 'internal/') }}
+          /p:IsInternalBuild=$(IsInternalBuild)
           /p:NugetPath=$(NuGetExeToolPath)
           /p:MaestroApiEndpoint='$(MaestroApiEndPoint)'
           /p:BuildAssetRegistryToken='$(AuthenticateMaestro.MaestroToken)'
@@ -123,7 +125,8 @@ stages:
           /p:AzureProject='$(AzDOProject)'
           /p:UseStreamingPublishing='true'
           /p:StreamingPublishingMaxClients=16
-          /p:NonStreamingPublishingMaxClients=12
+          /p:NonStreamingPublishingMaxClients=12       
+          /p:SkipInternalAssetToPublicFeedCheck='$(SkipInternalAssetToPublicFeedCheck)'
 
     - template: /eng/common/templates-official/steps/publish-logs.yml@self
       parameters:

eng/publishing/v3/validate-and-locate-build.ps1

@@ -60,6 +60,15 @@ try {
     $buildNumberName = $buildNumberName.Substring(0, 255)
   }
 
+  $isInternalBuild = $true
+  if ([string]::IsNullOrEmpty($buildInfo.gitHubRepository) -eq $false) {
+    $buildInfo.gitHubRepository -match "https://github.com/(.*)/(.*)" | Out-Null
+    $response = Invoke-WebRequest -Uri "https://api.github.com/repos/$($Matches[1])/$($Matches[2]))/commits/$($buildInfo.commit)"
+    if ($response.StatusCode -eq 200) {
+      $isInternalBuild = $false
+      Write-Host "This is a public build"
+    }
+  }
   # Set tags on publishing for visibility
   Write-Host "##vso[build.updatebuildnumber]$buildNumberName"
   Write-Host "##vso[build.addbuildtag]Channel(s) - $channelNames"
@@ -71,6 +80,7 @@ try {
   Write-Host "##vso[task.setvariable variable=AzDOBuildId]$($buildInfo.azureDevOpsBuildId)"
   Write-Host "##vso[task.setvariable variable=AzDOAccount]$($buildInfo.azureDevOpsAccount)"
   Write-Host "##vso[task.setvariable variable=AzDOBranch]$($buildInfo.azureDevOpsBranch)"
+  Write-Host "##vso[task.setvariable variable=IsInternalBuild]$isInternalBuild"
 }
 catch {
   Write-Host $_

src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishArtifactsInManifestTests.cs

@@ -95,7 +95,7 @@ public async Task PushNugetPackageTestsAsync(int pushAttemptsBeforeSuccess, bool
             // Functionality is the same as this is in the base class, create a v2 object to test. 
             var task = new PublishArtifactsInManifestV3
             {
-                InternalBuild = true,
+                IsInternalBuild = true,
                 BuildEngine = buildEngine,
                 NugetPath = fakeNugetExeName,
                 MaxRetryCount = 5, // In case the default changes, lock to 5 so the test data works

src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifest.cs

@@ -43,7 +43,6 @@ public class PublishArtifactsInManifest : MSBuildTaskBase
         /// Metadata Internal (optional): If true, the feed is only internally accessible.
         ///                               If false, the feed is publicly visible and internal builds wwill be rejected.
         ///                               If not provided, then this task will attempt to determine whether the feed URL is publicly visible or not.
-        ///                               Unless SkipSafetyChecks is passed, the publishing infrastructure will check the accessibility of the feed.
         /// Metadata Isolated (optional): If true, stable packages can be pushed to this feed.
         ///                               If false, stable packages will be rejected.
         ///                               If not provided then defaults to false.
@@ -122,7 +121,7 @@ public class PublishArtifactsInManifest : MSBuildTaskBase
         /// Whether this build is internal or not. If true, extra checks are done to avoid accidental
         /// publishing of assets to public feeds or storage accounts.
         /// </summary>
-        public bool InternalBuild { get; set; }
+        public bool IsInternalBuild { get; set; }
 
         public bool PublishInstallersAndChecksums { get; set; } = false;
 
@@ -159,11 +158,14 @@ public class PublishArtifactsInManifest : MSBuildTaskBase
         public bool PublishSpecialClrFiles { get; set; }
 
         /// <summary>
-        /// If true, safety checks only print messages and do not error
-        /// - Internal asset to public feed
-        /// - Stable packages to non-isolated feeds
+        /// If true, allows publishing of a stable package to a non isolated feed
         /// </summary>
-        public bool SkipSafetyChecks { get; set; } = false;
+        public bool SkipStablePackagesNonIsolatedFeedsCheck { get; set; } = false;
+
+        /// <summary>
+        /// If true, allows publishing of internal assets to public feeds
+        /// </summary>
+        public bool SkipInternalAssetToPublicFeedCheck { get; set; } = false;
 
         public string AkaMSClientId { get; set; }
 
@@ -347,8 +349,9 @@ internal PublishArtifactsInManifestBase ConstructPublishingV3Task(BuildModel bui
                 MaestroApiEndpoint = this.MaestroApiEndpoint,
                 BuildAssetRegistryToken = this.BuildAssetRegistryToken,
                 NugetPath = this.NugetPath,
-                InternalBuild = this.InternalBuild,
-                SkipSafetyChecks = this.SkipSafetyChecks,
+                IsInternalBuild = this.IsInternalBuild,
+                SkipInternalAssetToPublicFeedCheck = this.SkipInternalAssetToPublicFeedCheck,
+                SkipStablePackagesNonIsolatedFeedsCheck = this.SkipStablePackagesNonIsolatedFeedsCheck,
                 AkaMSClientId = this.AkaMSClientId,
                 AkaMSClientCertificate = !string.IsNullOrEmpty(AkaMSClientCertificate) ?
 #pragma warning disable SYSLIB0057 // https://github.com/dotnet/arcade/issues/14936

src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs

@@ -118,14 +118,17 @@ public abstract class PublishArtifactsInManifestBase : Microsoft.Build.Utilities
         /// Whether this build is internal or not. If true, extra checks are done to avoid accidental
         /// publishing of assets to public feeds or storage accounts.
         /// </summary>
-        public bool InternalBuild { get; set; } = false;
+        public bool IsInternalBuild { get; set; } = false;
 
         /// <summary>
-        /// If true, safety checks only print messages and do not error
-        /// - Internal asset to public feed
-        /// - Stable packages to non-isolated feeds
+        /// If true, allows publishing of a stable package to a non isolated feed
         /// </summary>
-        public bool SkipSafetyChecks { get; set; } = false;
+        public bool SkipStablePackagesNonIsolatedFeedsCheck { get; set; } = false;
+
+        /// <summary>
+        /// If true, allows publishing of internal assets to public feeds
+        /// </summary>
+        public bool SkipInternalAssetToPublicFeedCheck { get; set; } = false;
 
         /// <summary>
         /// Which build model (i.e., parsed build manifest) the publishing task will operate on.
@@ -349,17 +352,21 @@ protected async Task PersistPendingAssetLocationAsync(IMaestroApi client)
         }
 
         /// <summary>
-        /// Protect against accidental publishing of internal assets to non-internal feeds.
+        /// Run a check to verify that we're not publishing an internal build to non-internal feeds.
         /// </summary>
-        protected void CheckForInternalBuildsOnPublicFeeds(TargetFeedConfig feedConfig)
+        protected void CheckForInternalBuildsOnPublicFeeds()
         {
             // If separated out for clarity.
-            if (!SkipSafetyChecks)
+            if (!SkipInternalAssetToPublicFeedCheck && IsInternalBuild)
             {
-                if (InternalBuild && !feedConfig.Internal)
+                var publicFeeds = FeedConfigs.Values
+                    .SelectMany(f => f)
+                    .Where(f => f.Internal == false);
+
+                foreach (TargetFeedConfig feedConfig in publicFeeds)
                 {
-                    Log.LogError($"Use of non-internal feed '{feedConfig.TargetURL}' is invalid for an internal build. This can be overridden with '{nameof(SkipSafetyChecks)}= true'");
-                }
+                    Log.LogError($"Internal builds shouldn't be published to public feed '{feedConfig.TargetURL}'. This behavior can be overridden with '{nameof(SkipInternalAssetToPublicFeedCheck)}= true'");
+                }         
             }
         }
 
@@ -372,7 +379,7 @@ protected void CheckForInternalBuildsOnPublicFeeds(TargetFeedConfig feedConfig)
         /// </summary>
         public void CheckForStableAssetsInNonIsolatedFeeds()
         {
-            if (BuildModel.Identity.IsReleaseOnlyPackageVersion || SkipSafetyChecks)
+            if (BuildModel.Identity.IsReleaseOnlyPackageVersion || SkipStablePackagesNonIsolatedFeedsCheck)
             {
                 return;
             }

src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestV3.cs

@@ -182,6 +182,7 @@ public override async Task<bool> ExecuteAsync()
                 }
 
                 CheckForStableAssetsInNonIsolatedFeeds();
+                CheckForInternalBuildsOnPublicFeeds();
 
                 if (Log.HasLoggedErrors)
                 {