-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add getEnvState command with atomic injection #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ interface Storage { | |
| export default class DataStorage { | ||
| private readonly storage: Storage; | ||
| private readonly persistence: SecretStoragePersistence; | ||
| private injected = false; | ||
|
|
||
| private constructor(persistence: SecretStoragePersistence) { | ||
| this.storage = { | ||
|
|
@@ -22,6 +23,9 @@ export default class DataStorage { | |
| const storage = new DataStorage(persistence); | ||
| const persisted = await persistence.loadAll(); | ||
| storage.storage.environment = persisted; | ||
| // Treat restored non-empty state as already injected so that a pod/extension | ||
| // restart does not make consumers wait for a fresh injection that will not come. | ||
| storage.injected = Object.keys(persisted).length > 0; | ||
| logger.info(`Loaded ${Object.keys(persisted).length} persisted env var(s)`); | ||
| return storage; | ||
| } | ||
|
|
@@ -30,11 +34,36 @@ export default class DataStorage { | |
| return this.storage.environment[key]; | ||
| } | ||
|
|
||
| public getAll(): Record<string, string> { | ||
| return { ...this.storage.environment }; | ||
| } | ||
|
|
||
| public isInjected(): boolean { | ||
| return this.injected; | ||
| } | ||
|
|
||
| public async setEnv(key: string, value: string): Promise<void> { | ||
| this.storage.environment[key] = value; | ||
| logger.debug(`Environment variable set: ${key}`); | ||
| if (this.persistence) { | ||
| await this.persistence.saveAll(this.storage.environment); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Atomically applies a whole environment map: updates the in-memory store, | ||
| * persists once, and only then marks the storage as injected. Marking | ||
| * `injected` after the single persist call ensures a concurrent consumer | ||
| * never observes `injected === true` with a partially written map. | ||
| */ | ||
| public async setAll(env: Record<string, string>): Promise<void> { | ||
| for (const [key, value] of Object.entries(env)) { | ||
| this.storage.environment[key] = value; | ||
| } | ||
| logger.debug(`Environment variables set: ${Object.keys(env).length}`); | ||
| if (this.persistence) { | ||
| await this.persistence.saveAll(this.storage.environment); | ||
| } | ||
| this.injected = true; | ||
|
Comment on lines
+60
to
+67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win Do not retain an environment update after persistence fails. Line 61 mutates the live map before Build a candidate map, persist that map, and replace 🤖 Prompt for AI Agents |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,72 @@ | ||
| import * as assert from "assert"; | ||
|
|
||
| // You can import and use all API from the 'vscode' module | ||
| // as well as import your extension to test it | ||
| import * as vscode from "vscode"; | ||
| // import * as myExtension from '../../extension'; | ||
| import DataService from "../service/data"; | ||
| import DataStorage from "../service/storage"; | ||
| import SecretStoragePersistence from "../service/persistence"; | ||
|
|
||
| suite("Extension Test Suite", () => { | ||
| vscode.window.showInformationMessage("Start all tests."); | ||
| // Minimal in-memory SecretStorage so the storage layer can be exercised without | ||
| // a real extension context. | ||
| class InMemorySecretStorage implements vscode.SecretStorage { | ||
| private data = new Map<string, string>(); | ||
| private emitter = new vscode.EventEmitter<vscode.SecretStorageChangeEvent>(); | ||
| public readonly onDidChange = this.emitter.event; | ||
|
|
||
| test("Sample test", () => { | ||
| assert.strictEqual(-1, [1, 2, 3].indexOf(5)); | ||
| assert.strictEqual(-1, [1, 2, 3].indexOf(0)); | ||
| async get(key: string): Promise<string | undefined> { | ||
| return this.data.get(key); | ||
| } | ||
| async store(key: string, value: string): Promise<void> { | ||
| this.data.set(key, value); | ||
| this.emitter.fire({ key }); | ||
| } | ||
| async delete(key: string): Promise<void> { | ||
| this.data.delete(key); | ||
| this.emitter.fire({ key }); | ||
| } | ||
| async keys(): Promise<string[]> { | ||
| return [...this.data.keys()]; | ||
| } | ||
| } | ||
|
|
||
| async function newService(): Promise<DataService> { | ||
| const persistence = new SecretStoragePersistence(new InMemorySecretStorage()); | ||
| const storage = await DataStorage.withPersistence(persistence); | ||
| return new DataService(storage); | ||
| } | ||
|
|
||
| suite("Data Bridge - arbitrary env injection", () => { | ||
| test("getEnvState reports not-injected before any injection", async () => { | ||
| const service = await newService(); | ||
| const state = service.getEnvState(); | ||
| assert.strictEqual(state.injected, false); | ||
| assert.deepStrictEqual(state.environment, {}); | ||
| }); | ||
|
|
||
| test("inject stores arbitrary keys and getEnvState returns them all as ready", async () => { | ||
| const service = await newService(); | ||
| await service.inject({ | ||
| environment: { | ||
| THEIA: "true", | ||
| ARTEMIS_TOKEN: "token-123", | ||
| MY_VAR: "hello", | ||
| }, | ||
| }); | ||
|
|
||
| const state = service.getEnvState(); | ||
| assert.strictEqual(state.injected, true); | ||
| assert.deepStrictEqual(state.environment, { | ||
| THEIA: "true", | ||
| ARTEMIS_TOKEN: "token-123", | ||
| MY_VAR: "hello", | ||
| }); | ||
| }); | ||
|
|
||
| test("getEnv still returns only the requested keys", async () => { | ||
| const service = await newService(); | ||
| await service.inject({ environment: { A: "1", B: "2", C: "3" } }); | ||
| assert.deepStrictEqual(service.getEnvVars(["A", "C", "MISSING"]), { | ||
| A: "1", | ||
| C: "3", | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🔎 Supported by static analysis
🏁 Script executed:
Repository: EduIDE/EduIDE-data-bridge
Length of output: 23478
🌐 Web query:
Visual Studio Code Extension API commands.registerCommand executeCommand other extensions authorization caller identity💡 Result:
The Visual Studio Code Extension API does not provide a built-in mechanism to identify the caller of a command registered via vscode.commands.registerCommand [1][2]. When a command handler is invoked—whether via executeCommand, the Command Palette, a keybinding, or a UI gesture—the handler function receives only the arguments passed during the execution call [1][2]. The API does not automatically inject metadata regarding the identity, extension ID, or source of the caller [2]. If you need to establish a secure or authenticated interaction between extensions, consider the following patterns: 1. Use Extension Exports: Instead of relying on commands for inter-extension communication, define a formal public API by exporting an object from your extension's activate function [3]. Other extensions can then access this via vscode.extensions.getExtension('extension.id').exports [3]. This approach allows for direct, type-safe method calls rather than relying on loosely coupled command IDs [3]. 2. Pass Identity in Arguments: If you must use commands, you can require callers to pass an identity token or a callback object as part of the command arguments [2]. However, note that this relies on the caller voluntarily providing this information and is not a secure authorization mechanism, as any extension can craft the required arguments [2]. 3. Command Registration Constraints: The VS Code command registry does not implement authorization checks natively [4]. All registered commands are essentially global; once registered, any extension or UI component can execute them [1][2]. If you need to restrict execution, you must implement those checks logic-side within your command handler by verifying the context or state, though you cannot reliably restrict who is allowed to call the command from the API layer [1][2]. In summary, there is no technical "caller identity" field in the command execution flow, and you should design your extension's cross-extension communication based on the assumption that any command is publicly discoverable and callable by other extensions [1][2].
Citations:
Do not expose the complete environment through a global command.
getEnvStatereturns all persisted environment values, includingARTEMIS_TOKEN. VS Code commands provide no caller identity or authorization, and other extensions can invoke registered commands withvscode.commands.executeCommand. Expose only non-sensitive values through this command and move secret access to a deliberately scoped extension API.🤖 Prompt for AI Agents