diff --git a/.gitignore b/.gitignore index 15bedd29..f653d572 100644 --- a/.gitignore +++ b/.gitignore @@ -11,4 +11,5 @@ tsconfig.lsif.json *.vsix .env .DS_Store -secret.txt \ No newline at end of file +secret.txt +.vscode-test-web diff --git a/CHANGELOG.md b/CHANGELOG.md index c5c4922b..6766f07b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,3 +7,20 @@ Check [Keep a Changelog](http://keepachangelog.com/) for recommendations on how ## [Unreleased] - Initial release + +### Security + +- Gradle prewarm's execution of the cloned repository's `./gradlew` wrapper is now configurable + via the `GRADLE_PREWARM_AUTO_CONFIRM` environment variable. It defaults to `true` (prewarm runs + automatically, so the long first build is still skipped where exercise templates are provisioned + by a trusted platform). Set it to `false` to require the student to explicitly confirm before any + repository-controlled Gradle build is executed, removing the silent remote-code-execution path + for deployments that do not trust templates. `GRADLE_PREWARM=off` still disables prewarm entirely. +- The VCS/Git access token is no longer embedded in the clone URL or passed on the git command + line. It is injected through the git process environment (`GIT_CONFIG_*`) during clone and + persisted only into `.git/config` for later in-IDE fetch/push. +- STOMP/WebSocket frame logging is disabled so the Artemis JWT in the CONNECT frame is never + written to the debug console. +- The `ARTEMIS_URL` environment override is validated against the configured Artemis origin + (scheme + host + port) before it is used as the API base, preventing the Bearer token from + being sent to an untrusted host. diff --git a/README_DEVELOPER.md b/README_DEVELOPER.md index b240e1ce..9694eb14 100644 --- a/README_DEVELOPER.md +++ b/README_DEVELOPER.md @@ -43,7 +43,7 @@ The webview can NOT communicate with the Artemis API directly due to CORS. There ### Environment variables -At activation the core loads the session environment via a strategy (see [src/theia/env-strategy.ts](src/theia/env-strategy.ts)): from the process environment (lazy sessions) or by polling the data-bridge extension's `dataBridge.getEnvState` command until it reports `injected` (eager / prewarmed sessions). A fixed set of known keys (`ARTEMIS_TOKEN`, `ARTEMIS_URL`, `GIT_URI`, `GIT_USER`, `GIT_MAIL`, `GIT_TOKEN`, `THEIA`, `GRADLE_PREWARM`) drives the bespoke authentication, clone, git-identity, and prewarm logic. Any **other** variable an external system provides is applied to the integrated terminal environment via `context.environmentVariableCollection`, so terminals and terminal-spawned tasks inherit it. Known keys are deliberately kept out of the terminal environment (this also keeps `ARTEMIS_TOKEN` out of shells). Note that `environmentVariableCollection` affects only terminals started after activation, not already-running backend or language-server processes. +At activation the core loads the session environment via a strategy (see [src/theia/env-strategy.ts](src/theia/env-strategy.ts)): from the process environment (lazy sessions) or by polling the data-bridge extension's `dataBridge.getEnvState` command until it reports `injected` (eager / prewarmed sessions). A fixed set of known keys (`ARTEMIS_TOKEN`, `ARTEMIS_URL`, `GIT_URI`, `GIT_USER`, `GIT_MAIL`, `GIT_TOKEN`, `THEIA`, `GRADLE_PREWARM`, `GRADLE_PREWARM_AUTO_CONFIRM`) drives the bespoke authentication, clone, git-identity, and prewarm logic. `GRADLE_PREWARM` selects the prewarm depth (`off`/`daemon`/`deps`/`full`) and `GRADLE_PREWARM_AUTO_CONFIRM` (default `true`) decides whether the prewarm may run automatically or must be confirmed by the student first. Any **other** variable an external system provides is applied to the integrated terminal environment via `context.environmentVariableCollection`, so terminals and terminal-spawned tasks inherit it. Known keys are deliberately kept out of the terminal environment (this also keeps `ARTEMIS_TOKEN` out of shells). Note that `environmentVariableCollection` affects only terminals started after activation, not already-running backend or language-server processes. ### Webview diff --git a/shared/models/participation.model.ts b/shared/models/participation.model.ts index fe4275ad..405a2dd9 100644 --- a/shared/models/participation.model.ts +++ b/shared/models/participation.model.ts @@ -60,14 +60,3 @@ export function getProjectKeyFromRepoUrl(repoUrl: string): string { const projectKey = parts[4]; return projectKey; } - -export function addVcsTokenToUrl(url: string, username: string, vsctoken: string): string { - const credentials = `://${username}:${vsctoken}@`; - if (!url.includes("@")) { - // the url has the format https://vcs-server.com - return url.replace("://", credentials); - } else { - // the url has the format https://username@vcs-server.com -> replace ://username@ - return url.replace(/:\/\/.*@/, credentials); - } -} diff --git a/src/participation/cloning.service.ts b/src/participation/cloning.service.ts index 955433a7..ff73b569 100644 --- a/src/participation/cloning.service.ts +++ b/src/participation/cloning.service.ts @@ -3,13 +3,12 @@ import { settings } from "../shared/settings"; import { NotAuthenticatedError } from "../authentication/not_authenticated.error"; import { AUTH_ID } from "../authentication/authentication_provider"; import { getState } from "../shared/state"; -import simpleGit from "simple-git"; +import simpleGit, { GitConfigScope } from "simple-git"; import { tmpdir } from "os"; import * as path from "path"; import * as fs from "fs/promises"; import { retrieveVcsAccessToken } from "../artemis/authentication.client"; import { getWorkspaceFolder, theiaEnv } from "../theia/theia"; -import { addVcsTokenToUrl } from "@shared/models/participation.model"; import { ProgrammingExercise, ProgrammingLanguage } from "@shared/models/exercise.model"; import { warmupGradleDaemon } from "./gradle.service"; @@ -19,8 +18,10 @@ type CloneOptions = { mode?: CloneMode; preservedPaths?: string[]; // Full HTTP header line (e.g. "Authorization: Bearer ") to authenticate the clone. - // When set, it is passed as `git -c http.extraHeader=...` and persisted into the cloned - // repo's .git/config so subsequent in-IDE fetch/push stay authenticated. + // When set, it is injected into the git process via the GIT_CONFIG_* environment variables + // (never on the command line, so the token does not appear in process arguments) and then + // persisted into the cloned repo's .git/config so subsequent in-IDE fetch/push stay + // authenticated. httpExtraHeader?: string; }; @@ -79,15 +80,26 @@ export async function cloneUserRepo(repoUrl: string, username: string) { session.accessToken, getState().displayedExercise?.studentParticipations![0].id!, ); - // Clone the repository - const cloneUrlWithToken = new URL(addVcsTokenToUrl(repoUrl, username, vcsToken)); - const clonePath = await cloneByGivenURL(cloneUrlWithToken, destinationPath, cloneOptions); + // Authenticate the clone with an HTTP Basic header instead of embedding `user:token@host` + // in the URL. This keeps the short-lived VCS token out of the clone URL, out of the stored + // remote in .git/config and out of the git command line (process arguments). + const basicAuth = Buffer.from(`${username}:${vcsToken}`).toString("base64"); + const clonePath = await cloneByGivenURL(new URL(repoUrl), destinationPath, { + ...cloneOptions, + httpExtraHeader: `Authorization: Basic ${basicAuth}`, + }); // Pre-warm Gradle in the background so the student's first build is faster. // The depth (off/daemon/deps/full) is controlled by the GRADLE_PREWARM env var. + // By default this runs automatically; set GRADLE_PREWARM_AUTO_CONFIRM=false to + // require the student to confirm first (see warmupGradleDaemon). const exercise = getState().displayedExercise; if ((exercise as ProgrammingExercise)?.programmingLanguage === ProgrammingLanguage.JAVA) { - warmupGradleDaemon(clonePath, theiaEnv.GRADLE_PREWARM); + void warmupGradleDaemon( + clonePath, + theiaEnv.GRADLE_PREWARM, + theiaEnv.GRADLE_PREWARM_AUTO_CONFIRM, + ); } if (!theiaEnv.THEIA_FLAG) { @@ -134,25 +146,46 @@ export async function cloneByGivenURL( const clonePath = path.join(destinationPath, repoName); const gitForClone = gitClientFactory.simpleGit(destinationPath); + applyAuthHeaderEnv(gitForClone, options?.httpExtraHeader); try { - await gitForClone.clone(cloneUrl.toString(), clonePath, buildCloneOptions(options?.httpExtraHeader)); + await gitForClone.clone(cloneUrl.toString(), clonePath); } catch (e: any) { throw new Error(`Error cloning repository: ${e.message}`); } + await persistAuthHeader(clonePath, options?.httpExtraHeader); + return clonePath; } -// Builds the extra simple-git clone options. When an HTTP header is provided, it is passed as -// `git clone -c http.extraHeader=...`, which authenticates the initial fetch AND persists the -// header into the new repo's .git/config so later in-IDE fetch/push are authenticated too. -// The token is therefore stored in .git/config; this is acceptable since it is short-lived. -function buildCloneOptions(httpExtraHeader?: string): string[] { +// Injects the auth header into the git process via the GIT_CONFIG_* environment variables +// (git >= 2.31). This authenticates the clone WITHOUT placing the token on the command line, +// so it never appears in process arguments (`ps`). The env-provided config is not written to +// the new repo's .git/config, so persistAuthHeader stores it afterwards for later fetch/push. +function applyAuthHeaderEnv(git: ReturnType, httpExtraHeader?: string): void { if (!httpExtraHeader) { - return []; + return; } - return ["-c", `http.extraHeader=${httpExtraHeader}`]; + // `.env` replaces the child process environment, so carry over the current environment + // (PATH etc.) and add the single-entry git config override. + git.env({ + ...process.env, + GIT_CONFIG_COUNT: "1", + GIT_CONFIG_KEY_0: "http.extraHeader", + GIT_CONFIG_VALUE_0: httpExtraHeader, + }); +} + +// Persists the auth header into the cloned repo so later in-IDE fetch/push stay authenticated. +// Residual risk: the short-lived token is written to .git/config here. It is, however, no longer +// embedded in the clone URL / stored remote nor exposed in process arguments. +async function persistAuthHeader(repoPath: string, httpExtraHeader?: string): Promise { + if (!httpExtraHeader) { + return; + } + const git = gitClientFactory.simpleGit(repoPath); + await git.addConfig("http.extraHeader", httpExtraHeader, false, GitConfigScope.local); } async function cloneIntoWorkspaceRoot( @@ -179,10 +212,13 @@ async function cloneIntoWorkspaceRoot( await clearDirectory(workspacePath); const gitForClone = gitClientFactory.simpleGit(workspacePath); + applyAuthHeaderEnv(gitForClone, httpExtraHeader); // Clone into "." so the exercise repository becomes the workspace root in Theia. - await gitForClone.clone(cloneUrl.toString(), ".", buildCloneOptions(httpExtraHeader)); + await gitForClone.clone(cloneUrl.toString(), "."); cloneSucceeded = true; + await persistAuthHeader(workspacePath, httpExtraHeader); + return workspacePath; } catch (e: any) { throw new Error(`Error cloning repository into workspace root: ${e.message}`); diff --git a/src/participation/gradle.service.ts b/src/participation/gradle.service.ts index d980801b..3f8ccb37 100644 --- a/src/participation/gradle.service.ts +++ b/src/participation/gradle.service.ts @@ -1,6 +1,7 @@ import { spawn } from "child_process"; import * as fs from "fs"; import * as path from "path"; +import * as vscode from "vscode"; import { GradlePrewarmLevel } from "../theia/env-strategy"; /** @@ -18,41 +19,92 @@ const PREWARM_ARGS: Record, string[]> = { }; /** - * Pre-warms the Gradle build in the background after cloning a repository, so the - * student's first build can skip the phases warmed here. Runs detached and never - * blocks the caller. + * Side effects the prewarm performs, grouped behind a seam so tests can assert the confirmation + * gate (when it is active, no build starts without the student's confirmation) without touching + * the real filesystem, child_process or VS Code. + */ +export type GradlePrewarmEffects = { + gradlewExists: (gradlewPath: string) => boolean; + confirm: (projectPath: string) => Promise; + makeExecutable: (gradlewPath: string) => void; + run: (projectPath: string, args: string[], level: GradlePrewarmLevel) => void; +}; + +/** + * Asks the student whether the repository's Gradle wrapper may be executed to prewarm the + * build. Used only when GRADLE_PREWARM_AUTO_CONFIRM is false. The prompt is non-modal and + * defaults to "no": if the student ignores it nothing runs, removing the silent code-execution + * property for deployments that do not trust exercise templates. + */ +async function promptPrewarmConfirmation(_projectPath: string): Promise { + const choice = await vscode.window.showWarningMessage( + "Scorpio can pre-build this exercise with its Gradle wrapper to speed up your first build. " + + "This runs code from the cloned repository. Only continue for exercises you trust.", + "Pre-build now", + ); + return choice === "Pre-build now"; +} + +function spawnGradle(projectPath: string, args: string[], level: GradlePrewarmLevel): void { + const child = spawn("./gradlew", args, { + cwd: projectPath, + detached: true, + stdio: "ignore", + }); + // Detached failures surface asynchronously via the 'error' event, not a throw. + child.on("error", (error) => { + console.warn(`Gradle prewarm (${level}) failed to start: ${error.message}`); + }); + child.unref(); +} + +export const defaultGradlePrewarmEffects: GradlePrewarmEffects = { + gradlewExists: (gradlewPath) => fs.existsSync(gradlewPath), + confirm: promptPrewarmConfirmation, + makeExecutable: (gradlewPath) => fs.chmodSync(gradlewPath, 0o755), + run: spawnGradle, +}; + +/** + * Pre-warms the Gradle build in the background after cloning a repository, so the student's + * first build can skip the phases warmed here. * - * Silently skips if: + * Security: `./gradlew` and the bundled Gradle wrapper jar are repository-controlled, so a + * tampered exercise template could run arbitrary code during prewarm. The behavior is governed + * by `autoConfirm` (the GRADLE_PREWARM_AUTO_CONFIRM env var): + * - `true` (default): the prewarm runs automatically. Appropriate when exercise templates are + * provisioned by a trusted platform, where the long first build matters more than the risk. + * - `false`: the student must explicitly confirm before the wrapper runs, and confirmation + * defaults to "no". The safe choice when templates are not trusted. + * + * Silently skips (no prompt, no build) if: * - Prewarming is disabled (`level` is "off") * - Running on Windows (not a supported environment) * - The project does not contain a `gradlew` file (not a Gradle project) */ -export function warmupGradleDaemon( +export async function warmupGradleDaemon( projectPath: string, level: GradlePrewarmLevel = "daemon", -): void { + autoConfirm = true, + effects: GradlePrewarmEffects = defaultGradlePrewarmEffects, +): Promise { if (level === "off" || process.platform === "win32") { return; } const gradlewPath = path.join(projectPath, "gradlew"); - if (!fs.existsSync(gradlewPath)) { + if (!effects.gradlewExists(gradlewPath)) { + return; + } + + const confirmed = autoConfirm ? true : await effects.confirm(projectPath).catch(() => false); + if (!confirmed) { return; } try { - fs.chmodSync(gradlewPath, 0o755); - - const child = spawn("./gradlew", PREWARM_ARGS[level], { - cwd: projectPath, - detached: true, - stdio: "ignore", - }); - // Detached failures surface asynchronously via the 'error' event, not the throw below. - child.on("error", (error) => { - console.warn(`Gradle prewarm (${level}) failed to start: ${error.message}`); - }); - child.unref(); + effects.makeExecutable(gradlewPath); + effects.run(projectPath, PREWARM_ARGS[level], level); } catch (error: any) { console.warn(`Gradle prewarm (${level}) failed: ${error.message}`); } diff --git a/src/shared/artemis-url.ts b/src/shared/artemis-url.ts new file mode 100644 index 00000000..ea666bd4 --- /dev/null +++ b/src/shared/artemis-url.ts @@ -0,0 +1,45 @@ +/** + * Decides whether an externally supplied Artemis base URL (for example the `ARTEMIS_URL` + * environment variable injected into a Theia session) may be trusted as the API base. + * + * The candidate is only accepted when it is a well-formed http(s) URL whose origin + * (scheme + host + port) matches the origin the extension is already configured for. This + * prevents an attacker-controlled environment from silently repointing the extension - and the + * Bearer token it attaches to every request - at a rogue host. + * + * Returns the trusted URL string when the candidate is allowed, otherwise `undefined`. + */ +export function resolveAllowedArtemisUrl( + candidate: string | undefined, + configuredBaseUrl: string | undefined, +): string | undefined { + if (!candidate) { + return undefined; + } + + let candidateUrl: URL; + try { + candidateUrl = new URL(candidate); + } catch { + return undefined; + } + + if (candidateUrl.protocol !== "http:" && candidateUrl.protocol !== "https:") { + return undefined; + } + + // Without a configured origin to allowlist against we cannot establish trust, so the + // override is rejected rather than accepted blindly. + if (!configuredBaseUrl) { + return undefined; + } + + let configuredUrl: URL; + try { + configuredUrl = new URL(configuredBaseUrl); + } catch { + return undefined; + } + + return candidateUrl.origin === configuredUrl.origin ? candidate : undefined; +} diff --git a/src/shared/settings.ts b/src/shared/settings.ts index b7407967..3d6028cc 100644 --- a/src/shared/settings.ts +++ b/src/shared/settings.ts @@ -1,6 +1,7 @@ import * as vscode from "vscode"; import { authenticationProvider } from "../extension"; import { theiaEnv } from "../theia/theia"; +import { resolveAllowedArtemisUrl } from "./artemis-url"; export type Settings = { base_url: string | undefined; @@ -21,9 +22,19 @@ export function initSettings() { function getSettings(): Settings { let base_url = vscode.workspace.getConfiguration("scorpio").get("artemis.apiBaseUrl"); if (theiaEnv.ARTEMIS_URL) { - const config = vscode.workspace.getConfiguration("scorpio"); - config.update("artemis.apiBaseUrl", theiaEnv.ARTEMIS_URL, vscode.ConfigurationTarget.Global); - base_url = theiaEnv.ARTEMIS_URL; + // Only honor the environment override when it points at the same origin the extension is + // already configured for. An unvalidated ARTEMIS_URL could otherwise redirect every + // authenticated request (and the Bearer token) to an attacker-controlled host. + const trustedUrl = resolveAllowedArtemisUrl(theiaEnv.ARTEMIS_URL, base_url); + if (trustedUrl) { + const config = vscode.workspace.getConfiguration("scorpio"); + config.update("artemis.apiBaseUrl", trustedUrl, vscode.ConfigurationTarget.Global); + base_url = trustedUrl; + } else { + vscode.window.showErrorMessage( + `Ignoring ARTEMIS_URL "${theiaEnv.ARTEMIS_URL}": it does not match the configured Artemis origin${base_url ? ` (${base_url})` : ""}.`, + ); + } } if (!base_url) { diff --git a/src/shared/websocket.ts b/src/shared/websocket.ts index 45237eef..7db2d3e5 100644 --- a/src/shared/websocket.ts +++ b/src/shared/websocket.ts @@ -108,9 +108,10 @@ export class GenericWebSocket { connectHeaders: { Authorization: `Bearer ${session.accessToken}`, }, - debug: function (str: string) { - console.debug(str); - }, + // Security: never forward STOMP frames to the log. The CONNECT frame carries the + // Authorization header (the Artemis JWT), so logging frames leaks the token. Keep this + // a no-op so the access token is never written to the debug console. + debug: () => {}, reconnectDelay: 5000, heartbeatIncoming: 10000, heartbeatOutgoing: 10000, diff --git a/src/test/suite/artemis-url.test.ts b/src/test/suite/artemis-url.test.ts new file mode 100644 index 00000000..dfafab5c --- /dev/null +++ b/src/test/suite/artemis-url.test.ts @@ -0,0 +1,44 @@ +import * as assert from "assert"; + +import { resolveAllowedArtemisUrl } from "../../shared/artemis-url"; + +suite("resolveAllowedArtemisUrl", () => { + const configured = "https://artemis.cit.tum.de"; + + test("accepts a candidate with the same origin as the configured base URL", () => { + assert.strictEqual( + resolveAllowedArtemisUrl("https://artemis.cit.tum.de", configured), + "https://artemis.cit.tum.de", + ); + assert.strictEqual( + resolveAllowedArtemisUrl("https://artemis.cit.tum.de/api", configured), + "https://artemis.cit.tum.de/api", + ); + }); + + test("rejects a different host", () => { + assert.strictEqual(resolveAllowedArtemisUrl("https://evil.example.com", configured), undefined); + }); + + test("rejects a scheme downgrade and a different port", () => { + assert.strictEqual(resolveAllowedArtemisUrl("http://artemis.cit.tum.de", configured), undefined); + assert.strictEqual( + resolveAllowedArtemisUrl("https://artemis.cit.tum.de:8443", configured), + undefined, + ); + }); + + test("rejects non-http(s) and malformed candidates", () => { + assert.strictEqual( + resolveAllowedArtemisUrl("javascript:alert(1)", configured), + undefined, + ); + assert.strictEqual(resolveAllowedArtemisUrl("not a url", configured), undefined); + assert.strictEqual(resolveAllowedArtemisUrl("", configured), undefined); + assert.strictEqual(resolveAllowedArtemisUrl(undefined, configured), undefined); + }); + + test("rejects any candidate when no configured origin is available", () => { + assert.strictEqual(resolveAllowedArtemisUrl("https://artemis.cit.tum.de", undefined), undefined); + }); +}); diff --git a/src/test/suite/cloning.test.ts b/src/test/suite/cloning.test.ts index 7a6443e8..887a92d1 100644 --- a/src/test/suite/cloning.test.ts +++ b/src/test/suite/cloning.test.ts @@ -3,21 +3,35 @@ import * as assert from "assert"; import { cloneByGivenURL, gitClientFactory } from "../../participation/cloning.service"; type CloneCall = { repo: string; local: string; options: unknown }; +type ConfigCall = { key: string; value: string; append: unknown; scope: unknown }; suite("cloneByGivenURL httpExtraHeader handling", () => { const originalSimpleGit = gitClientFactory.simpleGit; let cloneCalls: CloneCall[]; + let configCalls: ConfigCall[]; + let envCalls: Record[]; setup(() => { cloneCalls = []; - // Swap the git factory for a fake that records the arguments passed to clone(). + configCalls = []; + envCalls = []; + // Swap the git factory for a fake that records env(), clone() and addConfig() arguments. gitClientFactory.simpleGit = (() => { - return { + const fake: any = { + env: (env: Record) => { + envCalls.push(env); + return fake; + }, clone: async (repo: string, local: string, options: unknown) => { cloneCalls.push({ repo, local, options }); return ""; }, + addConfig: async (key: string, value: string, append: unknown, scope: unknown) => { + configCalls.push({ key, value, append, scope }); + return ""; + }, }; + return fake; }) as unknown as typeof gitClientFactory.simpleGit; }); @@ -25,24 +39,43 @@ suite("cloneByGivenURL httpExtraHeader handling", () => { gitClientFactory.simpleGit = originalSimpleGit; }); - test("passes -c http.extraHeader when a header is provided", async () => { - await cloneByGivenURL( - new URL("https://gitea.example.com/course/repo.git"), - "/dest", - { httpExtraHeader: "Authorization: Bearer TESTTOKEN" }, - ); + test("injects the auth header via GIT_CONFIG_* env, never on the command line", async () => { + await cloneByGivenURL(new URL("https://gitea.example.com/course/repo.git"), "/dest", { + httpExtraHeader: "Authorization: Bearer TESTTOKEN", + }); assert.strictEqual(cloneCalls.length, 1); - assert.deepStrictEqual(cloneCalls[0].options, [ - "-c", - "http.extraHeader=Authorization: Bearer TESTTOKEN", - ]); + // The token must not be passed as a clone argument (would be visible in `ps`). + assert.deepStrictEqual(cloneCalls[0].options, undefined); + assert.ok( + !JSON.stringify(cloneCalls[0]).includes("TESTTOKEN"), + "clone arguments must not contain the token", + ); + + // The header is supplied through the git process environment instead. + assert.strictEqual(envCalls.length, 1); + assert.strictEqual(envCalls[0].GIT_CONFIG_COUNT, "1"); + assert.strictEqual(envCalls[0].GIT_CONFIG_KEY_0, "http.extraHeader"); + assert.strictEqual(envCalls[0].GIT_CONFIG_VALUE_0, "Authorization: Bearer TESTTOKEN"); + }); + + test("persists the auth header into the cloned repo for later fetch/push", async () => { + await cloneByGivenURL(new URL("https://gitea.example.com/course/repo.git"), "/dest", { + httpExtraHeader: "Authorization: Bearer TESTTOKEN", + }); + + assert.strictEqual(configCalls.length, 1); + assert.strictEqual(configCalls[0].key, "http.extraHeader"); + assert.strictEqual(configCalls[0].value, "Authorization: Bearer TESTTOKEN"); + assert.strictEqual(configCalls[0].scope, "local"); }); - test("passes no extra config when no header is provided (unchanged path)", async () => { + test("does not touch env or config when no header is provided (unchanged path)", async () => { await cloneByGivenURL(new URL("https://gitea.example.com/course/repo.git"), "/dest"); assert.strictEqual(cloneCalls.length, 1); - assert.deepStrictEqual(cloneCalls[0].options, []); + assert.deepStrictEqual(cloneCalls[0].options, undefined); + assert.strictEqual(envCalls.length, 0); + assert.strictEqual(configCalls.length, 0); }); }); diff --git a/src/test/suite/env-strategy.test.ts b/src/test/suite/env-strategy.test.ts index bc38d67c..ebc19ad3 100644 --- a/src/test/suite/env-strategy.test.ts +++ b/src/test/suite/env-strategy.test.ts @@ -1,6 +1,10 @@ import * as assert from "assert"; -import { KNOWN_ENV_KEYS, parseTheiaEnv } from "../../theia/env-strategy"; +import { + DEFAULT_GRADLE_PREWARM_AUTO_CONFIRM, + KNOWN_ENV_KEYS, + parseTheiaEnv, +} from "../../theia/env-strategy"; suite("env-strategy GIT_TOKEN handling", () => { test("parseTheiaEnv returns GIT_TOKEN when present", () => { @@ -45,3 +49,41 @@ suite("env-strategy GIT_TOKEN handling", () => { assert.strictEqual(parsed.GRADLE_PREWARM, "daemon"); }); }); + +suite("env-strategy GRADLE_PREWARM_AUTO_CONFIRM handling", () => { + function parse(value: string | undefined) { + return parseTheiaEnv({ + THEIA: "1", + GRADLE_PREWARM: "daemon", + GRADLE_PREWARM_AUTO_CONFIRM: value, + }).GRADLE_PREWARM_AUTO_CONFIRM; + } + + test("defaults to automatic prewarm (true) when unset", () => { + assert.strictEqual(DEFAULT_GRADLE_PREWARM_AUTO_CONFIRM, true); + assert.strictEqual(parse(undefined), true); + }); + + test("is queried by the env strategy", () => { + assert.ok( + KNOWN_ENV_KEYS.has("GRADLE_PREWARM_AUTO_CONFIRM"), + "GRADLE_PREWARM_AUTO_CONFIRM must be among the queried keys", + ); + }); + + test("parses falsy values as false (require confirmation)", () => { + for (const value of ["false", "0", "no", "off", "FALSE", " Off "]) { + assert.strictEqual(parse(value), false, `"${value}" should disable auto-confirm`); + } + }); + + test("parses truthy values as true", () => { + for (const value of ["true", "1", "yes", "on", "TRUE"]) { + assert.strictEqual(parse(value), true, `"${value}" should enable auto-confirm`); + } + }); + + test("falls back to the default on an unrecognized value", () => { + assert.strictEqual(parse("maybe"), DEFAULT_GRADLE_PREWARM_AUTO_CONFIRM); + }); +}); diff --git a/src/test/suite/gradle.test.ts b/src/test/suite/gradle.test.ts new file mode 100644 index 00000000..d03547aa --- /dev/null +++ b/src/test/suite/gradle.test.ts @@ -0,0 +1,93 @@ +import * as assert from "assert"; + +import { GradlePrewarmEffects, warmupGradleDaemon } from "../../participation/gradle.service"; + +type RunCall = { projectPath: string; args: string[]; level: string }; + +function makeEffects(overrides: Partial) { + const runCalls: RunCall[] = []; + const effects: GradlePrewarmEffects = { + gradlewExists: () => true, + confirm: async () => true, + makeExecutable: () => {}, + run: (projectPath, args, level) => { + runCalls.push({ projectPath, args, level }); + }, + ...overrides, + }; + return { effects, runCalls }; +} + +suite("warmupGradleDaemon confirmation gate (GRADLE_PREWARM_AUTO_CONFIRM=false)", () => { + test("does not run the Gradle wrapper when the user declines", async () => { + const { effects, runCalls } = makeEffects({ confirm: async () => false }); + + await warmupGradleDaemon("/repo", "daemon", false, effects); + + assert.strictEqual(runCalls.length, 0, "no build may start without confirmation"); + }); + + test("runs the Gradle wrapper only after explicit confirmation", async () => { + const { effects, runCalls } = makeEffects({ confirm: async () => true }); + + await warmupGradleDaemon("/repo", "daemon", false, effects); + + assert.strictEqual(runCalls.length, 1); + assert.deepStrictEqual(runCalls[0].args, ["--daemon", "help"]); + }); + + test("treats a confirmation failure as a decline", async () => { + const { effects, runCalls } = makeEffects({ + confirm: async () => { + throw new Error("prompt failed"); + }, + }); + + await warmupGradleDaemon("/repo", "deps", false, effects); + + assert.strictEqual(runCalls.length, 0); + }); +}); + +suite("warmupGradleDaemon auto-confirm (GRADLE_PREWARM_AUTO_CONFIRM=true, default)", () => { + test("runs the Gradle wrapper without prompting", async () => { + let prompted = false; + const { effects, runCalls } = makeEffects({ + confirm: async () => { + prompted = true; + return true; + }, + }); + + await warmupGradleDaemon("/repo", "full", true, effects); + + assert.strictEqual(prompted, false, "auto-confirm must not prompt the student"); + assert.strictEqual(runCalls.length, 1); + assert.deepStrictEqual(runCalls[0].args, ["--daemon", "build", "-x", "test"]); + }); +}); + +suite("warmupGradleDaemon skip conditions", () => { + test("never prompts or runs when prewarming is disabled", async () => { + let prompted = false; + const { effects, runCalls } = makeEffects({ + confirm: async () => { + prompted = true; + return true; + }, + }); + + await warmupGradleDaemon("/repo", "off", false, effects); + + assert.strictEqual(prompted, false, "disabled prewarm must not prompt"); + assert.strictEqual(runCalls.length, 0); + }); + + test("does not run when the project has no gradlew, even with auto-confirm", async () => { + const { effects, runCalls } = makeEffects({ gradlewExists: () => false }); + + await warmupGradleDaemon("/repo", "full", true, effects); + + assert.strictEqual(runCalls.length, 0, "non-Gradle project must not build"); + }); +}); diff --git a/src/theia/env-strategy.ts b/src/theia/env-strategy.ts index aae96b8f..6431236f 100644 --- a/src/theia/env-strategy.ts +++ b/src/theia/env-strategy.ts @@ -20,6 +20,7 @@ export type TheiaEnv = { GIT_MAIL: string | undefined; GIT_TOKEN: string | undefined; GRADLE_PREWARM: GradlePrewarmLevel; + GRADLE_PREWARM_AUTO_CONFIRM: boolean; }; const ENV_KEYS = [ @@ -31,6 +32,7 @@ const ENV_KEYS = [ "GIT_MAIL", "GIT_TOKEN", "GRADLE_PREWARM", + "GRADLE_PREWARM_AUTO_CONFIRM", ] as const satisfies Array; /** @@ -39,6 +41,17 @@ const ENV_KEYS = [ */ export const DEFAULT_GRADLE_PREWARM: GradlePrewarmLevel = "daemon"; +/** + * Whether the Gradle prewarm may run automatically after cloning, without asking + * the student first. Default true: exercise templates are provisioned by the + * platform and prewarming saves a long first build, so most deployments want it + * on. Set the GRADLE_PREWARM_AUTO_CONFIRM env var to false to require an explicit + * confirmation before any repository-controlled `gradlew` is executed (the safe + * choice when templates are not trusted). GRADLE_PREWARM="off" disables prewarm + * entirely regardless of this flag. + */ +export const DEFAULT_GRADLE_PREWARM_AUTO_CONFIRM = true; + function parseGradlePrewarm(value: string | undefined): GradlePrewarmLevel { const level = value?.trim().toLowerCase(); if (level === "off" || level === "daemon" || level === "deps" || level === "full") { @@ -52,6 +65,23 @@ function parseGradlePrewarm(value: string | undefined): GradlePrewarmLevel { return DEFAULT_GRADLE_PREWARM; } +function parseGradlePrewarmAutoConfirm(value: string | undefined): boolean { + const flag = value?.trim().toLowerCase(); + if (flag === undefined || flag === "") { + return DEFAULT_GRADLE_PREWARM_AUTO_CONFIRM; + } + if (flag === "true" || flag === "1" || flag === "yes" || flag === "on") { + return true; + } + if (flag === "false" || flag === "0" || flag === "no" || flag === "off") { + return false; + } + console.warn( + `Unknown GRADLE_PREWARM_AUTO_CONFIRM value "${value}", falling back to "${DEFAULT_GRADLE_PREWARM_AUTO_CONFIRM}"`, + ); + return DEFAULT_GRADLE_PREWARM_AUTO_CONFIRM; +} + /** * Result of loading the environment: the typed `env` (the known keys with their * bespoke handling) plus `raw`, the full untyped map of every variable that was @@ -81,6 +111,7 @@ export function parseTheiaEnv(env: Record): TheiaEnv GIT_MAIL: env["GIT_MAIL"], GIT_TOKEN: env["GIT_TOKEN"], GRADLE_PREWARM: parseGradlePrewarm(env["GRADLE_PREWARM"]), + GRADLE_PREWARM_AUTO_CONFIRM: parseGradlePrewarmAutoConfirm(env["GRADLE_PREWARM_AUTO_CONFIRM"]), }; } diff --git a/src/theia/theia.ts b/src/theia/theia.ts index 20880be6..2138785a 100644 --- a/src/theia/theia.ts +++ b/src/theia/theia.ts @@ -2,7 +2,12 @@ import * as vscode from "vscode"; import simpleGit, { GitConfigScope } from "simple-git"; import { hostname } from "os"; import { cloneByGivenURL } from "../participation/cloning.service"; -import { createTheiaEnvStrategy, DEFAULT_GRADLE_PREWARM, TheiaEnv } from "./env-strategy"; +import { + createTheiaEnvStrategy, + DEFAULT_GRADLE_PREWARM, + DEFAULT_GRADLE_PREWARM_AUTO_CONFIRM, + TheiaEnv, +} from "./env-strategy"; // Mutable theiaEnv that gets populated after loading export let theiaEnv: TheiaEnv = { @@ -14,6 +19,7 @@ export let theiaEnv: TheiaEnv = { GIT_MAIL: undefined, GIT_TOKEN: undefined, GRADLE_PREWARM: DEFAULT_GRADLE_PREWARM, + GRADLE_PREWARM_AUTO_CONFIRM: DEFAULT_GRADLE_PREWARM_AUTO_CONFIRM, }; // Full untyped map of every variable that was provided (including arbitrary keys @@ -55,9 +61,7 @@ export async function initTheia() { await cloneByGivenURL(theiaEnv.GIT_URI, workspaceFolderUri.fsPath, { mode: "workspace-root", - httpExtraHeader: useGiteaToken - ? `Authorization: Bearer ${theiaEnv.GIT_TOKEN}` - : undefined, + httpExtraHeader: useGiteaToken ? `Authorization: Bearer ${theiaEnv.GIT_TOKEN}` : undefined, }); }