From ec1b1dce630a9b9e2ddfec8a814aab6887663469 Mon Sep 17 00:00:00 2001 From: Stefan Petrushevski Date: Fri, 8 Aug 2025 16:41:29 +0200 Subject: [PATCH 1/2] refactor cli and env access token implementation --- src/Octoshift/Factories/GithubApiFactory.cs | 6 ++--- .../IntegrateBoardsCommandHandler.cs | 2 +- .../MigrateRepo/MigrateRepoCommandHandler.cs | 2 +- .../MigrateRepo/MigrateRepoCommandHandler.cs | 4 ++-- .../MigrateCodeScanningAlertsCommand.cs | 5 +--- .../MigrateOrg/MigrateOrgCommandHandler.cs | 2 +- .../MigrateRepo/MigrateRepoCommandHandler.cs | 4 ++-- .../MigrateSecretAlertsCommand.cs | 5 +--- .../CodeScanningAlertServiceFactory.cs | 24 +++++++++++++++---- .../SecretScanningAlertServiceFactory.cs | 24 +++++++++++++++---- 10 files changed, 50 insertions(+), 28 deletions(-) diff --git a/src/Octoshift/Factories/GithubApiFactory.cs b/src/Octoshift/Factories/GithubApiFactory.cs index 8e6c24582..09462e99e 100644 --- a/src/Octoshift/Factories/GithubApiFactory.cs +++ b/src/Octoshift/Factories/GithubApiFactory.cs @@ -30,7 +30,7 @@ GithubApi ISourceGithubApiFactory.Create(string apiUrl, string uploadsUrl, strin { apiUrl ??= DEFAULT_API_URL; uploadsUrl ??= DEFAULT_UPLOADS_URL; - sourcePersonalAccessToken ??= _environmentVariableProvider.SourceGithubPersonalAccessToken(); + sourcePersonalAccessToken ??= _environmentVariableProvider.SourceGithubPersonalAccessToken(throwIfNotFound: false); var githubClient = new GithubClient(_octoLogger, _clientFactory.CreateClient("Default"), _versionProvider, _retryPolicy, _dateTimeProvider, sourcePersonalAccessToken); var multipartUploader = new ArchiveUploader(githubClient, uploadsUrl, _octoLogger, _retryPolicy); return new GithubApi(githubClient, apiUrl, _retryPolicy, multipartUploader); @@ -40,7 +40,7 @@ GithubApi ISourceGithubApiFactory.CreateClientNoSsl(string apiUrl, string upload { apiUrl ??= DEFAULT_API_URL; uploadsUrl ??= DEFAULT_UPLOADS_URL; - sourcePersonalAccessToken ??= _environmentVariableProvider.SourceGithubPersonalAccessToken(); + sourcePersonalAccessToken ??= _environmentVariableProvider.SourceGithubPersonalAccessToken(throwIfNotFound: false); var githubClient = new GithubClient(_octoLogger, _clientFactory.CreateClient("NoSSL"), _versionProvider, _retryPolicy, _dateTimeProvider, sourcePersonalAccessToken); var multipartUploader = new ArchiveUploader(githubClient, uploadsUrl, _octoLogger, _retryPolicy); return new GithubApi(githubClient, apiUrl, _retryPolicy, multipartUploader); @@ -50,7 +50,7 @@ GithubApi ITargetGithubApiFactory.Create(string apiUrl, string uploadsUrl, strin { apiUrl ??= DEFAULT_API_URL; uploadsUrl ??= DEFAULT_UPLOADS_URL; - targetPersonalAccessToken ??= _environmentVariableProvider.TargetGithubPersonalAccessToken(); + targetPersonalAccessToken ??= _environmentVariableProvider.TargetGithubPersonalAccessToken(throwIfNotFound: false); var githubClient = new GithubClient(_octoLogger, _clientFactory.CreateClient("Default"), _versionProvider, _retryPolicy, _dateTimeProvider, targetPersonalAccessToken); var multipartUploader = new ArchiveUploader(githubClient, uploadsUrl, _octoLogger, _retryPolicy); return new GithubApi(githubClient, apiUrl, _retryPolicy, multipartUploader); diff --git a/src/ado2gh/Commands/IntegrateBoards/IntegrateBoardsCommandHandler.cs b/src/ado2gh/Commands/IntegrateBoards/IntegrateBoardsCommandHandler.cs index df472382f..956d6a086 100644 --- a/src/ado2gh/Commands/IntegrateBoards/IntegrateBoardsCommandHandler.cs +++ b/src/ado2gh/Commands/IntegrateBoards/IntegrateBoardsCommandHandler.cs @@ -29,7 +29,7 @@ public async Task Handle(IntegrateBoardsCommandArgs args) _log.LogInformation("Integrating Azure Boards..."); - args.GithubPat ??= _environmentVariableProvider.TargetGithubPersonalAccessToken(); + args.GithubPat ??= _environmentVariableProvider.TargetGithubPersonalAccessToken(throwIfNotFound: false); var adoTeamProjectId = await _adoApi.GetTeamProjectId(args.AdoOrg, args.AdoTeamProject); var githubHandle = await _adoApi.GetGithubHandle(args.AdoOrg, args.AdoTeamProject, args.GithubPat); diff --git a/src/ado2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs b/src/ado2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs index a1a15d101..33c85fa22 100644 --- a/src/ado2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs +++ b/src/ado2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs @@ -30,7 +30,7 @@ public async Task Handle(MigrateRepoCommandArgs args) _log.LogInformation("Migrating Repo..."); - args.GithubPat ??= _environmentVariableProvider.TargetGithubPersonalAccessToken(); + args.GithubPat ??= _environmentVariableProvider.TargetGithubPersonalAccessToken(throwIfNotFound: false); var adoRepoUrl = GetAdoRepoUrl(args.AdoOrg, args.AdoTeamProject, args.AdoRepo, args.AdoServerUrl); diff --git a/src/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs b/src/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs index 03c3d7754..31d04fe47 100644 --- a/src/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs +++ b/src/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs @@ -227,7 +227,7 @@ private async Task CreateMigrationSource(MigrateRepoCommandArgs args) { _log.LogInformation("Creating Migration Source..."); - args.GithubPat ??= _environmentVariableProvider.TargetGithubPersonalAccessToken(); + args.GithubPat ??= _environmentVariableProvider.TargetGithubPersonalAccessToken(throwIfNotFound: false); var githubOrgId = await _githubApi.GetOrganizationId(args.GithubOrg); try @@ -250,7 +250,7 @@ private async Task ImportArchive(MigrateRepoCommandArgs args, string migrationSo var bbsRepoUrl = GetBbsRepoUrl(args); - args.GithubPat ??= _environmentVariableProvider.TargetGithubPersonalAccessToken(); + args.GithubPat ??= _environmentVariableProvider.TargetGithubPersonalAccessToken(throwIfNotFound: false); var githubOrgId = await _githubApi.GetOrganizationId(args.GithubOrg); string migrationId; diff --git a/src/gei/Commands/MigrateCodeScanningAlerts/MigrateCodeScanningAlertsCommand.cs b/src/gei/Commands/MigrateCodeScanningAlerts/MigrateCodeScanningAlertsCommand.cs index c5019b464..17cd7d455 100644 --- a/src/gei/Commands/MigrateCodeScanningAlerts/MigrateCodeScanningAlertsCommand.cs +++ b/src/gei/Commands/MigrateCodeScanningAlerts/MigrateCodeScanningAlertsCommand.cs @@ -78,10 +78,7 @@ public override MigrateCodeScanningAlertsCommandHandler BuildHandler(MigrateCode throw new ArgumentNullException(nameof(sp)); } - var environmentVariableProvider = sp.GetRequiredService(); - args.GithubSourcePat ??= environmentVariableProvider.SourceGithubPersonalAccessToken(false); - args.GithubTargetPat ??= environmentVariableProvider.TargetGithubPersonalAccessToken(); - + // The factory handles environment variable resolution if (args.GithubSourcePat.IsNullOrWhiteSpace()) { args.GithubSourcePat = args.GithubTargetPat; diff --git a/src/gei/Commands/MigrateOrg/MigrateOrgCommandHandler.cs b/src/gei/Commands/MigrateOrg/MigrateOrgCommandHandler.cs index af163a7ef..4a7dba519 100644 --- a/src/gei/Commands/MigrateOrg/MigrateOrgCommandHandler.cs +++ b/src/gei/Commands/MigrateOrg/MigrateOrgCommandHandler.cs @@ -72,7 +72,7 @@ public async Task Handle(MigrateOrgCommandArgs args) } private string GetSourceToken(MigrateOrgCommandArgs args) => - args.GithubSourcePat ?? _environmentVariableProvider.SourceGithubPersonalAccessToken(); + args.GithubSourcePat ?? _environmentVariableProvider.SourceGithubPersonalAccessToken(throwIfNotFound: false); private string GetGithubOrgUrl(string org, string baseUrl) => $"{baseUrl ?? DEFAULT_GITHUB_BASE_URL}/{org.EscapeDataString()}"; } diff --git a/src/gei/Commands/MigrateRepo/MigrateRepoCommandHandler.cs b/src/gei/Commands/MigrateRepo/MigrateRepoCommandHandler.cs index 072197515..864e8ea50 100644 --- a/src/gei/Commands/MigrateRepo/MigrateRepoCommandHandler.cs +++ b/src/gei/Commands/MigrateRepo/MigrateRepoCommandHandler.cs @@ -151,7 +151,7 @@ public async Task Handle(MigrateRepoCommandArgs args) var sourceRepoUrl = GetSourceRepoUrl(args); var sourceToken = GetSourceToken(args); - var targetToken = args.GithubTargetPat ?? _environmentVariableProvider.TargetGithubPersonalAccessToken(); + var targetToken = args.GithubTargetPat ?? _environmentVariableProvider.TargetGithubPersonalAccessToken(throwIfNotFound: false); string migrationId; @@ -212,7 +212,7 @@ public async Task Handle(MigrateRepoCommandArgs args) _log.LogInformation($"Migration log available at {migrationLogUrl} or by running `gh {CliContext.RootCommand} download-logs --github-target-org {args.GithubTargetOrg} --target-repo {args.TargetRepo}`"); } - private string GetSourceToken(MigrateRepoCommandArgs args) => args.GithubSourcePat ?? _environmentVariableProvider.SourceGithubPersonalAccessToken(); + private string GetSourceToken(MigrateRepoCommandArgs args) => args.GithubSourcePat ?? _environmentVariableProvider.SourceGithubPersonalAccessToken(throwIfNotFound: false); private string GetSourceRepoUrl(MigrateRepoCommandArgs args) => GetGithubRepoUrl(args.GithubSourceOrg, args.SourceRepo, args.GhesApiUrl.HasValue() ? ExtractGhesBaseUrl(args.GhesApiUrl) : null); diff --git a/src/gei/Commands/MigrateSecretAlerts/MigrateSecretAlertsCommand.cs b/src/gei/Commands/MigrateSecretAlerts/MigrateSecretAlertsCommand.cs index 7e2da37ba..c1998ecb9 100644 --- a/src/gei/Commands/MigrateSecretAlerts/MigrateSecretAlertsCommand.cs +++ b/src/gei/Commands/MigrateSecretAlerts/MigrateSecretAlertsCommand.cs @@ -78,10 +78,7 @@ public override MigrateSecretAlertsCommandHandler BuildHandler(MigrateSecretAler throw new ArgumentNullException(nameof(sp)); } - var environmentVariableProvider = sp.GetRequiredService(); - args.GithubSourcePat ??= environmentVariableProvider.SourceGithubPersonalAccessToken(false); - args.GithubTargetPat ??= environmentVariableProvider.TargetGithubPersonalAccessToken(); - + // The factory handles environment variable resolution if (args.GithubSourcePat.IsNullOrWhiteSpace()) { args.GithubSourcePat = args.GithubTargetPat; diff --git a/src/gei/Factories/CodeScanningAlertServiceFactory.cs b/src/gei/Factories/CodeScanningAlertServiceFactory.cs index c07d23563..2535bd4dc 100644 --- a/src/gei/Factories/CodeScanningAlertServiceFactory.cs +++ b/src/gei/Factories/CodeScanningAlertServiceFactory.cs @@ -25,13 +25,27 @@ public CodeScanningAlertServiceFactory( public virtual CodeScanningAlertService Create(string sourceApi, string sourceToken, string targetApi, string targetToken, bool sourceApiNoSsl = false) { - sourceToken ??= _environmentVariableProvider.SourceGithubPersonalAccessToken(); - targetToken ??= _environmentVariableProvider.TargetGithubPersonalAccessToken(); + // Only resolve from environment if tokens are explicitly null + // Use consistent throwIfNotFound=false to prevent exceptions when CLI args are preferred + sourceToken ??= _environmentVariableProvider.SourceGithubPersonalAccessToken(false); + + targetToken ??= _environmentVariableProvider.TargetGithubPersonalAccessToken(false); + + // Validate that we have tokens after all resolution attempts + if (string.IsNullOrWhiteSpace(sourceToken)) + { + throw new OctoshiftCliException("Source GitHub Personal Access Token is required. Provide it via --github-source-pat argument or GH_SOURCE_PAT/GH_PAT environment variable."); + } + + if (string.IsNullOrWhiteSpace(targetToken)) + { + throw new OctoshiftCliException("Target GitHub Personal Access Token is required. Provide it via --github-target-pat argument or GH_PAT environment variable."); + } var sourceGithubApi = sourceApiNoSsl - ? _sourceGithubApiFactory.CreateClientNoSsl(sourceApi, sourceToken) - : _sourceGithubApiFactory.Create(sourceApi, sourceToken); + ? _sourceGithubApiFactory.CreateClientNoSsl(sourceApi, null, sourceToken) + : _sourceGithubApiFactory.Create(sourceApi, null, sourceToken); - return new(sourceGithubApi, _targetGithubApiFactory.Create(targetApi, targetToken), _octoLogger); + return new(sourceGithubApi, _targetGithubApiFactory.Create(targetApi, null, targetToken), _octoLogger); } } diff --git a/src/gei/Factories/SecretScanningAlertServiceFactory.cs b/src/gei/Factories/SecretScanningAlertServiceFactory.cs index 84f74c3a9..2486ab101 100644 --- a/src/gei/Factories/SecretScanningAlertServiceFactory.cs +++ b/src/gei/Factories/SecretScanningAlertServiceFactory.cs @@ -25,13 +25,27 @@ public SecretScanningAlertServiceFactory( public virtual SecretScanningAlertService Create(string sourceApi, string sourceToken, string targetApi, string targetToken, bool sourceApiNoSsl = false) { - sourceToken ??= _environmentVariableProvider.SourceGithubPersonalAccessToken(); - targetToken ??= _environmentVariableProvider.TargetGithubPersonalAccessToken(); + // Only resolve from environment if tokens are explicitly null + // Use consistent throwIfNotFound=false to prevent exceptions when CLI args are preferred + sourceToken ??= _environmentVariableProvider.SourceGithubPersonalAccessToken(false); + + targetToken ??= _environmentVariableProvider.TargetGithubPersonalAccessToken(false); + + // Validate that we have tokens after all resolution attempts + if (string.IsNullOrWhiteSpace(sourceToken)) + { + throw new OctoshiftCliException("Source GitHub Personal Access Token is required. Provide it via --github-source-pat argument or GH_SOURCE_PAT/GH_PAT environment variable."); + } + + if (string.IsNullOrWhiteSpace(targetToken)) + { + throw new OctoshiftCliException("Target GitHub Personal Access Token is required. Provide it via --github-target-pat argument or GH_PAT environment variable."); + } var sourceGithubApi = sourceApiNoSsl - ? _sourceGithubApiFactory.CreateClientNoSsl(sourceApi, sourceToken) - : _sourceGithubApiFactory.Create(sourceApi, sourceToken); + ? _sourceGithubApiFactory.CreateClientNoSsl(sourceApi, null, sourceToken) + : _sourceGithubApiFactory.Create(sourceApi, null, sourceToken); - return new(sourceGithubApi, _targetGithubApiFactory.Create(targetApi, targetToken), _octoLogger); + return new(sourceGithubApi, _targetGithubApiFactory.Create(targetApi, null, targetToken), _octoLogger); } } From 5a717213be9ce2c1e0dc6285dea9bc8f9ea882bd Mon Sep 17 00:00:00 2001 From: Stefan Petrushevski Date: Fri, 8 Aug 2025 16:48:01 +0200 Subject: [PATCH 2/2] update releasenotes --- RELEASENOTES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 8b1378917..793cf1aac 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -1 +1,2 @@ +- Fixed issue where CLI commands required GH_PAT environment variable even when GitHub tokens were provided via command-line arguments (--github-source-pat, --github-target-pat). All migration commands now work correctly with CLI-only token authentication.