fix(internal/librarian): handle slashes in library IDs (#2463)

This ensures that we don't create nested output directories during the
generate command.

Fixes #2305

Author: jskeet

internal/librarian/generate.go

@@ -21,6 +21,7 @@ import (
 	"log/slog"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/googleapis/librarian/internal/config"
 	"github.com/googleapis/librarian/internal/docker"
@@ -173,9 +174,10 @@ func (r *generateRunner) run(ctx context.Context) error {
 //
 // Returns the last generated commit before the generation and error, if any.
 func (r *generateRunner) generateSingleLibrary(ctx context.Context, libraryID, outputDir string) (string, error) {
+	safeLibraryDirectory := getSafeDirectoryName(libraryID)
 	if r.needsConfigure() {
 		slog.Info("library not configured, start initial configuration", "library", r.library)
-		configureOutputDir := filepath.Join(outputDir, libraryID, "configure")
+		configureOutputDir := filepath.Join(outputDir, safeLibraryDirectory, "configure")
 		if err := os.MkdirAll(configureOutputDir, 0755); err != nil {
 			return "", err
 		}
@@ -201,7 +203,7 @@ func (r *generateRunner) generateSingleLibrary(ctx context.Context, libraryID, o
 	// For each library, create a separate output directory. This avoids
 	// libraries interfering with each other, and makes it easier to see what
 	// was generated for each library when debugging.
-	libraryOutputDir := filepath.Join(outputDir, libraryID)
+	libraryOutputDir := filepath.Join(outputDir, safeLibraryDirectory)
 	if err := os.MkdirAll(libraryOutputDir, 0755); err != nil {
 		return "", err
 	}
@@ -449,3 +451,18 @@ func setAllAPIStatus(state *config.LibrarianState, status string) {
 		}
 	}
 }
+
+// getSafeDirectoryName returns a directory name which doesn't contain slashes
+// based on a library ID. This avoids cases where a library ID contains
+// slashes but we want generateSingleLibrary to create a directory which
+// is not a subdirectory of some other directory. For example, if there
+// are library IDs of "pubsub" and "pubsub/v2" we don't want to create
+// "output/pubsub/v2" and then "output/pubsub" later. This function does
+// not protect against malicious library IDs, e.g. ".", ".." or deliberate
+// collisions (e.g. "pubsub/v2" and "pubsub-slash-v2").
+//
+// The exact implementation may change over time - nothing should rely on this.
+// The current implementation simply replaces any slashes with "-slash-".
+func getSafeDirectoryName(libraryID string) string {
+	return strings.ReplaceAll(libraryID, "/", "-slash-")
+}

internal/librarian/generate_test.go

@@ -1217,3 +1217,34 @@ func TestGetExistingSrc(t *testing.T) {
 		})
 	}
 }
+
+func TestGetSafeDirectoryName(t *testing.T) {
+	for _, test := range []struct {
+		name string
+		id   string
+		want string
+	}{
+		{
+			name: "simple",
+			id:   "pubsub",
+			want: "pubsub",
+		},
+		{
+			name: "nested",
+			id:   "pubsub/v2",
+			want: "pubsub-slash-v2",
+		},
+		{
+			name: "deeply nested",
+			id:   "compute/metadata/v2",
+			want: "compute-slash-metadata-slash-v2",
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			got := getSafeDirectoryName(test.id)
+			if test.want != got {
+				t.Errorf("getSafeDirectoryName() = %q; want %q", got, test.want)
+			}
+		})
+	}
+}