From c6a741df4505255a70bd91b05494dcf7c65f5e6d Mon Sep 17 00:00:00 2001 From: Yohann Bacha Date: Fri, 3 Feb 2023 19:22:06 +0100 Subject: [PATCH 01/21] feat: added the option to enable or disable review app creation from forks --- cmd/integration_link.go | 42 +++++++++++++++++++++++++++++++++------ integrationlink/params.go | 13 ++++++++++++ integrationlink/show.go | 11 ++++++++++ 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/cmd/integration_link.go b/cmd/integration_link.go index c00228f83..cd2d5015c 100644 --- a/cmd/integration_link.go +++ b/cmd/integration_link.go @@ -57,11 +57,13 @@ var ( &appFlag, &cli.StringFlag{Name: "branch", Usage: "Branch used in auto deploy"}, &cli.BoolFlag{Name: "auto-deploy", Usage: "Enable auto deploy of application after each branch change"}, - &cli.BoolFlag{Name: "deploy-review-apps", Usage: "Enable auto deploy of review app when new pull request is opened"}, + &cli.BoolFlag{Name: "deploy-review-apps", Usage: "Enable auto deploy of review apps when new pull request is opened"}, &cli.BoolFlag{Name: "destroy-on-close", Usage: "Auto destroy review apps when pull request is closed"}, - &cli.BoolFlag{Name: "no-auto-deploy", Usage: "Enable auto deploy of application after each branch change"}, - &cli.BoolFlag{Name: "no-deploy-review-apps", Usage: "Enable auto deploy of review app when new pull request is opened"}, + &cli.BoolFlag{Name: "no-auto-deploy", Usage: "Disable auto deploy of application after each branch change"}, + &cli.BoolFlag{Name: "no-deploy-review-apps", Usage: "Disable auto deploy of review app when new pull request is opened"}, &cli.BoolFlag{Name: "no-destroy-on-close", Usage: "Auto destroy review apps when pull request is closed"}, + &cli.BoolFlag{Name: "review-apps-on-forks", Usage: "Enable auto deploy of review apps when new pull request is opened from a fork"}, + &cli.BoolFlag{Name: "no-review-apps-on-forks", Usage: "Disable auto deploy of review apps when new pull request is opened from a fork"}, &cli.UintFlag{Name: "hours-before-destroy-on-close", Usage: "Time delay before auto destroying a review app when pull request is closed"}, &cli.BoolFlag{Name: "destroy-on-stale", Usage: "Auto destroy review apps when no deploy/commits has happened"}, &cli.BoolFlag{Name: "no-destroy-on-stale", Usage: "Auto destroy review apps when no deploy/commits has happened"}, @@ -80,7 +82,7 @@ List of available integrations: `, Examples: []string{ "scalingo --app my-app integration-link-create https://gitlab.com/gitlab-org/gitlab-ce", - "scalingo --app my-app integration-link-create --branch master --auto-deploy https://ghe.example.org/test/frontend-app", + "scalingo --app my-app integration-link-create --branch master --auto-deploy --deploy-review-apps --no-review-apps-on-forks https://ghe.example.org/test/frontend-app", }, SeeAlso: []string{"integration-link", "integration-link-update", "integration-link-delete", "integration-link-manual-deploy", "integration-link-manual-review-app"}, }.Render(), @@ -157,6 +159,13 @@ List of available integrations: } hoursBeforeDestroyOnStale := c.Uint("hours-before-destroy-on-stale") + reviewAppsOnForks := c.Bool("review-apps-on-forks") + noReviewAppsOnForks := c.Bool("no-review-apps-on-forks") + + if reviewAppsOnForks && noReviewAppsOnForks { + errorQuit(errors.New("cannot define both review-apps-on-forks and no-review-apps-on-forks")) + } + params = scalingo.SCMRepoLinkCreateParams{ Branch: &branch, AutoDeployEnabled: &autoDeploy, @@ -165,6 +174,7 @@ List of available integrations: HoursBeforeDeleteOnClose: &hoursBeforeDestroyOnClose, DestroyStaleEnabled: &destroyOnStale, HoursBeforeDeleteStale: &hoursBeforeDestroyOnStale, + ForksAllowed: &reviewAppsOnForks, } } @@ -206,9 +216,11 @@ List of available integrations: &appFlag, &cli.StringFlag{Name: "branch", Usage: "Branch used in auto deploy"}, &cli.BoolFlag{Name: "auto-deploy", Usage: "Enable auto deploy of application after each branch change"}, - &cli.BoolFlag{Name: "no-auto-deploy", Usage: "Enable auto deploy of application after each branch change"}, + &cli.BoolFlag{Name: "no-auto-deploy", Usage: "Disable auto deploy of application after each branch change"}, &cli.BoolFlag{Name: "deploy-review-apps", Usage: "Enable auto deploy of review app when new pull request is opened"}, - &cli.BoolFlag{Name: "no-deploy-review-apps", Usage: "Enable auto deploy of review app when new pull request is opened"}, + &cli.BoolFlag{Name: "no-deploy-review-apps", Usage: "Disable auto deploy of review app when new pull request is opened"}, + &cli.BoolFlag{Name: "review-apps-on-forks", Usage: "Enable auto deploy of review apps when new pull request is opened from a fork"}, + &cli.BoolFlag{Name: "no-review-apps-on-forks", Usage: "Disable auto deploy of review apps when new pull request is opened from a fork"}, &cli.BoolFlag{Name: "destroy-on-close", Usage: "Auto destroy review apps when pull request is closed"}, &cli.BoolFlag{Name: "no-destroy-on-close", Usage: "Auto destroy review apps when pull request is closed"}, &cli.UintFlag{Name: "hours-before-destroy-on-close", Usage: "Time delay before auto destroying a review app when pull request is closed"}, @@ -253,6 +265,13 @@ List of available integrations: errorQuit(errors.New("cannot define both destroy-on-stale and no-destroy-on-stale")) } + reviewAppsOnForks := c.Bool("review-apps-on-forks") + noReviewAppsOnForks := c.Bool("no-review-apps-on-forks") + + if reviewAppsOnForks && noReviewAppsOnForks { + errorQuit(errors.New("cannot define both review-apps-on-forks and no-review-apps-on-forks")) + } + currentApp := detect.CurrentApp(c) params, err := integrationlink.CheckAndFillParams(c, currentApp) if err != nil { @@ -447,6 +466,17 @@ func interactiveCreate() (scalingo.SCMRepoLinkCreateParams, error) { hoursBeforeDestroyOnStale := uint(hoursBeforeDestroyOnStale64) params.HoursBeforeDeleteStale = &hoursBeforeDestroyOnStale } + + forksAllowed := false + err = survey.AskOne(&survey.Confirm{ + Message: "Allow review apps to be created from forks:", + Default: forksAllowed, + }, &forksAllowed, nil) + if err != nil { + return params, err + } + params.ForksAllowed = &forksAllowed + return params, nil } diff --git a/integrationlink/params.go b/integrationlink/params.go index 2f9b09479..2b585353a 100644 --- a/integrationlink/params.go +++ b/integrationlink/params.go @@ -16,6 +16,7 @@ func CheckAndFillParams(c *cli.Context, app string) (*scalingo.SCMRepoLinkUpdate HoursBeforeDeleteOnClose: paramsChecker.lookupHoursBeforeDestroyOnClose(), DestroyStaleEnabled: paramsChecker.lookupDestroyOnStale(), HoursBeforeDeleteStale: paramsChecker.lookupHoursBeforeDestroyOnStale(), + ForksAllowed: paramsChecker.lookupReviewAppsOnForks(), } return params, nil @@ -95,6 +96,18 @@ func (p *paramsChecker) lookupDestroyOnStale() *bool { return nil } +func (p *paramsChecker) lookupReviewAppsOnForks() *bool { + if p.ctx.IsSet("review-apps-on-forks") { + t := true + return &t + } + if p.ctx.IsSet("no-review-apps-on-forks") { + f := false + return &f + } + return nil +} + func (p *paramsChecker) lookupHoursBeforeDestroyOnStale() *uint { if !p.ctx.IsSet("hours-before-destroy-on-stale") { return nil diff --git a/integrationlink/show.go b/integrationlink/show.go index ca6019d51..2c8f72d58 100644 --- a/integrationlink/show.go +++ b/integrationlink/show.go @@ -100,6 +100,17 @@ func Show(ctx context.Context, app string) error { color.New(color.FgYellow).Sprint("Destroy on Stale"), deleteOnStale, ) + + var forksAllowed string + if repoLink.ForksAllowed { + forksAllowed = color.GreenString(utils.Success) + } else { + forksAllowed = color.RedString(utils.Error) + } + fmt.Printf("\t%s: %s\n", + color.New(color.FgYellow).Sprint("Creation from forks"), + forksAllowed, + ) } return nil From f8a27658a724edf6c178641b1a7931b13c7d6e28 Mon Sep 17 00:00:00 2001 From: Yohann Bacha Date: Mon, 6 Feb 2023 14:37:09 +0100 Subject: [PATCH 02/21] fix: changed the option name --- cmd/integration_link.go | 18 +++++++++--------- integrationlink/params.go | 16 ++++++++-------- integrationlink/show.go | 4 ++-- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/cmd/integration_link.go b/cmd/integration_link.go index cd2d5015c..d890565b2 100644 --- a/cmd/integration_link.go +++ b/cmd/integration_link.go @@ -167,14 +167,14 @@ List of available integrations: } params = scalingo.SCMRepoLinkCreateParams{ - Branch: &branch, - AutoDeployEnabled: &autoDeploy, - DeployReviewAppsEnabled: &deployReviewApps, - DestroyOnCloseEnabled: &destroyOnClose, - HoursBeforeDeleteOnClose: &hoursBeforeDestroyOnClose, - DestroyStaleEnabled: &destroyOnStale, - HoursBeforeDeleteStale: &hoursBeforeDestroyOnStale, - ForksAllowed: &reviewAppsOnForks, + Branch: &branch, + AutoDeployEnabled: &autoDeploy, + DeployReviewAppsEnabled: &deployReviewApps, + DestroyOnCloseEnabled: &destroyOnClose, + HoursBeforeDeleteOnClose: &hoursBeforeDestroyOnClose, + DestroyStaleEnabled: &destroyOnStale, + HoursBeforeDeleteStale: &hoursBeforeDestroyOnStale, + AutomaticCreationFromForksAllowed: &reviewAppsOnForks, } } @@ -475,7 +475,7 @@ func interactiveCreate() (scalingo.SCMRepoLinkCreateParams, error) { if err != nil { return params, err } - params.ForksAllowed = &forksAllowed + params.AutomaticCreationFromForksAllowed = &forksAllowed return params, nil } diff --git a/integrationlink/params.go b/integrationlink/params.go index 2b585353a..4bd604812 100644 --- a/integrationlink/params.go +++ b/integrationlink/params.go @@ -9,14 +9,14 @@ import ( func CheckAndFillParams(c *cli.Context, app string) (*scalingo.SCMRepoLinkUpdateParams, error) { paramsChecker := newParamsChecker(c) params := &scalingo.SCMRepoLinkUpdateParams{ - Branch: paramsChecker.lookupBranch(), - AutoDeployEnabled: paramsChecker.lookupAutoDeploy(), - DeployReviewAppsEnabled: paramsChecker.lookupDeployReviewApps(), - DestroyOnCloseEnabled: paramsChecker.lookupDestroyOnClose(), - HoursBeforeDeleteOnClose: paramsChecker.lookupHoursBeforeDestroyOnClose(), - DestroyStaleEnabled: paramsChecker.lookupDestroyOnStale(), - HoursBeforeDeleteStale: paramsChecker.lookupHoursBeforeDestroyOnStale(), - ForksAllowed: paramsChecker.lookupReviewAppsOnForks(), + Branch: paramsChecker.lookupBranch(), + AutoDeployEnabled: paramsChecker.lookupAutoDeploy(), + DeployReviewAppsEnabled: paramsChecker.lookupDeployReviewApps(), + DestroyOnCloseEnabled: paramsChecker.lookupDestroyOnClose(), + HoursBeforeDeleteOnClose: paramsChecker.lookupHoursBeforeDestroyOnClose(), + DestroyStaleEnabled: paramsChecker.lookupDestroyOnStale(), + HoursBeforeDeleteStale: paramsChecker.lookupHoursBeforeDestroyOnStale(), + AutomaticCreationFromForksAllowed: paramsChecker.lookupReviewAppsOnForks(), } return params, nil diff --git a/integrationlink/show.go b/integrationlink/show.go index 2c8f72d58..89ad11ee4 100644 --- a/integrationlink/show.go +++ b/integrationlink/show.go @@ -102,13 +102,13 @@ func Show(ctx context.Context, app string) error { ) var forksAllowed string - if repoLink.ForksAllowed { + if repoLink.AutomaticCreationFromForksAllowed { forksAllowed = color.GreenString(utils.Success) } else { forksAllowed = color.RedString(utils.Error) } fmt.Printf("\t%s: %s\n", - color.New(color.FgYellow).Sprint("Creation from forks"), + color.New(color.FgYellow).Sprint("Automatic creation from forks"), forksAllowed, ) } From 678c261636276c6c3cac110599b560d619ed80f3 Mon Sep 17 00:00:00 2001 From: Yohann Bacha Date: Fri, 10 Feb 2023 15:06:17 +0100 Subject: [PATCH 03/21] Rename the flag for setting the parameter --- cmd/integration_link.go | 28 ++++++++++++++-------------- integrationlink/params.go | 8 ++++---- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/cmd/integration_link.go b/cmd/integration_link.go index d890565b2..0b27521c7 100644 --- a/cmd/integration_link.go +++ b/cmd/integration_link.go @@ -62,8 +62,8 @@ var ( &cli.BoolFlag{Name: "no-auto-deploy", Usage: "Disable auto deploy of application after each branch change"}, &cli.BoolFlag{Name: "no-deploy-review-apps", Usage: "Disable auto deploy of review app when new pull request is opened"}, &cli.BoolFlag{Name: "no-destroy-on-close", Usage: "Auto destroy review apps when pull request is closed"}, - &cli.BoolFlag{Name: "review-apps-on-forks", Usage: "Enable auto deploy of review apps when new pull request is opened from a fork"}, - &cli.BoolFlag{Name: "no-review-apps-on-forks", Usage: "Disable auto deploy of review apps when new pull request is opened from a fork"}, + &cli.BoolFlag{Name: "allow-review-apps-from-forks", Usage: "Enable auto deploy of review apps when new pull request is opened from a fork"}, + &cli.BoolFlag{Name: "no-allow-review-apps-from-forks", Usage: "Disable auto deploy of review apps when new pull request is opened from a fork"}, &cli.UintFlag{Name: "hours-before-destroy-on-close", Usage: "Time delay before auto destroying a review app when pull request is closed"}, &cli.BoolFlag{Name: "destroy-on-stale", Usage: "Auto destroy review apps when no deploy/commits has happened"}, &cli.BoolFlag{Name: "no-destroy-on-stale", Usage: "Auto destroy review apps when no deploy/commits has happened"}, @@ -82,7 +82,7 @@ List of available integrations: `, Examples: []string{ "scalingo --app my-app integration-link-create https://gitlab.com/gitlab-org/gitlab-ce", - "scalingo --app my-app integration-link-create --branch master --auto-deploy --deploy-review-apps --no-review-apps-on-forks https://ghe.example.org/test/frontend-app", + "scalingo --app my-app integration-link-create --branch master --auto-deploy --deploy-review-apps --no-allow-review-apps-from-forks https://ghe.example.org/test/frontend-app", }, SeeAlso: []string{"integration-link", "integration-link-update", "integration-link-delete", "integration-link-manual-deploy", "integration-link-manual-review-app"}, }.Render(), @@ -159,11 +159,11 @@ List of available integrations: } hoursBeforeDestroyOnStale := c.Uint("hours-before-destroy-on-stale") - reviewAppsOnForks := c.Bool("review-apps-on-forks") - noReviewAppsOnForks := c.Bool("no-review-apps-on-forks") + allowReviewAppsFromForks := c.Bool("allow-review-apps-from-forks") + noAllowReviewAppsFromForks := c.Bool("no-allow-review-apps-from-forks") - if reviewAppsOnForks && noReviewAppsOnForks { - errorQuit(errors.New("cannot define both review-apps-on-forks and no-review-apps-on-forks")) + if allowReviewAppsFromForks && noAllowReviewAppsFromForks { + errorQuit(errors.New("cannot define both allow-review-apps-from-forks and no-allow-review-apps-from-forks")) } params = scalingo.SCMRepoLinkCreateParams{ @@ -174,7 +174,7 @@ List of available integrations: HoursBeforeDeleteOnClose: &hoursBeforeDestroyOnClose, DestroyStaleEnabled: &destroyOnStale, HoursBeforeDeleteStale: &hoursBeforeDestroyOnStale, - AutomaticCreationFromForksAllowed: &reviewAppsOnForks, + AutomaticCreationFromForksAllowed: &allowReviewAppsFromForks, } } @@ -219,8 +219,8 @@ List of available integrations: &cli.BoolFlag{Name: "no-auto-deploy", Usage: "Disable auto deploy of application after each branch change"}, &cli.BoolFlag{Name: "deploy-review-apps", Usage: "Enable auto deploy of review app when new pull request is opened"}, &cli.BoolFlag{Name: "no-deploy-review-apps", Usage: "Disable auto deploy of review app when new pull request is opened"}, - &cli.BoolFlag{Name: "review-apps-on-forks", Usage: "Enable auto deploy of review apps when new pull request is opened from a fork"}, - &cli.BoolFlag{Name: "no-review-apps-on-forks", Usage: "Disable auto deploy of review apps when new pull request is opened from a fork"}, + &cli.BoolFlag{Name: "allow-review-apps-from-forks", Usage: "Enable auto deploy of review apps when new pull request is opened from a fork"}, + &cli.BoolFlag{Name: "no-allow-review-apps-from-forks", Usage: "Disable auto deploy of review apps when new pull request is opened from a fork"}, &cli.BoolFlag{Name: "destroy-on-close", Usage: "Auto destroy review apps when pull request is closed"}, &cli.BoolFlag{Name: "no-destroy-on-close", Usage: "Auto destroy review apps when pull request is closed"}, &cli.UintFlag{Name: "hours-before-destroy-on-close", Usage: "Time delay before auto destroying a review app when pull request is closed"}, @@ -265,11 +265,11 @@ List of available integrations: errorQuit(errors.New("cannot define both destroy-on-stale and no-destroy-on-stale")) } - reviewAppsOnForks := c.Bool("review-apps-on-forks") - noReviewAppsOnForks := c.Bool("no-review-apps-on-forks") + allowReviewAppsFromForks := c.Bool("allow-review-apps-from-forks") + noAllowReviewAppsFromForks := c.Bool("no-allow-review-apps-from-forks") - if reviewAppsOnForks && noReviewAppsOnForks { - errorQuit(errors.New("cannot define both review-apps-on-forks and no-review-apps-on-forks")) + if allowReviewAppsFromForks && noAllowReviewAppsFromForks { + errorQuit(errors.New("cannot define both allow-review-apps-from-forks and no-allow-review-apps-from-forks")) } currentApp := detect.CurrentApp(c) diff --git a/integrationlink/params.go b/integrationlink/params.go index 4bd604812..bda4445d7 100644 --- a/integrationlink/params.go +++ b/integrationlink/params.go @@ -16,7 +16,7 @@ func CheckAndFillParams(c *cli.Context, app string) (*scalingo.SCMRepoLinkUpdate HoursBeforeDeleteOnClose: paramsChecker.lookupHoursBeforeDestroyOnClose(), DestroyStaleEnabled: paramsChecker.lookupDestroyOnStale(), HoursBeforeDeleteStale: paramsChecker.lookupHoursBeforeDestroyOnStale(), - AutomaticCreationFromForksAllowed: paramsChecker.lookupReviewAppsOnForks(), + AutomaticCreationFromForksAllowed: paramsChecker.lookupAllowReviewAppsFromForks(), } return params, nil @@ -96,12 +96,12 @@ func (p *paramsChecker) lookupDestroyOnStale() *bool { return nil } -func (p *paramsChecker) lookupReviewAppsOnForks() *bool { - if p.ctx.IsSet("review-apps-on-forks") { +func (p *paramsChecker) lookupAllowReviewAppsFromForks() *bool { + if p.ctx.IsSet("allow-review-apps-from-forks") { t := true return &t } - if p.ctx.IsSet("no-review-apps-on-forks") { + if p.ctx.IsSet("no-allow-review-apps-from-forks") { f := false return &f } From 9f67acb5f3e81917a65ef96ea1fdacafa25065d6 Mon Sep 17 00:00:00 2001 From: Yohann Bacha Date: Tue, 21 Feb 2023 10:47:16 +0100 Subject: [PATCH 04/21] fix: help message for interactive creation --- cmd/integration_link.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/integration_link.go b/cmd/integration_link.go index 0b27521c7..2a56de47b 100644 --- a/cmd/integration_link.go +++ b/cmd/integration_link.go @@ -470,6 +470,7 @@ func interactiveCreate() (scalingo.SCMRepoLinkCreateParams, error) { forksAllowed := false err = survey.AskOne(&survey.Confirm{ Message: "Allow review apps to be created from forks:", + Help: "Only allow automatic review apps deployments from forks if you trust the owners of those forks, as this could lead to security issues", Default: forksAllowed, }, &forksAllowed, nil) if err != nil { From 7b9f785d3d12f49bba42d659554e04eb1337c88d Mon Sep 17 00:00:00 2001 From: Yohann Bacha Date: Wed, 22 Feb 2023 16:51:16 +0100 Subject: [PATCH 05/21] feat(review-apps): security warning Signed-off-by: Yohann Bacha --- cmd/integration_link.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/cmd/integration_link.go b/cmd/integration_link.go index 2a56de47b..19c22e132 100644 --- a/cmd/integration_link.go +++ b/cmd/integration_link.go @@ -22,6 +22,11 @@ import ( ) var ( + warningSecurityMessageForAutomaticReviewAppsFromForks = `You are about to allow automatic review apps creation from forks. +Allowing this on a public repository can expose your application to security concerns, +as exposing the review app environment variables to whoever forks your repository, and opens a pull request. +Being aware of the risks, do you want to allow automatic review apps creation from forks?` + integrationLinkShowCommand = cli.Command{ Name: "integration-link", Category: "Integration Link", @@ -495,3 +500,20 @@ func validateHoursBeforeDelete(ans interface{}) error { } return nil } + +func isUserAwareOfSecurityRisksInteractive() (bool, error) { + io.Warning(warningSecurityMessageForAutomaticReviewAppsFromForks) + forksAllowed := false + err := survey.AskOne(&survey.Confirm{ + Message: "Automatic review apps creation from forks:", + Default: forksAllowed, + }, &forksAllowed, nil) + if err != nil { + return false, err + } + if forksAllowed { + io.Info("To bypass this security warning next time, you can provide the flag --aware-of-security-risks ") + } + + return forksAllowed, nil +} From 0bd2178084bd674210ab2344d82a2c47a622eb96 Mon Sep 17 00:00:00 2001 From: Yohann Bacha Date: Wed, 22 Feb 2023 18:16:14 +0100 Subject: [PATCH 06/21] feat(review-apps): interactive security warning when creating and updating integration links Signed-off-by: Yohann Bacha --- cmd/integration_link.go | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/cmd/integration_link.go b/cmd/integration_link.go index 19c22e132..e13fc3e77 100644 --- a/cmd/integration_link.go +++ b/cmd/integration_link.go @@ -68,6 +68,7 @@ Being aware of the risks, do you want to allow automatic review apps creation fr &cli.BoolFlag{Name: "no-deploy-review-apps", Usage: "Disable auto deploy of review app when new pull request is opened"}, &cli.BoolFlag{Name: "no-destroy-on-close", Usage: "Auto destroy review apps when pull request is closed"}, &cli.BoolFlag{Name: "allow-review-apps-from-forks", Usage: "Enable auto deploy of review apps when new pull request is opened from a fork"}, + &cli.BoolFlag{Name: "aware-of-security-risks", Usage: "Bypass the security warning about allowing automatic review app creation from forks"}, &cli.BoolFlag{Name: "no-allow-review-apps-from-forks", Usage: "Disable auto deploy of review apps when new pull request is opened from a fork"}, &cli.UintFlag{Name: "hours-before-destroy-on-close", Usage: "Time delay before auto destroying a review app when pull request is closed"}, &cli.BoolFlag{Name: "destroy-on-stale", Usage: "Auto destroy review apps when no deploy/commits has happened"}, @@ -171,6 +172,15 @@ List of available integrations: errorQuit(errors.New("cannot define both allow-review-apps-from-forks and no-allow-review-apps-from-forks")) } + awareOfSecurityRisks := c.Bool("aware-of-security-risks") + + if allowReviewAppsFromForks && !awareOfSecurityRisks { + allowReviewAppsFromForks, err = isAutomaticReviewAppFromForksStillAllowedAwaringUserOfSecurityConcerns() + if err != nil { + errorQuit(err) + } + } + params = scalingo.SCMRepoLinkCreateParams{ Branch: &branch, AutoDeployEnabled: &autoDeploy, @@ -225,6 +235,7 @@ List of available integrations: &cli.BoolFlag{Name: "deploy-review-apps", Usage: "Enable auto deploy of review app when new pull request is opened"}, &cli.BoolFlag{Name: "no-deploy-review-apps", Usage: "Disable auto deploy of review app when new pull request is opened"}, &cli.BoolFlag{Name: "allow-review-apps-from-forks", Usage: "Enable auto deploy of review apps when new pull request is opened from a fork"}, + &cli.BoolFlag{Name: "aware-of-security-risks", Usage: "Bypass the security warning about allowing automatic review app creation from forks"}, &cli.BoolFlag{Name: "no-allow-review-apps-from-forks", Usage: "Disable auto deploy of review apps when new pull request is opened from a fork"}, &cli.BoolFlag{Name: "destroy-on-close", Usage: "Auto destroy review apps when pull request is closed"}, &cli.BoolFlag{Name: "no-destroy-on-close", Usage: "Auto destroy review apps when pull request is closed"}, @@ -277,6 +288,16 @@ List of available integrations: errorQuit(errors.New("cannot define both allow-review-apps-from-forks and no-allow-review-apps-from-forks")) } + awareOfSecurityRisks := c.Bool("aware-of-security-risks") + + if allowReviewAppsFromForks && !awareOfSecurityRisks { + stillAllowed, err := isAutomaticReviewAppFromForksStillAllowedAwaringUserOfSecurityConcerns() + if err != nil { + errorQuit(err) + } + c.Set("allow-review-apps-from-forks", strconv.FormatBool(stillAllowed)) + } + currentApp := detect.CurrentApp(c) params, err := integrationlink.CheckAndFillParams(c, currentApp) if err != nil { @@ -501,7 +522,7 @@ func validateHoursBeforeDelete(ans interface{}) error { return nil } -func isUserAwareOfSecurityRisksInteractive() (bool, error) { +func isAutomaticReviewAppFromForksStillAllowedAwaringUserOfSecurityConcerns() (bool, error) { io.Warning(warningSecurityMessageForAutomaticReviewAppsFromForks) forksAllowed := false err := survey.AskOne(&survey.Confirm{ @@ -512,7 +533,7 @@ func isUserAwareOfSecurityRisksInteractive() (bool, error) { return false, err } if forksAllowed { - io.Info("To bypass this security warning next time, you can provide the flag --aware-of-security-risks ") + io.Info("To bypass this security warning next time, you can provide the flag --aware-of-security-risks") } return forksAllowed, nil From c67487bb20bd6e14beae883473f4a9ceef180051 Mon Sep 17 00:00:00 2001 From: Yohann Bacha Date: Wed, 22 Feb 2023 18:20:33 +0100 Subject: [PATCH 07/21] fix(review-apps): linting issues Signed-off-by: Yohann Bacha --- cmd/integration_link.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/integration_link.go b/cmd/integration_link.go index e13fc3e77..06385a4fc 100644 --- a/cmd/integration_link.go +++ b/cmd/integration_link.go @@ -295,7 +295,9 @@ List of available integrations: if err != nil { errorQuit(err) } - c.Set("allow-review-apps-from-forks", strconv.FormatBool(stillAllowed)) + if err = c.Set("allow-review-apps-from-forks", strconv.FormatBool(stillAllowed)); err != nil { + errorQuit(err) + } } currentApp := detect.CurrentApp(c) From 2e3cf1d6b9e74292e685261eacb14014b4fcaa77 Mon Sep 17 00:00:00 2001 From: yohann-bacha <122296171+yohann-bacha@users.noreply.github.com> Date: Thu, 23 Feb 2023 10:45:23 +0100 Subject: [PATCH 08/21] fix: apply suggestions from code review Co-authored-by: aurelien-reeves-scalingo <111763415+aurelien-reeves-scalingo@users.noreply.github.com> --- cmd/integration_link.go | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/cmd/integration_link.go b/cmd/integration_link.go index 06385a4fc..bb009ea6b 100644 --- a/cmd/integration_link.go +++ b/cmd/integration_link.go @@ -22,10 +22,7 @@ import ( ) var ( - warningSecurityMessageForAutomaticReviewAppsFromForks = `You are about to allow automatic review apps creation from forks. -Allowing this on a public repository can expose your application to security concerns, -as exposing the review app environment variables to whoever forks your repository, and opens a pull request. -Being aware of the risks, do you want to allow automatic review apps creation from forks?` + reviewAppsFromForksSecurityWarning = "Only allow automatic review apps deployments from forks if you trust the owners of those forks, as this could lead to security issues" integrationLinkShowCommand = cli.Command{ Name: "integration-link", @@ -295,7 +292,8 @@ List of available integrations: if err != nil { errorQuit(err) } - if err = c.Set("allow-review-apps-from-forks", strconv.FormatBool(stillAllowed)); err != nil { + err = c.Set("allow-review-apps-from-forks", strconv.FormatBool(stillAllowed)) + if err != nil { errorQuit(err) } } @@ -524,11 +522,11 @@ func validateHoursBeforeDelete(ans interface{}) error { return nil } -func isAutomaticReviewAppFromForksStillAllowedAwaringUserOfSecurityConcerns() (bool, error) { - io.Warning(warningSecurityMessageForAutomaticReviewAppsFromForks) - forksAllowed := false +func askForConfirmation(message string) (bool, error) { + io.Warning(message) + confirmed := false err := survey.AskOne(&survey.Confirm{ - Message: "Automatic review apps creation from forks:", + Message: "Are your sure?", Default: forksAllowed, }, &forksAllowed, nil) if err != nil { From bfbf6869d626c0383dcd1ac7a4927d3185a03a81 Mon Sep 17 00:00:00 2001 From: Yohann Bacha Date: Thu, 23 Feb 2023 11:09:20 +0100 Subject: [PATCH 09/21] fix: applied reviews --- cmd/integration_link.go | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/cmd/integration_link.go b/cmd/integration_link.go index bb009ea6b..043d72797 100644 --- a/cmd/integration_link.go +++ b/cmd/integration_link.go @@ -172,7 +172,7 @@ List of available integrations: awareOfSecurityRisks := c.Bool("aware-of-security-risks") if allowReviewAppsFromForks && !awareOfSecurityRisks { - allowReviewAppsFromForks, err = isAutomaticReviewAppFromForksStillAllowedAwaringUserOfSecurityConcerns() + allowReviewAppsFromForks, err = askForConfirmation(reviewAppsFromForksSecurityWarning) if err != nil { errorQuit(err) } @@ -288,7 +288,7 @@ List of available integrations: awareOfSecurityRisks := c.Bool("aware-of-security-risks") if allowReviewAppsFromForks && !awareOfSecurityRisks { - stillAllowed, err := isAutomaticReviewAppFromForksStillAllowedAwaringUserOfSecurityConcerns() + stillAllowed, err := askForConfirmation(reviewAppsFromForksSecurityWarning) if err != nil { errorQuit(err) } @@ -522,19 +522,18 @@ func validateHoursBeforeDelete(ans interface{}) error { return nil } -func askForConfirmation(message string) (bool, error) { +func askForConfirmation(message string) (bool, error) { io.Warning(message) - confirmed := false + var confirmed bool + err := survey.AskOne(&survey.Confirm{ Message: "Are your sure?", - Default: forksAllowed, - }, &forksAllowed, nil) + Default: false, + }, &confirmed, nil) + if err != nil { return false, err } - if forksAllowed { - io.Info("To bypass this security warning next time, you can provide the flag --aware-of-security-risks") - } - return forksAllowed, nil + return confirmed, nil } From 66972e030dc5e8c6a965194325859eda00528047 Mon Sep 17 00:00:00 2001 From: Yohann Bacha Date: Thu, 23 Feb 2023 11:30:30 +0100 Subject: [PATCH 10/21] fix(review-apps): when interactive creating, display review apps security concerns as a warning Signed-off-by: Yohann Bacha --- cmd/integration_link.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cmd/integration_link.go b/cmd/integration_link.go index 043d72797..1b85cc196 100644 --- a/cmd/integration_link.go +++ b/cmd/integration_link.go @@ -493,11 +493,12 @@ func interactiveCreate() (scalingo.SCMRepoLinkCreateParams, error) { params.HoursBeforeDeleteStale = &hoursBeforeDestroyOnStale } - forksAllowed := false + io.Warning("Only allow automatic review apps deployments from forks if you trust the owners of those forks, as this could lead to security issues") + var forksAllowed bool + err = survey.AskOne(&survey.Confirm{ Message: "Allow review apps to be created from forks:", - Help: "Only allow automatic review apps deployments from forks if you trust the owners of those forks, as this could lead to security issues", - Default: forksAllowed, + Default: false, }, &forksAllowed, nil) if err != nil { return params, err From fc566d7d6d0636ffea68e55553e43a44b3720034 Mon Sep 17 00:00:00 2001 From: Yohann Bacha Date: Thu, 23 Feb 2023 11:58:12 +0100 Subject: [PATCH 11/21] feat(review-apps): giving some context to errors Signed-off-by: Yohann Bacha --- cmd/integration_link.go | 17 ++++++++--------- integrationlink/params.go | 4 ++-- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/cmd/integration_link.go b/cmd/integration_link.go index 1b85cc196..e8275d13d 100644 --- a/cmd/integration_link.go +++ b/cmd/integration_link.go @@ -3,6 +3,7 @@ package cmd import ( "errors" "fmt" + "gopkg.in/errgo.v1" "net/url" "os" "strconv" @@ -43,7 +44,7 @@ var ( currentApp := detect.CurrentApp(c) err := integrationlink.Show(c.Context, currentApp) if err != nil { - errorQuit(err) + errorQuit(errgo.Notef(err, "error while showing integration link details")) } return nil }, @@ -99,7 +100,7 @@ List of available integrations: integrationURL := c.Args().First() integrationURLParsed, err := url.Parse(integrationURL) if err != nil { - errorQuit(err) + errorQuit(errgo.Notef(err, "error parsing the repository url")) } // If the customer forgot to specify the scheme, we automatically prefix with https:// if integrationURLParsed.Scheme == "" { @@ -132,7 +133,7 @@ List of available integrations: if c.NumFlags() == 0 { params, err = interactiveCreate() if err != nil { - errorQuit(err) + errorQuit(errgo.Notef(err, "error with the interactive creation of the integration link")) } } else { branch := c.String("branch") @@ -290,7 +291,7 @@ List of available integrations: if allowReviewAppsFromForks && !awareOfSecurityRisks { stillAllowed, err := askForConfirmation(reviewAppsFromForksSecurityWarning) if err != nil { - errorQuit(err) + errorQuit(errgo.Notef(err, "error while asking for security concerns awareness")) } err = c.Set("allow-review-apps-from-forks", strconv.FormatBool(stillAllowed)) if err != nil { @@ -299,12 +300,9 @@ List of available integrations: } currentApp := detect.CurrentApp(c) - params, err := integrationlink.CheckAndFillParams(c, currentApp) - if err != nil { - errorQuit(err) - } + params := integrationlink.CheckAndFillParams(c) - err = integrationlink.Update(c.Context, currentApp, *params) + err := integrationlink.Update(c.Context, currentApp, *params) if err != nil { errorQuit(err) } @@ -500,6 +498,7 @@ func interactiveCreate() (scalingo.SCMRepoLinkCreateParams, error) { Message: "Allow review apps to be created from forks:", Default: false, }, &forksAllowed, nil) + if err != nil { return params, err } diff --git a/integrationlink/params.go b/integrationlink/params.go index bda4445d7..0c8867ccc 100644 --- a/integrationlink/params.go +++ b/integrationlink/params.go @@ -6,7 +6,7 @@ import ( "github.com/Scalingo/go-scalingo/v6" ) -func CheckAndFillParams(c *cli.Context, app string) (*scalingo.SCMRepoLinkUpdateParams, error) { +func CheckAndFillParams(c *cli.Context) *scalingo.SCMRepoLinkUpdateParams { paramsChecker := newParamsChecker(c) params := &scalingo.SCMRepoLinkUpdateParams{ Branch: paramsChecker.lookupBranch(), @@ -19,7 +19,7 @@ func CheckAndFillParams(c *cli.Context, app string) (*scalingo.SCMRepoLinkUpdate AutomaticCreationFromForksAllowed: paramsChecker.lookupAllowReviewAppsFromForks(), } - return params, nil + return params } type paramsChecker struct { From 763a3e4455ea4bde302ba8872a51f117e59bbbf0 Mon Sep 17 00:00:00 2001 From: Yohann Bacha Date: Thu, 23 Feb 2023 15:41:03 +0100 Subject: [PATCH 12/21] feat(review-apps): errors have even more context in the integration-link commands Signed-off-by: Yohann Bacha --- cmd/integration_link.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/cmd/integration_link.go b/cmd/integration_link.go index e8275d13d..3fd8bca62 100644 --- a/cmd/integration_link.go +++ b/cmd/integration_link.go @@ -44,7 +44,7 @@ var ( currentApp := detect.CurrentApp(c) err := integrationlink.Show(c.Context, currentApp) if err != nil { - errorQuit(errgo.Notef(err, "error while showing integration link details")) + errorQuit(err) } return nil }, @@ -133,7 +133,7 @@ List of available integrations: if c.NumFlags() == 0 { params, err = interactiveCreate() if err != nil { - errorQuit(errgo.Notef(err, "error with the interactive creation of the integration link")) + errorQuit(err) } } else { branch := c.String("branch") @@ -291,11 +291,11 @@ List of available integrations: if allowReviewAppsFromForks && !awareOfSecurityRisks { stillAllowed, err := askForConfirmation(reviewAppsFromForksSecurityWarning) if err != nil { - errorQuit(errgo.Notef(err, "error while asking for security concerns awareness")) + errorQuit(err) } err = c.Set("allow-review-apps-from-forks", strconv.FormatBool(stillAllowed)) if err != nil { - errorQuit(err) + errorQuit(errgo.Notef(err, "error updating if review apps creation from forks are allowed")) } } @@ -431,7 +431,7 @@ func interactiveCreate() (scalingo.SCMRepoLinkCreateParams, error) { }{} err := survey.Ask(qs, &answers) if err != nil { - return params, err + return params, errgo.Notef(err, "error enquiring about branch and automatic review apps deployment") } t := true @@ -451,7 +451,7 @@ func interactiveCreate() (scalingo.SCMRepoLinkCreateParams, error) { Default: destroyOnClose, }, &destroyOnClose, nil) if err != nil { - return params, err + return params, errgo.Notef(err, "error enquiring about destroy on close") } params.DestroyOnCloseEnabled = &destroyOnClose if destroyOnClose { @@ -461,7 +461,7 @@ func interactiveCreate() (scalingo.SCMRepoLinkCreateParams, error) { Default: "0", }, &answerHoursBeforeDestroyOnClose, survey.WithValidator(validateHoursBeforeDelete)) if err != nil { - return params, err + return params, errgo.Notef(err, "error enquiring about review apps destroy delay") } hoursBeforeDestroyOnClose64, _ := strconv.ParseUint(answerHoursBeforeDestroyOnClose, 10, 32) hoursBeforeDestroyOnClose := uint(hoursBeforeDestroyOnClose64) @@ -474,7 +474,7 @@ func interactiveCreate() (scalingo.SCMRepoLinkCreateParams, error) { Default: destroyOnStale, }, &destroyOnStale, nil) if err != nil { - return params, err + return params, errgo.Notef(err, "error enquiring about stale review apps destroy") } params.DestroyStaleEnabled = &destroyOnStale if destroyOnStale { @@ -484,7 +484,7 @@ func interactiveCreate() (scalingo.SCMRepoLinkCreateParams, error) { Default: "0", }, &answerHoursBeforeDestroyOnStale, survey.WithValidator(validateHoursBeforeDelete)) if err != nil { - return params, err + return params, errgo.Notef(err, "error enquiring about stale review apps destroy") } hoursBeforeDestroyOnStale64, _ := strconv.ParseUint(answerHoursBeforeDestroyOnStale, 10, 32) hoursBeforeDestroyOnStale := uint(hoursBeforeDestroyOnStale64) @@ -500,7 +500,7 @@ func interactiveCreate() (scalingo.SCMRepoLinkCreateParams, error) { }, &forksAllowed, nil) if err != nil { - return params, err + return params, errgo.Notef(err, "error enquiring about automatic review apps creation from forks") } params.AutomaticCreationFromForksAllowed = &forksAllowed @@ -514,7 +514,7 @@ func validateHoursBeforeDelete(ans interface{}) error { } i, err := strconv.ParseInt(str, 10, 32) if err != nil { - return err + return errgo.Notef(err, "error parsing hours") } if i < 0 { return errors.New("must be positive") From 94c4574e0c71ddb8d231c2825d9eafbc5fe0a882 Mon Sep 17 00:00:00 2001 From: Yohann Bacha Date: Thu, 23 Feb 2023 15:41:56 +0100 Subject: [PATCH 13/21] feat(review-apps): more information on the warning message Signed-off-by: Yohann Bacha --- cmd/integration_link.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/integration_link.go b/cmd/integration_link.go index 3fd8bca62..9a454c14d 100644 --- a/cmd/integration_link.go +++ b/cmd/integration_link.go @@ -23,7 +23,7 @@ import ( ) var ( - reviewAppsFromForksSecurityWarning = "Only allow automatic review apps deployments from forks if you trust the owners of those forks, as this could lead to security issues" + reviewAppsFromForksSecurityWarning = "Only allow automatic review apps deployments from forks if you trust the owners of those forks, as this could lead to security issues. More info here: https://doc.scalingo.com/platform/app/review-apps#addons-collaborators-and-environment-variables" integrationLinkShowCommand = cli.Command{ Name: "integration-link", From c0d3f61c8a82c8e3526e6c7fbe6fc7464f369aec Mon Sep 17 00:00:00 2001 From: Yohann Bacha Date: Thu, 23 Feb 2023 15:59:18 +0100 Subject: [PATCH 14/21] feat(review-apps): applying pr returns Signed-off-by: Yohann Bacha --- cmd/integration_link.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/integration_link.go b/cmd/integration_link.go index 9a454c14d..b96ee8dd9 100644 --- a/cmd/integration_link.go +++ b/cmd/integration_link.go @@ -491,7 +491,7 @@ func interactiveCreate() (scalingo.SCMRepoLinkCreateParams, error) { params.HoursBeforeDeleteStale = &hoursBeforeDestroyOnStale } - io.Warning("Only allow automatic review apps deployments from forks if you trust the owners of those forks, as this could lead to security issues") + io.Warning(reviewAppsFromForksSecurityWarning) var forksAllowed bool err = survey.AskOne(&survey.Confirm{ From 8def4b9b452ead3148b95f02de5aba9f958234c0 Mon Sep 17 00:00:00 2001 From: Yohann Bacha Date: Thu, 23 Feb 2023 16:22:10 +0100 Subject: [PATCH 15/21] fix(review-apps): trying to fix the linter error Signed-off-by: Yohann Bacha --- cmd/integration_link.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/integration_link.go b/cmd/integration_link.go index b96ee8dd9..ab0670f42 100644 --- a/cmd/integration_link.go +++ b/cmd/integration_link.go @@ -3,7 +3,6 @@ package cmd import ( "errors" "fmt" - "gopkg.in/errgo.v1" "net/url" "os" "strconv" @@ -20,6 +19,7 @@ import ( "github.com/Scalingo/go-scalingo/v6" "github.com/Scalingo/go-scalingo/v6/http" scalingoerrors "github.com/Scalingo/go-utils/errors" + "gopkg.in/errgo.v1" ) var ( From 60f78fa98808eb081ebe2d65f29e1dff5d65b638 Mon Sep 17 00:00:00 2001 From: Yohann Bacha Date: Thu, 23 Feb 2023 16:32:28 +0100 Subject: [PATCH 16/21] fix(review-apps): trying to fix the linter error again Signed-off-by: Yohann Bacha --- cmd/integration_link.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/integration_link.go b/cmd/integration_link.go index ab0670f42..cdcde2a39 100644 --- a/cmd/integration_link.go +++ b/cmd/integration_link.go @@ -7,6 +7,8 @@ import ( "os" "strconv" + "gopkg.in/errgo.v1" + "github.com/AlecAivazis/survey/v2" "github.com/urfave/cli/v2" @@ -19,7 +21,6 @@ import ( "github.com/Scalingo/go-scalingo/v6" "github.com/Scalingo/go-scalingo/v6/http" scalingoerrors "github.com/Scalingo/go-utils/errors" - "gopkg.in/errgo.v1" ) var ( From 8bac5fce173f08172768df9deef4cd01c0931e9b Mon Sep 17 00:00:00 2001 From: Yohann Bacha Date: Mon, 27 Feb 2023 11:50:16 +0100 Subject: [PATCH 17/21] fix: do not prompt when review apps are not deployed --- cmd/integration_link.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/integration_link.go b/cmd/integration_link.go index cdcde2a39..8695cb980 100644 --- a/cmd/integration_link.go +++ b/cmd/integration_link.go @@ -173,7 +173,7 @@ List of available integrations: awareOfSecurityRisks := c.Bool("aware-of-security-risks") - if allowReviewAppsFromForks && !awareOfSecurityRisks { + if deployReviewApps && allowReviewAppsFromForks && !awareOfSecurityRisks { allowReviewAppsFromForks, err = askForConfirmation(reviewAppsFromForksSecurityWarning) if err != nil { errorQuit(err) @@ -295,6 +295,7 @@ List of available integrations: errorQuit(err) } err = c.Set("allow-review-apps-from-forks", strconv.FormatBool(stillAllowed)) + c.Value("allow-review-apps-from-forks") if err != nil { errorQuit(errgo.Notef(err, "error updating if review apps creation from forks are allowed")) } @@ -395,6 +396,7 @@ List of available integrations: currentApp := detect.CurrentApp(c) pullRequestID := c.Args().First() + err := integrationlink.ManualReviewApp(c.Context, currentApp, pullRequestID) if err != nil { errorQuit(err) From 6d032af74d5b6b383ffea72b4b9da73915a6f8c3 Mon Sep 17 00:00:00 2001 From: Yohann Bacha Date: Mon, 27 Feb 2023 12:05:37 +0100 Subject: [PATCH 18/21] feat(review-apps): added changelog entry Signed-off-by: Yohann Bacha --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 021cc60e7..cd0c9bd26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * feat(deployments): add Image Size to the list of deployments * fix(backups): backup flag is not required * build(publish): replace `rm-dist` with `clean` +* feat(review-apps): permit automatic review app deployment from forks ### 1.27.2 From ee8190d7a7548374eac58036bbb4462ae9d332f4 Mon Sep 17 00:00:00 2001 From: Yohann Bacha Date: Mon, 27 Feb 2023 13:03:21 +0100 Subject: [PATCH 19/21] fix(review-apps): change forks allowed on update depending on user awareness Signed-off-by: Yohann Bacha --- cmd/integration_link.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/cmd/integration_link.go b/cmd/integration_link.go index 8695cb980..975ac5317 100644 --- a/cmd/integration_link.go +++ b/cmd/integration_link.go @@ -289,20 +289,17 @@ List of available integrations: awareOfSecurityRisks := c.Bool("aware-of-security-risks") + currentApp := detect.CurrentApp(c) + params := integrationlink.CheckAndFillParams(c) + if allowReviewAppsFromForks && !awareOfSecurityRisks { stillAllowed, err := askForConfirmation(reviewAppsFromForksSecurityWarning) if err != nil { errorQuit(err) } - err = c.Set("allow-review-apps-from-forks", strconv.FormatBool(stillAllowed)) - c.Value("allow-review-apps-from-forks") - if err != nil { - errorQuit(errgo.Notef(err, "error updating if review apps creation from forks are allowed")) - } - } - currentApp := detect.CurrentApp(c) - params := integrationlink.CheckAndFillParams(c) + params.AutomaticCreationFromForksAllowed = &stillAllowed + } err := integrationlink.Update(c.Context, currentApp, *params) if err != nil { From 0878f3d4481aac88bc818c403d644e952adbd95f Mon Sep 17 00:00:00 2001 From: Yohann Bacha Date: Mon, 27 Feb 2023 17:52:42 +0100 Subject: [PATCH 20/21] fix(review-apps): refactoring the prompt message Signed-off-by: Yohann Bacha --- cmd/integration_link.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/integration_link.go b/cmd/integration_link.go index 975ac5317..5a0e4a923 100644 --- a/cmd/integration_link.go +++ b/cmd/integration_link.go @@ -174,7 +174,7 @@ List of available integrations: awareOfSecurityRisks := c.Bool("aware-of-security-risks") if deployReviewApps && allowReviewAppsFromForks && !awareOfSecurityRisks { - allowReviewAppsFromForks, err = askForConfirmation(reviewAppsFromForksSecurityWarning) + allowReviewAppsFromForks, err = askForConfirmationToAllowReviewAppsFromForks() if err != nil { errorQuit(err) } @@ -293,7 +293,7 @@ List of available integrations: params := integrationlink.CheckAndFillParams(c) if allowReviewAppsFromForks && !awareOfSecurityRisks { - stillAllowed, err := askForConfirmation(reviewAppsFromForksSecurityWarning) + stillAllowed, err := askForConfirmationToAllowReviewAppsFromForks() if err != nil { errorQuit(err) } @@ -522,12 +522,12 @@ func validateHoursBeforeDelete(ans interface{}) error { return nil } -func askForConfirmation(message string) (bool, error) { - io.Warning(message) +func askForConfirmationToAllowReviewAppsFromForks() (bool, error) { + io.Warning(reviewAppsFromForksSecurityWarning) var confirmed bool err := survey.AskOne(&survey.Confirm{ - Message: "Are your sure?", + Message: "Allow review apps from forks?", Default: false, }, &confirmed, nil) From 7d372a763266c188968a0bb458b6a1730ecf28b6 Mon Sep 17 00:00:00 2001 From: aurelien-reeves-scalingo Date: Tue, 28 Feb 2023 09:10:39 +0100 Subject: [PATCH 21/21] Improve warning message and prompt --- cmd/integration_link.go | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/cmd/integration_link.go b/cmd/integration_link.go index 5a0e4a923..a8899e87d 100644 --- a/cmd/integration_link.go +++ b/cmd/integration_link.go @@ -83,7 +83,7 @@ List of available integrations: - github => GitHub.com - github-enterprise => GitHub Enterprise (private instance) - gitlab => GitLab.com -- gitlab-self-hosted => GitLab Self-hosted (private instance) +- gitlab-self-hosted => GitLab Self-hosted (private instance) `, Examples: []string{ "scalingo --app my-app integration-link-create https://gitlab.com/gitlab-org/gitlab-ce", @@ -491,14 +491,7 @@ func interactiveCreate() (scalingo.SCMRepoLinkCreateParams, error) { params.HoursBeforeDeleteStale = &hoursBeforeDestroyOnStale } - io.Warning(reviewAppsFromForksSecurityWarning) - var forksAllowed bool - - err = survey.AskOne(&survey.Confirm{ - Message: "Allow review apps to be created from forks:", - Default: false, - }, &forksAllowed, nil) - + forksAllowed, err := askForConfirmationToAllowReviewAppsFromForks() if err != nil { return params, errgo.Notef(err, "error enquiring about automatic review apps creation from forks") } @@ -523,11 +516,13 @@ func validateHoursBeforeDelete(ans interface{}) error { } func askForConfirmationToAllowReviewAppsFromForks() (bool, error) { + fmt.Println() io.Warning(reviewAppsFromForksSecurityWarning) - var confirmed bool + fmt.Println() + var confirmed bool err := survey.AskOne(&survey.Confirm{ - Message: "Allow review apps from forks?", + Message: "Allow automatic creation of review apps from forks?", Default: false, }, &confirmed, nil)