Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ deployment. Treat them as untrusted:

- Run the CLI with `execFile`/`spawn` argv (or a terminal's `shellPath` and
`shellArgs`), never through a shell or `terminal.sendText`.
- Pass `env: getCliProcessEnv()` (plus `strictEnv: true` for a terminal) so
a host `CODER_SESSION_TOKEN` cannot replace the stored session.
- Put flags first, then `--`, then deployment-provided operands
(`coder ping -- owner/ws`) so a value like `--foo` is never read as a flag.
- Validate names with `src/common/validation.ts` before they reach SSH hosts,
Expand Down
21 changes: 18 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,24 @@

### Fixed

- OAuth sign-in now works on deployments that enforce token scopes. Users signed
in with OAuth are asked to sign in again once.
- Revoke OAuth tokens on logout even when their scopes are outdated.
- OAuth sign-in now works on deployments that enforce token scopes. Sessions
missing required scopes stop refreshing and require sign-in when they expire.
- Revoke OAuth tokens on logout and after successful session replacement, even
when their scopes are outdated. Failed or canceled sign-ins do not revoke
the stored session.
- Preserve OAuth refresh credentials when reusing a stored token, and request
the permissions needed to refresh workspace external-auth links.
- OAuth sign-in is offered only when the deployment allows dynamic client
registration, which is off by default from Coder 2.37; otherwise a notice
says sign-in uses a session token. OAuth becomes the default sign-in method
in a future release, so ask your Coder administrator to enable dynamic client
registration. For instructions, see
[OAuth2 provider](https://coder.com/docs/admin/integrations/oauth2-provider).
- Keep session tokens, OAuth tokens, and client secrets out of the Coder output
channel and its buffered logs, including from failed requests.
- CLI commands and SSH connections that the extension starts ignore
`CODER_SESSION_TOKEN` from the environment, so they no longer fail or run as a
different user inside a Coder workspace.

### Fixed

Expand Down
2 changes: 1 addition & 1 deletion src/api/authInterceptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export class AuthInterceptor implements vscode.Disposable {

// 1) OAuth refresh path.
const isOAuth =
await this.oauthSessionManager.isLoggedInWithOAuth(hostname);
await this.oauthSessionManager.canRefreshOAuthSession(hostname);
recorder.setRefreshAttempted(isOAuth);
if (isOAuth) {
const newToken = await this.tryOAuthRefresh();
Expand Down
8 changes: 6 additions & 2 deletions src/api/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ import { spawn } from "node:child_process";
import * as vscode from "vscode";

import { errToStr } from "../error/normalize";
import { getGlobalFlags, type CliAuth } from "../settings/cli";
import {
getCliProcessEnv,
getGlobalFlags,
type CliAuth,
} from "../settings/cli";

import { createWorkspaceIdentifier } from "./models";

Expand Down Expand Up @@ -81,7 +85,7 @@ function runCliCommand(ctx: CliContext, args: string[]): Promise<void> {
"--",
createWorkspaceIdentifier(ctx.workspace),
];
const proc = spawn(ctx.binPath, fullArgs);
const proc = spawn(ctx.binPath, fullArgs, { env: getCliProcessEnv() });
// Unexpected prompts EOF instead of hanging forever.
proc.stdin.end();

Expand Down
4 changes: 3 additions & 1 deletion src/cli/cliCredentialManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
import { recordError } from "../instrumentation/outcomes";
import {
type CliAuth,
getCliProcessEnv,
getGlobalFlags,
mayUseCliStore,
resolveCliAuth,
Expand Down Expand Up @@ -76,7 +77,7 @@ export class CliCredentialManager {
}
span.setProperty("store", cli.auth.store);
await this.exec(cli, ["login", "--use-token-as-session", url], {
env: { ...process.env, CODER_SESSION_TOKEN: token },
env: { ...getCliProcessEnv(), CODER_SESSION_TOKEN: token },
signal: options?.signal,
});
span.setProperty("outcome", "stored");
Expand Down Expand Up @@ -234,6 +235,7 @@ export class CliCredentialManager {
}, EXEC_LOG_INTERVAL_MS);
try {
return await execFileAsync(cli.binPath, [...cli.flags, ...args], {
env: getCliProcessEnv(),
...options,
timeout: EXEC_TIMEOUT_MS,
});
Expand Down
24 changes: 19 additions & 5 deletions src/cli/cliExec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ import { validateIdentifier } from "../common/validation";
import { isAbortError } from "../error/abort";
import { toError } from "../error/normalize";
import { type CliFeatureSet } from "../featureSet";
import { type CliAuth, getGlobalFlags } from "../settings/cli";
import {
type CliAuth,
getCliProcessEnv,
getGlobalFlags,
} from "../settings/cli";

/** Resolved CLI environment for invoking the coder binary. */
export interface CliEnv {
Expand Down Expand Up @@ -86,7 +90,10 @@ export async function speedtest(
}
args.push("--", workspaceName);
try {
const result = await execFileAsync(env.binary, args, { signal });
const result = await execFileAsync(env.binary, args, {
env: getCliProcessEnv(),
signal,
});
return result.stdout;
} catch (error) {
throw cliError(error);
Expand All @@ -105,7 +112,7 @@ export async function netcheck(
const result = await execFileAsync(
env.binary,
[...globalFlags, "netcheck"],
{ signal },
{ env: getCliProcessEnv(), signal },
);
return result.stdout;
} catch (error) {
Expand Down Expand Up @@ -141,7 +148,7 @@ export async function supportBundle(
...(agentName ? [agentName] : []),
];
try {
await execFileAsync(env.binary, args, { signal });
await execFileAsync(env.binary, args, { env: getCliProcessEnv(), signal });
} catch (error) {
throw cliError(error);
}
Expand Down Expand Up @@ -185,7 +192,13 @@ export function openAppStatusTerminal(
app.command,
];
vscode.window
.createTerminal({ name: app.name, shellPath: env.binary, shellArgs: args })
.createTerminal({
name: app.name,
shellPath: env.binary,
shellArgs: args,
env: getCliProcessEnv(),
strictEnv: true,
})
.show(false);
}

Expand Down Expand Up @@ -228,6 +241,7 @@ function spawnCliInTerminal(options: {
// fall back to proc.kill() there.
const useProcessGroup = process.platform !== "win32";
const proc = spawn(options.binary, options.args, {
env: getCliProcessEnv(),
detached: useProcessGroup,
});

Expand Down
13 changes: 9 additions & 4 deletions src/logging/flightRecorder.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { safeStringify } from "./formatters";
import { redactSecrets, safeStringify } from "./formatters";

import type { LogSink, Logger } from "./logger";

Expand Down Expand Up @@ -109,8 +109,13 @@ export class FlightRecorder implements Logger {
/** Pass a call through to the sink and buffer it when below the level. */
private wrap(level: Level): (message: string, ...args: unknown[]) => void {
return (message, ...args) => {
this.record(level, message, args);
this.inner[level](message, ...args);
if (SEVERITY[level] < this.channel.logLevel) {
// The channel drops this entry, so only the buffered copy is formatted.
this.record(level, message, args);
this.inner[level](message, ...args);
} else {
this.inner[level](message, ...args.map(redactSecrets));
}
};
}

Expand All @@ -130,7 +135,7 @@ export class FlightRecorder implements Logger {
}

private record(level: Level, message: string, args: unknown[]): void {
if (this.capacity === 0 || SEVERITY[level] >= this.channel.logLevel) {
if (this.capacity === 0) {
return;
}
const text = [message, ...args.map((arg) => safeStringify(arg) ?? "")].join(
Expand Down
Loading
Loading