Allow user env vars for GitLab subgroups

components/gitpod-protocol/src/protocol.ts

@@ -427,6 +427,15 @@ export interface UserEnvVar extends UserEnvVarValue {
 }
 
 export namespace UserEnvVar {
+    export const DELIMITER = "/";
+    export const WILDCARD_ASTERISK = "*";
+    const WILDCARD_SHARP = "#"; // TODO(gpl) Where does this come from? Bc we have/had patterns as part of URLs somewhere, maybe...?
+    const MIN_PATTERN_SEGMENTS = 2;
+
+    function isWildcard(c: string): boolean {
+        return c === WILDCARD_ASTERISK || c === WILDCARD_SHARP;
+    }
+
     /**
      * @param variable
      * @returns Either a string containing an error message or undefined.
@@ -452,8 +461,8 @@ export namespace UserEnvVar {
         if (pattern.trim() === "") {
             return "Scope must not be empty.";
         }
-        const split = pattern.split("/");
-        if (split.length < 2) {
+        const split = splitRepositoryPattern(pattern);
+        if (split.length < MIN_PATTERN_SEGMENTS) {
             return "A scope must use the form 'organization/repo'.";
         }
         for (const name of split) {
@@ -466,55 +475,92 @@ export namespace UserEnvVar {
         return undefined;
     }
 
-    // DEPRECATED: Use ProjectEnvVar instead of repositoryPattern - https://github.com/gitpod-com/gitpod/issues/5322
     export function normalizeRepoPattern(pattern: string) {
         return pattern.toLocaleLowerCase();
     }
 
-    // DEPRECATED: Use ProjectEnvVar instead of repositoryPattern - https://github.com/gitpod-com/gitpod/issues/5322
-    export function score(value: UserEnvVarValue): number {
-        // We use a score to enforce precedence:
-        //      value/value = 0
-        //      value/*     = 1
-        //      */value     = 2
-        //      */*         = 3
-        //      #/#         = 4 (used for env vars passed through the URL)
-        // the lower the score, the higher the precedence.
-        const [ownerPattern, repoPattern] = splitRepositoryPattern(value.repositoryPattern);
-        let score = 0;
-        if (repoPattern == "*") {
-            score += 1;
-        }
-        if (ownerPattern == "*") {
-            score += 2;
-        }
-        if (ownerPattern == "#" || repoPattern == "#") {
-            score = 4;
-        }
-        return score;
+    function splitRepositoryPattern(pattern: string): string[] {
+        return pattern.split(DELIMITER);
     }
 
-    // DEPRECATED: Use ProjectEnvVar instead of repositoryPattern - https://github.com/gitpod-com/gitpod/issues/5322
-    export function filter<T extends UserEnvVarValue>(vars: T[], owner: string, repo: string): T[] {
-        let result = vars.filter((e) => {
-            const [ownerPattern, repoPattern] = splitRepositoryPattern(e.repositoryPattern);
-            if (ownerPattern !== "*" && ownerPattern !== "#" && !!owner && ownerPattern !== owner.toLocaleLowerCase()) {
+    function joinRepositoryPattern(...parts: string[]): string {
+        return parts.join(DELIMITER);
+    }
+
+    /**
+     * Matches the given EnvVar pattern against the fully qualified name of the repository:
+     *  - GitHub: "repo/owner"
+     *  - GitLab: "some/nested/repo" (up to MAX_PATTERN_SEGMENTS deep)
+     * @param pattern
+     * @param repoFqn
+     * @returns True if the pattern matches that fully qualified name
+     */
+    export function matchEnvVarPattern(pattern: string, repoFqn: string): boolean {
+        const fqnSegments = splitRepositoryPattern(repoFqn);
+        const patternSegments = splitRepositoryPattern(pattern);
+        if (patternSegments.length < MIN_PATTERN_SEGMENTS) {
+            // Technically not a problem, but we should not allow this for safety reasons.
+            // And because we hisotrically never allowed this (GitHub would always require at least "*/*") we are just keeping old constraints.
+            // Note: Most importantly this guards against arbitrary wildcard matches
+            return false;
+        }
+
+        let i = 0;
+        for (; i < patternSegments.length; i++) {
+            if (i >= fqnSegments.length) {
                 return false;
             }
-            if (repoPattern !== "*" && repoPattern !== "#" && !!repo && repoPattern !== repo.toLocaleLowerCase()) {
+            const fqnSegment = fqnSegments[i];
+            const patternSegment = patternSegments[i];
+            if (!isWildcard(patternSegment) && patternSegment !== fqnSegment.toLocaleLowerCase()) {
                 return false;
             }
-            return true;
-        });
-
+        }
+        if (fqnSegments.length > i + 1) {
+            // Special case: "*" also matches arbitrary # of segments to the right
+            if (isWildcard(patternSegments[i])) {
+                return true;
+            }
+            return false;
+        }
+        return true;
+    }
+
+    // Matches the following patterns for "some/nested/repo", ordered by highest score:
+    //  - exact: some/nested/repo ()
+    //  - partial:
+    //    - some/nested/*
+    //    - some/*
+    //  - generic:
+    //    - */*/*
+    //    - */*
+    // Does NOT match:
+    //  - */repo (number of parts does not match)
+    // cmp. test cases in env-var-service.spec.ts
+    export function filter<T extends UserEnvVarValue>(vars: T[], owner: string, repo: string): T[] {
+        const matchedEnvVars = vars.filter((e) =>
+            matchEnvVarPattern(e.repositoryPattern, joinRepositoryPattern(owner, repo)),
+        );
         const resmap = new Map<string, T[]>();
-        result.forEach((e) => {
+        matchedEnvVars.forEach((e) => {
             const l = resmap.get(e.name) || [];
             l.push(e);
             resmap.set(e.name, l);
         });
 
-        result = [];
+        // In cases of multiple matches per env var: find the best match
+        // Best match is the most specific pattern, where the left-most segment is most significant
+        function scoreMatchedEnvVar(e: T): number {
+            const patternSegments = splitRepositoryPattern(e.repositoryPattern);
+            let result = 0;
+            for (const segment of patternSegments) {
+                // We can assume the pattern matches from context, so we just need to check whether it's a wildcard or not
+                const val = isWildcard(segment) ? 1 : 2;
+                result = result * 10 + val;
+            }
+            return result;
+        }
+        const result = [];
         for (const name of resmap.keys()) {
             const candidates = resmap.get(name);
             if (!candidates) {
@@ -527,12 +573,12 @@ export namespace UserEnvVar {
                 continue;
             }
 
-            let minscore = 10;
-            let bestCandidate: T | undefined;
-            for (const e of candidates) {
-                const score = UserEnvVar.score(e);
-                if (!bestCandidate || score < minscore) {
-                    minscore = score;
+            let bestCandidate = candidates[0];
+            let bestScore = scoreMatchedEnvVar(bestCandidate);
+            for (const e of candidates.slice(1)) {
+                const score = scoreMatchedEnvVar(e);
+                if (score > bestScore) {
+                    bestScore = score;
                     bestCandidate = e;
                 }
             }
@@ -541,14 +587,6 @@ export namespace UserEnvVar {
 
         return result;
     }
-
-    // DEPRECATED: Use ProjectEnvVar instead of repositoryPattern - https://github.com/gitpod-com/gitpod/issues/5322
-    export function splitRepositoryPattern(repositoryPattern: string): string[] {
-        const patterns = repositoryPattern.split("/");
-        const repoPattern = patterns.slice(1).join("/");
-        const ownerPattern = patterns[0];
-        return [ownerPattern, repoPattern];
-    }
 }
 
 export interface SSHPublicKeyValue {

components/server/src/auth/resource-access.ts

@@ -314,15 +314,22 @@ export class ScopedResourceGuard<K extends GuardedResourceKind = GuardedResource
 }
 
 export class WorkspaceEnvVarAccessGuard extends ScopedResourceGuard<"envVar"> {
-    private readAccessWildcardPatterns: Set<string> | undefined;
+    private _envVarScopes: string[];
+    protected get envVarScopes() {
+        return (this._envVarScopes = this._envVarScopes || []);
+    }
 
     async canAccess(resource: GuardedResource, operation: ResourceAccessOp): Promise<boolean> {
         if (resource.kind !== "envVar") {
             return false;
         }
         // allow read access based on wildcard repo patterns matching
-        if (operation === "get" && this.readAccessWildcardPatterns?.has(resource.subject.repositoryPattern)) {
-            return true;
+        if (operation === "get") {
+            for (const scope of this.envVarScopes) {
+                if (UserEnvVar.matchEnvVarPattern(resource.subject.repositoryPattern, scope)) {
+                    return true;
+                }
+            }
         }
         // but mutations only based on exact matching
         return super.canAccess(resource, operation);
@@ -333,11 +340,7 @@ export class WorkspaceEnvVarAccessGuard extends ScopedResourceGuard<"envVar"> {
         if (!scope.operations.includes("get")) {
             return;
         }
-        const [owner, repo] = UserEnvVar.splitRepositoryPattern(scope.subjectID);
-        this.readAccessWildcardPatterns = this.readAccessWildcardPatterns || new Set<string>();
-        this.readAccessWildcardPatterns.add("*/*");
-        this.readAccessWildcardPatterns.add(`${owner}/*`);
-        this.readAccessWildcardPatterns.add(`*/${repo}`);
+        this.envVarScopes.push(scope.subjectID); // the repository that this scope allows access to
     }
 }
 

components/server/src/workspace/env-var-service.spec.ts

@@ -286,6 +286,83 @@ class TestEnvVarService {
             });
         }
     }
+
+    @test
+    public async testRegularGitlabSubgroupWithUserEnvVars() {
+        const gitlabSubgroupCommitContext = {
+            repository: {
+                owner: "geropl-test-group/subgroup1",
+                name: "test-project-1",
+            },
+            revision: "abcd123",
+            title: "geropl-test-group/subgroup1/test-project-1",
+        } as CommitContext;
+
+        const userEnvVars: UserEnvVar[] = [
+            {
+                id: "1",
+                name: "USER_GLOBAL_TEST",
+                value: "true",
+                repositoryPattern: "*/*",
+                userId: "1",
+            },
+            {
+                id: "2",
+                name: "USER_GROUP_TEST",
+                value: "true",
+                repositoryPattern: "geropl-test-group/*",
+                userId: "1",
+            },
+            {
+                id: "3",
+                name: "USER_SUBGROUP_TEST",
+                value: "true",
+                repositoryPattern: "geropl-test-group/subgroup1/*",
+                userId: "1",
+            },
+            {
+                id: "4",
+                name: "USER_SUBGROUP_PROJECT_NEGATIVE_TEST",
+                value: "false",
+                repositoryPattern: "*/test-project-1",
+                userId: "1",
+            },
+            {
+                id: "5",
+                name: "USER_SUBGROUP_STAR_TEST",
+                value: "true",
+                repositoryPattern: "geropl-test-group/*/*",
+                userId: "1",
+            },
+            {
+                id: "6",
+                name: "USER_SUBGROUP_NEGATIVE_TEST",
+                value: "false",
+                repositoryPattern: "geropl-test-group/subgroup2/*",
+                userId: "1",
+            },
+            {
+                id: "7",
+                name: "USER_SUBGROUP_PROJECT_TEST",
+                value: "true",
+                repositoryPattern: "*/subgroup1/test-project-1",
+                userId: "1",
+            },
+        ];
+
+        this.init(userEnvVars, []);
+
+        const envVars = await this.envVarService.resolve({
+            owerId: "1",
+            type: "regular",
+            context: { ...gitlabSubgroupCommitContext },
+            projectId: "1",
+        } as any);
+        expect(envVars).to.deep.equal({
+            project: [],
+            workspace: userEnvVars.filter((ev) => ev.value === "true"),
+        });
+    }
 }
 
 module.exports = new TestEnvVarService();