fix #12208: in workspace project env var management

Author: akosyakov

components/gitpod-cli/cmd/env.go

@@ -78,6 +78,7 @@ delete environment variables with a repository pattern of */foo, foo/* or */*.
 
 type connectToServerResult struct {
 	repositoryPattern string
+	wsInfo            *supervisor.WorkspaceInfoResponse
 	client            *serverapi.APIoverJSONRPC
 }
 
@@ -97,17 +98,20 @@ func connectToServer(ctx context.Context) (*connectToServerResult, error) {
 		return nil, xerrors.New("repository info is missing owner")
 	}
 	if wsinfo.Repository.Name == "" {
-		return nil, xerrors.New("repository info is missing name")
+		xerrors.New("repository info is missing name")
 	}
 	repositoryPattern := wsinfo.Repository.Owner + "/" + wsinfo.Repository.Name
 	clientToken, err := supervisor.NewTokenServiceClient(supervisorConn).GetToken(ctx, &supervisor.GetTokenRequest{
 		Host: wsinfo.GitpodApi.Host,
 		Kind: "gitpod",
 		Scope: []string{
-			"function:getEnvVars",
+			"function:getWorkspaceEnvVars",
+			"function:getEnvVars", // TODO remove then getWorkspaceEnvVars is deployed
 			"function:setEnvVar",
 			"function:deleteEnvVar",
 			"resource:envVar::" + repositoryPattern + "::create/get/update/delete",
+			// TODO actually we should have here team resource scope, but we don't have it, only project id,
+			// shoud we expose it? shoudl we introduce project guard
 		},
 	})
 	if err != nil {
@@ -121,7 +125,7 @@ func connectToServer(ctx context.Context) (*connectToServerResult, error) {
 	if err != nil {
 		return nil, xerrors.Errorf("failed connecting to server: %w", err)
 	}
-	return &connectToServerResult{repositoryPattern, client}, nil
+	return &connectToServerResult{repositoryPattern, wsinfo, client}, nil
 }
 
 func getEnvs(ctx context.Context) error {
@@ -131,13 +135,17 @@ func getEnvs(ctx context.Context) error {
 	}
 	defer result.client.Close()
 
-	vars, err := result.client.GetEnvVars(ctx)
+	vars, err := result.client.GetWorkspaceEnvVars(ctx, result.wsInfo.WorkspaceId)
+	if err != nil {
+		// TODO remove then GetWorkspaceEnvVars is deployed
+		vars, err = result.client.GetEnvVars(ctx)
+	}
 	if err != nil {
 		return xerrors.Errorf("failed to fetch env vars from server: %w", err)
 	}
 
 	for _, v := range vars {
-		printVar(v, exportEnvs)
+		printVar(v.Name, v.Value, exportEnvs)
 	}
 
 	return nil
@@ -163,7 +171,7 @@ func setEnvs(ctx context.Context, args []string) error {
 			if err != nil {
 				return err
 			}
-			printVar(v, exportEnvs)
+			printVar(v.Name, v.Value, exportEnvs)
 			return nil
 		})
 	}
@@ -189,12 +197,12 @@ func deleteEnvs(ctx context.Context, args []string) error {
 	return g.Wait()
 }
 
-func printVar(v *serverapi.UserEnvVarValue, export bool) {
-	val := strings.Replace(v.Value, "\"", "\\\"", -1)
+func printVar(name string, value string, export bool) {
+	val := strings.Replace(value, "\"", "\\\"", -1)
 	if export {
-		fmt.Printf("export %s=\"%s\"\n", v.Name, val)
+		fmt.Printf("export %s=\"%s\"\n", name, val)
 	} else {
-		fmt.Printf("%s=%s\n", v.Name, val)
+		fmt.Printf("%s=%s\n", name, val)
 	}
 }
 

components/gitpod-protocol/go/gitpod-service.go

@@ -65,7 +65,8 @@ type APIInterface interface {
 	ClosePort(ctx context.Context, workspaceID string, port float32) (err error)
 	GetUserStorageResource(ctx context.Context, options *GetUserStorageResourceOptions) (res string, err error)
 	UpdateUserStorageResource(ctx context.Context, options *UpdateUserStorageResourceOptions) (err error)
-	GetEnvVars(ctx context.Context) (res []*UserEnvVarValue, err error)
+	GetWorkspaceEnvVars(ctx context.Context, workspaceID string) (res []*EnvVar, err error)
+	GetEnvVars(ctx context.Context) (res []*EnvVar, err error)
 	SetEnvVar(ctx context.Context, variable *UserEnvVarValue) (err error)
 	DeleteEnvVar(ctx context.Context, variable *UserEnvVarValue) (err error)
 	HasSSHPublicKey(ctx context.Context) (res bool, err error)
@@ -1128,15 +1129,35 @@ func (gp *APIoverJSONRPC) UpdateUserStorageResource(ctx context.Context, options
 	return
 }
 
+// GetWorkspaceEnvVars calls GetWorkspaceEnvVars on the server
+func (gp *APIoverJSONRPC) GetWorkspaceEnvVars(ctx context.Context, workspaceID string) (res []*EnvVar, err error) {
+	if gp == nil {
+		err = errNotConnected
+		return
+	}
+	var _params []interface{}
+
+	_params = append(_params, workspaceID)
+
+	var result []*EnvVar
+	err = gp.C.Call(ctx, "GetWorkspaceEnvVars", _params, &result)
+	if err != nil {
+		return
+	}
+	res = result
+
+	return
+}
+
 // GetEnvVars calls getEnvVars on the server
-func (gp *APIoverJSONRPC) GetEnvVars(ctx context.Context) (res []*UserEnvVarValue, err error) {
+func (gp *APIoverJSONRPC) GetEnvVars(ctx context.Context) (res []*EnvVar, err error) {
 	if gp == nil {
 		err = errNotConnected
 		return
 	}
 	var _params []interface{}
 
-	var result []*UserEnvVarValue
+	var result []*EnvVar
 	err = gp.C.Call(ctx, "getEnvVars", _params, &result)
 	if err != nil {
 		return
@@ -1978,6 +1999,13 @@ type WhitelistedRepository struct {
 	URL         string `json:"url,omitempty"`
 }
 
+// EnvVar is the EnvVar message type
+type EnvVar struct {
+	ID    string `json:"id,omitempty"`
+	Name  string `json:"name,omitempty"`
+	Value string `json:"value,omitempty"`
+}
+
 // UserEnvVarValue is the UserEnvVarValue message type
 type UserEnvVarValue struct {
 	ID                string `json:"id,omitempty"`

components/gitpod-protocol/go/mock.go

@@ -27,6 +27,7 @@ import {
     UserSSHPublicKeyValue,
     SSHPublicKeyValue,
     IDESettings,
+    EnvVarWithValue,
 } from "./protocol";
 import {
     Team,
@@ -152,6 +153,9 @@ export interface GitpodServer extends JsonRpcServer<GitpodClient>, AdminServer,
     getUserStorageResource(options: GitpodServer.GetUserStorageResourceOptions): Promise<string>;
     updateUserStorageResource(options: GitpodServer.UpdateUserStorageResourceOptions): Promise<void>;
 
+    // Workspace env vars
+    getWorkspaceEnvVars(workspaceId: string): Promise<EnvVarWithValue[]>;
+
     // User env vars
     getEnvVars(): Promise<UserEnvVarValue[]>;
     getAllEnvVars(): Promise<UserEnvVarValue[]>;

components/gitpod-protocol/src/gitpod-service.ts

@@ -311,6 +311,8 @@ export namespace NamedWorkspaceFeatureFlag {
     }
 }
 
+export type EnvVar = UserEnvVar | ProjectEnvVarWithValue;
+
 export interface EnvVarWithValue {
     name: string;
     value: string;

components/gitpod-protocol/src/protocol.ts

@@ -89,6 +89,7 @@ const defaultFunctions: FunctionsConfig = {
     closePort: { group: "default", points: 1 },
     getUserStorageResource: { group: "default", points: 1 },
     updateUserStorageResource: { group: "default", points: 1 },
+    getWorkspaceEnvVars: { group: "default", points: 1 },
     getEnvVars: { group: "default", points: 1 },
     getAllEnvVars: { group: "default", points: 1 },
     setEnvVar: { group: "default", points: 1 },

components/server/src/auth/rate-limiter.ts

@@ -162,7 +162,7 @@ import { IDEOptions } from "@gitpod/gitpod-protocol/lib/ide-protocol";
 import { PartialProject } from "@gitpod/gitpod-protocol/lib/teams-projects-protocol";
 import { ClientMetadata, traceClientMetadata } from "../websocket/websocket-connection-manager";
 import { ConfigurationService } from "../config/configuration-service";
-import { ProjectEnvVar } from "@gitpod/gitpod-protocol/lib/protocol";
+import { EnvVarWithValue, ProjectEnvVar } from "@gitpod/gitpod-protocol/lib/protocol";
 import { InstallationAdminSettings, TelemetryData } from "@gitpod/gitpod-protocol";
 import { Deferred } from "@gitpod/gitpod-protocol/lib/util/deferred";
 import { InstallationAdminTelemetryDataProvider } from "../installation-admin/telemetry-data-provider";
@@ -200,6 +200,7 @@ import {
 import { reportCentralizedPermsValidation } from "../prometheus-metrics";
 import { RegionService } from "./region-service";
 import { isWorkspaceRegion, WorkspaceRegion } from "@gitpod/gitpod-protocol/lib/workspace-cluster";
+import { EnvVar } from "@gitpod/gitpod-protocol/src/protocol";
 
 // shortcut
 export const traceWI = (ctx: TraceContext, wi: Omit<LogContext, "userId">) => TraceContext.setOWI(ctx, wi); // userId is already taken care of in WebsocketConnectionManager
@@ -741,8 +742,11 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
         if (workspace.deleted) {
             throw new ResponseError(ErrorCodes.PERMISSION_DENIED, "Cannot (re-)start a deleted workspace.");
         }
-        const userEnvVars = this.userDB.getEnvVars(user.id);
-        const projectEnvVarsPromise = this.internalGetProjectEnvVars(workspace.projectId);
+        const envVarsPromise = this.resolveWorkspaceEnvVars(user, {
+            repository: CommitContext.is(workspace.context) ? workspace.context.repository : undefined,
+            projectId: workspace.projectId,
+            includeCensored: false,
+        });
         const projectPromise = workspace.projectId
             ? this.projectDB.findProjectById(workspace.projectId)
             : Promise.resolve(undefined);
@@ -751,20 +755,66 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
 
         options.region = await this.determineWorkspaceRegion(workspace, options.region || "");
 
+        const { workspaceEnvVars, projectEnvVars } = await envVarsPromise;
+
         // at this point we're about to actually start a new workspace
         const result = await this.workspaceStarter.startWorkspace(
             ctx,
             workspace,
             user,
             await projectPromise,
-            await userEnvVars,
-            await projectEnvVarsPromise,
+            workspaceEnvVars,
+            projectEnvVars,
             options,
         );
         traceWI(ctx, { instanceId: result.instanceID });
         return result;
     }
 
+    private async resolveWorkspaceEnvVars(
+        user: User,
+        options: {
+            repository:
+                | {
+                      owner: string;
+                      name: string;
+                  }
+                | undefined;
+            projectId: string | undefined;
+            includeCensored: boolean;
+        },
+    ): Promise<{
+        workspaceEnvVars: EnvVar[];
+        projectEnvVars: ProjectEnvVar[];
+    }> {
+        let userEnvVars = await this.userDB.getEnvVars(user.id);
+        if (options.repository) {
+            // this is a commit context, thus we can filter the env vars
+            userEnvVars = UserEnvVar.filter(userEnvVars, options.repository.owner, options.repository.name);
+        }
+        const workspaceEnvVars: Map<String, EnvVar> = new Map(userEnvVars.map((e) => [e.name, e]));
+
+        const projectEnvVars = await this.internalGetProjectEnvVars(options.projectId);
+        if (projectEnvVars.length > 0) {
+            // Instead of using an access guard for Project environment variables, we let Project owners decide whether
+            // a variable should be:
+            //   - exposed in all workspaces (even for non-Project members when the repository is public), or
+            //   - censored from all workspaces (even for Project members)
+            const availablePrjEnvVars = projectEnvVars.filter(
+                (variable) => variable.censored === options.includeCensored,
+            );
+            const withValues = await this.projectDB.getProjectEnvironmentVariableValues(availablePrjEnvVars);
+            for (const e of withValues) {
+                workspaceEnvVars.set(e.name, e);
+            }
+        }
+
+        return {
+            workspaceEnvVars: [...workspaceEnvVars.values()],
+            projectEnvVars,
+        };
+    }
+
     public async stopWorkspace(ctx: TraceContext, workspaceId: string): Promise<void> {
         traceAPIParams(ctx, { workspaceId });
         traceWI(ctx, { workspaceId });
@@ -1087,7 +1137,6 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
             const user = this.checkAndBlockUser("createWorkspace", { options });
             await this.checkTermsAcceptance();
 
-            const envVars = this.userDB.getEnvVars(user.id);
             logContext = { userId: user.id };
 
             // Credit check runs in parallel with the other operations up until we start consuming resources.
@@ -1225,18 +1274,24 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
                 throw err;
             }
 
-            let projectEnvVarsPromise = this.internalGetProjectEnvVars(workspace.projectId);
+            const envVarsPromise = this.resolveWorkspaceEnvVars(user, {
+                repository: CommitContext.is(workspace.context) ? workspace.context.repository : undefined,
+                projectId: workspace.projectId,
+                includeCensored: workspace.type === "prebuild",
+            });
             options.region = await this.determineWorkspaceRegion(workspace, options.region || "");
 
+            const { workspaceEnvVars, projectEnvVars } = await envVarsPromise;
+
             logContext.workspaceId = workspace.id;
             traceWI(ctx, { workspaceId: workspace.id });
             const startWorkspaceResult = await this.workspaceStarter.startWorkspace(
                 ctx,
                 workspace,
                 user,
                 project,
-                await envVars,
-                await projectEnvVarsPromise,
+                workspaceEnvVars,
+                projectEnvVars,
                 options,
             );
             ctx.span?.log({ event: "startWorkspaceComplete", ...startWorkspaceResult });
@@ -1837,7 +1892,36 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
         return [];
     }
 
+    async getWorkspaceEnvVars(ctx: TraceContext, workspaceId: string): Promise<EnvVarWithValue[]> {
+        const user = this.checkUser("getWorkspaceEnvVars");
+        const workspace = await this.internalGetWorkspace(workspaceId, this.workspaceDb.trace(ctx));
+        await this.guardAccess({ kind: "workspace", subject: workspace }, "get");
+
+        if (workspace.projectId) {
+            // TODO should we really leak errors to a user? or just return empty and log a warning? block a user?
+            await this.guardProjectOperation(user, workspace.projectId, "get");
+        }
+
+        const result: EnvVarWithValue[] = [];
+        const { workspaceEnvVars } = await this.resolveWorkspaceEnvVars(user, {
+            repository: undefined, // filter below with access guard
+            projectId: workspace.projectId,
+            includeCensored: false, // for now workspace cannot get hidden, i.e. may change later if someone needs it for prebuild
+        });
+        for (const value of workspaceEnvVars) {
+            if (
+                "repositoryPattern" in value &&
+                !(await this.resourceAccessGuard.canAccess({ kind: "envVar", subject: value }, "get"))
+            ) {
+                continue;
+            }
+            result.push(value);
+        }
+        return result;
+    }
+
     // Get environment variables (filter by repository pattern precedence)
+    // TODO do we really still need it, gitpod cli was the onyl client?
     async getEnvVars(ctx: TraceContext): Promise<UserEnvVarValue[]> {
         const user = this.checkUser("getEnvVars");
         const result = new Map<string, { value: UserEnvVar; score: number }>();