Follow-ups from gitpod-io#20269 (gitpod-io#20272)

* Follow-ups from gitpod-io#20269 * `matchesNewWorkspaceIdExactly` to handle `undefined`
cmcc-ict · Oct 7, 2024 · 9d42ec3 · 9d42ec3
1 parent 3f0fc73
commit 9d42ec3
Show file tree

Hide file tree

Showing 5 changed files with 13 additions and 12 deletions.
diff --git a/components/gitpod-protocol/src/util/gitpod-host-url.ts b/components/gitpod-protocol/src/util/gitpod-host-url.ts
@@ -16,7 +16,7 @@ const baseWorkspaceIDRegex =
     "(([a-f][0-9a-f]{7}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})|([0-9a-z]{2,16}-[0-9a-z]{2,16}-[0-9a-z]{8,11}))";
 
 // this pattern matches v4 UUIDs as well as the new generated workspace ids (e.g. pink-panda-ns35kd21)
-export const workspaceIDRegex = RegExp(`^(?:debug-)?${baseWorkspaceIDRegex}$`);
+const workspaceIDRegex = RegExp(`^(?:debug-)?${baseWorkspaceIDRegex}$`);
 
 // this pattern matches URL prefixes of workspaces
 const workspaceUrlPrefixRegex = RegExp(`^(([0-9]{4,6}|debug)-)?${baseWorkspaceIDRegex}\\.`);

diff --git a/components/gitpod-protocol/src/util/parse-workspace-id.spec.ts b/components/gitpod-protocol/src/util/parse-workspace-id.spec.ts
@@ -81,5 +81,9 @@ export class ParseWorkspaceIdTest {
         const actual = matchesNewWorkspaceIdExactly("moccasin-ferret-15599b3");
         expect(actual).to.be.false;
     }
+    @test public matchesWorkspaceIdExactly_new_negative_empty() {
+        const actual = matchesNewWorkspaceIdExactly(undefined);
+        expect(actual).to.be.false;
+    }
 }
 module.exports = new ParseWorkspaceIdTest();
diff --git a/components/gitpod-protocol/src/util/parse-workspace-id.ts b/components/gitpod-protocol/src/util/parse-workspace-id.ts
@@ -33,7 +33,7 @@ export const parseWorkspaceIdFromHostname = function (hostname: string) {
     }
 };
 
-/** Equalls UUIDv4 (and REGEX_WORKSPACE_ID_LEGACY!) */
+/** Equals UUIDv4 (and REGEX_WORKSPACE_ID_LEGACY!) */
 const REGEX_INSTANCE_ID = /[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/;
 const REGEX_INSTANCE_ID_EXACT = new RegExp(`^${REGEX_INSTANCE_ID.source}$`);
 
@@ -49,6 +49,10 @@ export const matchesInstanceIdOrLegacyWorkspaceIdExactly = function (maybeId: st
  * @param maybeWorkspaceId
  * @returns
  */
-export const matchesNewWorkspaceIdExactly = function (maybeWorkspaceId: string): boolean {
+export const matchesNewWorkspaceIdExactly = function (maybeWorkspaceId?: string): boolean {
+    if (!maybeWorkspaceId) {
+        return false;
+    }
+
     return REGEX_WORKSPACE_ID_EXACT.test(maybeWorkspaceId);
 };
diff --git a/components/server/src/api/workspace-service-api.ts b/components/server/src/api/workspace-service-api.ts
@@ -58,15 +58,7 @@ import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messag
 import { ContextService } from "../workspace/context-service";
 import { UserService } from "../user/user-service";
 import { ContextParser } from "../workspace/context-parser-service";
-import { workspaceIDRegex } from "@gitpod/gitpod-protocol/lib/util/gitpod-host-url";
-
-const isWorkspaceId = (workspaceId?: string) => {
-    if (!workspaceId) {
-        return false;
-    }
-
-    return workspaceIDRegex.test(workspaceId);
-};
+import { matchesNewWorkspaceIdExactly as isWorkspaceId } from "@gitpod/gitpod-protocol/lib/util/parse-workspace-id";
 
 @injectable()
 export class WorkspaceServiceAPI implements ServiceImpl<typeof WorkspaceServiceInterface> {

diff --git a/components/server/src/authorization/spicedb-authorizer.ts b/components/server/src/authorization/spicedb-authorizer.ts
@@ -105,6 +105,7 @@ export class SpiceDBAuthorizer {
                 const permitted = response.permissionship === v1.CheckPermissionResponse_Permissionship.HAS_PERMISSION;
                 return { permitted, checkedAt: response.checkedAt?.token };
             } catch (err) {
+                // we should not consider users supplying invalid requests as internal server errors
                 if (isGrpcError(err) && err.code === grpc.status.INVALID_ARGUMENT) {
                     throw new ApplicationError(ErrorCodes.BAD_REQUEST, `Invalid request for permission check: ${err}`);
                 }