Preventing dolt_checkout from creating new branches in read-only databases.

Fixes: https://github.com/dolthub/dolt/issues/6785

go/cmd/dolt/cli/arg_parser_helpers.go

@@ -151,7 +151,7 @@ func CreateCleanArgParser() *argparser.ArgParser {
 
 func CreateCheckoutArgParser() *argparser.ArgParser {
 	ap := argparser.NewArgParserWithVariableArgs("checkout")
-	ap.SupportsString(CheckoutCoBranch, "", "branch", "Create a new branch named {{.LessThan}}new_branch{{.GreaterThan}} and start it at {{.LessThan}}start_point{{.GreaterThan}}.")
+	ap.SupportsString(CheckoutCreateBranch, "", "branch", "Create a new branch named {{.LessThan}}new_branch{{.GreaterThan}} and start it at {{.LessThan}}start_point{{.GreaterThan}}.")
 	ap.SupportsFlag(ForceFlag, "f", "If there is any changes in working set, the force flag will wipe out the current changes and checkout the new branch.")
 	ap.SupportsString(TrackFlag, "t", "", "When creating a new branch, set up 'upstream' configuration.")
 	return ap

go/cmd/dolt/cli/flags.go

@@ -17,53 +17,53 @@ package cli
 // Constants for command line flags names. These tend to be used in multiple places, so defining
 // them low in the package dependency tree makes sense.
 const (
-	AbortParam       = "abort"
-	AllFlag          = "all"
-	AllowEmptyFlag   = "allow-empty"
-	AmendFlag        = "amend"
-	AuthorParam      = "author"
-	BranchParam      = "branch"
-	CachedFlag       = "cached"
-	CheckoutCoBranch = "b"
-	CommitFlag       = "commit"
-	CopyFlag         = "copy"
-	DateParam        = "date"
-	DecorateFlag     = "decorate"
-	DeleteFlag       = "delete"
-	DeleteForceFlag  = "D"
-	DryRunFlag       = "dry-run"
-	ForceFlag        = "force"
-	HardResetParam   = "hard"
-	HostFlag         = "host"
-	ListFlag         = "list"
-	MergesFlag       = "merges"
-	MessageArg       = "message"
-	MinParentsFlag   = "min-parents"
-	MoveFlag         = "move"
-	NoCommitFlag     = "no-commit"
-	NoEditFlag       = "no-edit"
-	NoFFParam        = "no-ff"
-	NoPrettyFlag     = "no-pretty"
-	NoTLSFlag        = "no-tls"
-	NotFlag          = "not"
-	NumberFlag       = "number"
-	OneLineFlag      = "oneline"
-	OursFlag         = "ours"
-	OutputOnlyFlag   = "output-only"
-	ParentsFlag      = "parents"
-	PasswordFlag     = "password"
-	PortFlag         = "port"
-	PruneFlag        = "prune"
-	RemoteParam      = "remote"
-	SetUpstreamFlag  = "set-upstream"
-	ShallowFlag      = "shallow"
-	ShowIgnoredFlag  = "ignored"
-	SkipEmptyFlag    = "skip-empty"
-	SoftResetParam   = "soft"
-	SquashParam      = "squash"
-	TablesFlag       = "tables"
-	TheirsFlag       = "theirs"
-	TrackFlag        = "track"
-	UpperCaseAllFlag = "ALL"
-	UserFlag         = "user"
+	AbortParam           = "abort"
+	AllFlag              = "all"
+	AllowEmptyFlag       = "allow-empty"
+	AmendFlag            = "amend"
+	AuthorParam          = "author"
+	BranchParam          = "branch"
+	CachedFlag           = "cached"
+	CheckoutCreateBranch = "b"
+	CommitFlag           = "commit"
+	CopyFlag             = "copy"
+	DateParam            = "date"
+	DecorateFlag         = "decorate"
+	DeleteFlag           = "delete"
+	DeleteForceFlag      = "D"
+	DryRunFlag           = "dry-run"
+	ForceFlag            = "force"
+	HardResetParam       = "hard"
+	HostFlag             = "host"
+	ListFlag             = "list"
+	MergesFlag           = "merges"
+	MessageArg           = "message"
+	MinParentsFlag       = "min-parents"
+	MoveFlag             = "move"
+	NoCommitFlag         = "no-commit"
+	NoEditFlag           = "no-edit"
+	NoFFParam            = "no-ff"
+	NoPrettyFlag         = "no-pretty"
+	NoTLSFlag            = "no-tls"
+	NotFlag              = "not"
+	NumberFlag           = "number"
+	OneLineFlag          = "oneline"
+	OursFlag             = "ours"
+	OutputOnlyFlag       = "output-only"
+	ParentsFlag          = "parents"
+	PasswordFlag         = "password"
+	PortFlag             = "port"
+	PruneFlag            = "prune"
+	RemoteParam          = "remote"
+	SetUpstreamFlag      = "set-upstream"
+	ShallowFlag          = "shallow"
+	ShowIgnoredFlag      = "ignored"
+	SkipEmptyFlag        = "skip-empty"
+	SoftResetParam       = "soft"
+	SquashParam          = "squash"
+	TablesFlag           = "tables"
+	TheirsFlag           = "theirs"
+	TrackFlag            = "track"
+	UpperCaseAllFlag     = "ALL"
+	UserFlag             = "user"
 )

go/cmd/dolt/commands/checkout.go

@@ -106,15 +106,15 @@ func (cmd CheckoutCmd) Exec(ctx context.Context, commandStr string, args []strin
 		return 1
 	}
 
-	branchOrTrack := apr.Contains(cli.CheckoutCoBranch) || apr.Contains(cli.TrackFlag)
+	branchOrTrack := apr.Contains(cli.CheckoutCreateBranch) || apr.Contains(cli.TrackFlag)
 	if (branchOrTrack && apr.NArg() > 1) || (!branchOrTrack && apr.NArg() == 0) {
 		usagePrt()
 		return 1
 	}
 
 	var branchName string
-	if apr.Contains(cli.CheckoutCoBranch) {
-		branchName, _ = apr.GetValue(cli.CheckoutCoBranch)
+	if apr.Contains(cli.CheckoutCreateBranch) {
+		branchName, _ = apr.GetValue(cli.CheckoutCreateBranch)
 	} else if apr.Contains(cli.TrackFlag) {
 		if apr.NArg() > 0 {
 			usagePrt()

go/libraries/doltcore/sqle/dprocedures/dolt_checkout.go

@@ -71,7 +71,7 @@ func doDoltCheckout(ctx *sql.Context, args []string) (statusCode int, successMes
 		return 1, "", err
 	}
 
-	branchOrTrack := apr.Contains(cli.CheckoutCoBranch) || apr.Contains(cli.TrackFlag)
+	branchOrTrack := apr.Contains(cli.CheckoutCreateBranch) || apr.Contains(cli.TrackFlag)
 	if apr.Contains(cli.TrackFlag) && apr.NArg() > 0 {
 		return 1, "", errors.New("Improper usage.")
 	}
@@ -85,6 +85,15 @@ func doDoltCheckout(ctx *sql.Context, args []string) (statusCode int, successMes
 		return 1, "", fmt.Errorf("Could not load database %s", currentDbName)
 	}
 
+	// Prevent the -b option from being used to create new branches on read-only databases
+	readOnlyDatabase, err := isReadOnlyDatabase(ctx, currentDbName)
+	if err != nil {
+		return 1, "", err
+	}
+	if apr.Contains(cli.CheckoutCreateBranch) && readOnlyDatabase {
+		return 1, "", fmt.Errorf("unable to create new branch in a read-only database")
+	}
+
 	updateHead := apr.Contains(cli.MoveFlag)
 
 	var rsc doltdb.ReplicationStatusController
@@ -190,6 +199,19 @@ func doDoltCheckout(ctx *sql.Context, args []string) (statusCode int, successMes
 	return 0, successMessage, nil
 }
 
+// isReadOnlyDatabase returns true if the named database is a read-only database. An error is returned
+// if any issues are encountered while looking up the named database.
+func isReadOnlyDatabase(ctx *sql.Context, dbName string) (bool, error) {
+	doltSession := dsess.DSessFromSess(ctx.Session)
+	db, err := doltSession.Provider().Database(ctx, dbName)
+	if err != nil {
+		return false, err
+	}
+
+	rodb, ok := db.(sql.ReadOnlyDatabase)
+	return ok && rodb.IsReadOnly(), nil
+}
+
 // createWorkingSetForLocalBranch will make a new working set for a local
 // branch ref if one does not already exist. Can be used to fix up local branch
 // state when branches have been created without working sets in the past.
@@ -324,7 +346,7 @@ func checkoutNewBranch(ctx *sql.Context, dbName string, dbData env.DbData, apr *
 		newBranchName = remoteBranchName
 	}
 
-	if newBranch, ok := apr.GetValue(cli.CheckoutCoBranch); ok {
+	if newBranch, ok := apr.GetValue(cli.CheckoutCreateBranch); ok {
 		if len(newBranch) == 0 {
 			return "", "", ErrEmptyBranchName
 		}

go/libraries/doltcore/sqle/dprocedures/dolt_reset.go

@@ -62,19 +62,15 @@ func doDoltReset(ctx *sql.Context, args []string) (int, error) {
 		return 1, fmt.Errorf("error: --%s and --%s are mutually exclusive options.", cli.HardResetParam, cli.SoftResetParam)
 	}
 
-	provider := dSess.Provider()
-	db, err := provider.Database(ctx, dbName)
-	if err != nil {
-		return 1, err
-	}
-
 	// Disallow manipulating any roots for read-only databases – changing the branch
 	// HEAD would allow changing data, and working set and index shouldn't ever have
 	// any contents for a read-only database.
-	if rodb, ok := db.(sql.ReadOnlyDatabase); ok {
-		if rodb.IsReadOnly() {
-			return 1, fmt.Errorf("unable to reset HEAD in read-only databases")
-		}
+	isReadOnly, err := isReadOnlyDatabase(ctx, dbName)
+	if err != nil {
+		return 1, err
+	}
+	if isReadOnly {
+		return 1, fmt.Errorf("unable to reset HEAD in read-only databases")
 	}
 
 	// Get all the needed roots.

go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go

@@ -1701,6 +1701,17 @@ func TestDoltCheckout(t *testing.T) {
 			enginetest.TestScript(t, h, script)
 		}()
 	}
+
+	h := newDoltHarness(t)
+	defer h.Close()
+	engine, err := h.NewEngine(t)
+	require.NoError(t, err)
+	readOnlyEngine, err := h.NewReadOnlyEngine(engine.EngineAnalyzer().Catalog.DbProvider)
+	require.NoError(t, err)
+
+	for _, script := range DoltCheckoutReadOnlyScripts {
+		enginetest.TestScriptWithEngine(t, readOnlyEngine, h, script)
+	}
 }
 
 func TestDoltCheckoutPrepared(t *testing.T) {
@@ -1711,6 +1722,17 @@ func TestDoltCheckoutPrepared(t *testing.T) {
 			enginetest.TestScriptPrepared(t, h, script)
 		}()
 	}
+
+	h := newDoltHarness(t)
+	defer h.Close()
+	engine, err := h.NewEngine(t)
+	require.NoError(t, err)
+	readOnlyEngine, err := h.NewReadOnlyEngine(engine.EngineAnalyzer().Catalog.DbProvider)
+	require.NoError(t, err)
+
+	for _, script := range DoltCheckoutReadOnlyScripts {
+		enginetest.TestScriptWithEnginePrepared(t, readOnlyEngine, h, script)
+	}
 }
 
 func TestDoltBranch(t *testing.T) {

go/libraries/doltcore/sqle/enginetest/dolt_queries.go

@@ -2676,6 +2676,18 @@ var DoltCheckoutScripts = []queries.ScriptTest{
 	},
 }
 
+var DoltCheckoutReadOnlyScripts = []queries.ScriptTest{
+	{
+		Name: "dolt checkout -b returns an error for read-only databases",
+		Assertions: []queries.ScriptTestAssertion{
+			{
+				Query:          "call dolt_checkout('-b', 'newBranch');",
+				ExpectedErrStr: "unable to create new branch in a read-only database",
+			},
+		},
+	},
+}
+
 var DoltInfoSchemaScripts = []queries.ScriptTest{
 	{
 		Name: "info_schema changes with dolt_checkout",

integration-tests/bats/sql-server.bats

@@ -852,6 +852,11 @@ SQL
     [ $status -ne 0 ]
     [[ $output =~ "read-only" ]] || false
 
+    # dolt checkout can't create new branches on a read only database
+    run dolt sql-client --use-db "repo1/$hash" -u dolt -P $PORT -q "call dolt_checkout('-b', 'newBranch');"
+    [ $status -ne 0 ]
+    [[ $output =~ "unable to create new branch in a read-only database" ]] || false
+
     # server should still be alive after an error
     run dolt sql-client --use-db "repo1/$hash" -u dolt -P $PORT -q "select count(*) from test"
     [ $status -eq 0 ]