From eafdabc48cfa04c5d89d54c5c862a7b58e9ed545 Mon Sep 17 00:00:00 2001 From: Atif Ali Date: Sat, 13 Dec 2025 12:54:13 +0000 Subject: [PATCH] fix(zed): fix settings JSON parsing with base64 encoding The previous implementation used quote escaping which produced invalid JSON: SETTINGS_JSON='{"theme":"dark"}' This broke jq parsing because the escaped quotes are not valid JSON. Changed to use base64 encoding internally: settings_b64 = base64encode(var.settings) echo -n "${SETTINGS_B64}" | base64 -d > settings.json User interface remains the same - pass JSON via jsonencode() or heredoc. Simplified script to just write settings directly (no jq merging) since Zed already handles merging remote and local settings internally. Also added end-to-end container tests and fixed existing tests to use override_data for workspace mocking. --- registry/coder/modules/zed/main.test.ts | 105 +++++++++++----------- registry/coder/modules/zed/main.tf | 13 +-- registry/coder/modules/zed/zed.tftest.hcl | 74 +++++++++++++++ 3 files changed, 133 insertions(+), 59 deletions(-) diff --git a/registry/coder/modules/zed/main.test.ts b/registry/coder/modules/zed/main.test.ts index 12413750..8e5ec792 100644 --- a/registry/coder/modules/zed/main.test.ts +++ b/registry/coder/modules/zed/main.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from "bun:test"; import { + execContainer, + findResourceInstance, + removeContainer, + runContainer, runTerraformApply, runTerraformInit, testRequiredVariables, @@ -12,66 +16,67 @@ describe("zed", async () => { agent_id: "foo", }); - it("default output", async () => { + it("creates settings file with correct JSON", async () => { + const settings = { + theme: "One Dark", + buffer_font_size: 14, + vim_mode: true, + telemetry: { + diagnostics: false, + metrics: false, + }, + // Test special characters: single quotes, backslashes, URLs + message: "it's working", + path: "C:\\Users\\test", + api_url: "https://api.example.com/v1?token=abc&user=test", + }; + const state = await runTerraformApply(import.meta.dir, { agent_id: "foo", + settings: JSON.stringify(settings), }); - expect(state.outputs.zed_url.value).toBe("zed://ssh/default.coder"); - const coder_app = state.resources.find( - (res) => res.type === "coder_app" && res.name === "zed", - ); + const instance = findResourceInstance(state, "coder_script"); + const id = await runContainer("alpine:latest"); - expect(coder_app).not.toBeNull(); - expect(coder_app?.instances.length).toBe(1); - expect(coder_app?.instances[0].attributes.order).toBeNull(); - }); + try { + const result = await execContainer(id, ["sh", "-c", instance.script]); + expect(result.exitCode).toBe(0); - it("adds folder", async () => { + const catResult = await execContainer(id, [ + "cat", + "/root/.config/zed/settings.json", + ]); + expect(catResult.exitCode).toBe(0); + + const written = JSON.parse(catResult.stdout.trim()); + expect(written).toEqual(settings); + } finally { + await removeContainer(id); + } + }, 30000); + + it("exits early with empty settings", async () => { const state = await runTerraformApply(import.meta.dir, { agent_id: "foo", - folder: "/foo/bar", - }); - expect(state.outputs.zed_url.value).toBe("zed://ssh/default.coder/foo/bar"); - }); - - it("expect order to be set", async () => { - const state = await runTerraformApply(import.meta.dir, { - agent_id: "foo", - order: "22", + settings: "", }); - const coder_app = state.resources.find( - (res) => res.type === "coder_app" && res.name === "zed", - ); + const instance = findResourceInstance(state, "coder_script"); + const id = await runContainer("alpine:latest"); - expect(coder_app).not.toBeNull(); - expect(coder_app?.instances.length).toBe(1); - expect(coder_app?.instances[0].attributes.order).toBe(22); - }); + try { + const result = await execContainer(id, ["sh", "-c", instance.script]); + expect(result.exitCode).toBe(0); - it("expect display_name to be set", async () => { - const state = await runTerraformApply(import.meta.dir, { - agent_id: "foo", - display_name: "Custom Zed", - }); - - const coder_app = state.resources.find( - (res) => res.type === "coder_app" && res.name === "zed", - ); - - expect(coder_app).not.toBeNull(); - expect(coder_app?.instances.length).toBe(1); - expect(coder_app?.instances[0].attributes.display_name).toBe("Custom Zed"); - }); - - it("adds agent_name to hostname", async () => { - const state = await runTerraformApply(import.meta.dir, { - agent_id: "foo", - agent_name: "myagent", - }); - expect(state.outputs.zed_url.value).toBe( - "zed://ssh/myagent.default.default.coder", - ); - }); + // Settings file should not be created + const catResult = await execContainer(id, [ + "cat", + "/root/.config/zed/settings.json", + ]); + expect(catResult.exitCode).not.toBe(0); + } finally { + await removeContainer(id); + } + }, 30000); }); diff --git a/registry/coder/modules/zed/main.tf b/registry/coder/modules/zed/main.tf index 745254be..6085a93b 100644 --- a/registry/coder/modules/zed/main.tf +++ b/registry/coder/modules/zed/main.tf @@ -65,6 +65,7 @@ locals { owner_name = lower(data.coder_workspace_owner.me.name) agent_name = lower(var.agent_name) hostname = var.agent_name != "" ? "${local.agent_name}.${local.workspace_name}.${local.owner_name}.coder" : "${local.workspace_name}.coder" + settings_b64 = var.settings != "" ? base64encode(var.settings) : "" } resource "coder_script" "zed_settings" { @@ -75,20 +76,14 @@ resource "coder_script" "zed_settings" { script = <<-EOT #!/usr/bin/env bash set -eu - SETTINGS_JSON='${replace(var.settings, "\"", "\\\"")}' - if [ -z "$${SETTINGS_JSON}" ] || [ "$${SETTINGS_JSON}" = "{}" ]; then + SETTINGS_B64='${local.settings_b64}' + if [ -z "$${SETTINGS_B64}" ]; then exit 0 fi CONFIG_HOME="$${XDG_CONFIG_HOME:-$HOME/.config}" ZED_DIR="$${CONFIG_HOME}/zed" mkdir -p "$${ZED_DIR}" - SETTINGS_FILE="$${ZED_DIR}/settings.json" - if command -v jq >/dev/null 2>&1 && [ -s "$${SETTINGS_FILE}" ]; then - tmpfile="$(mktemp)" - jq -s '.[0] * .[1]' "$${SETTINGS_FILE}" <(printf '%s\n' "$${SETTINGS_JSON}") > "$${tmpfile}" && mv "$${tmpfile}" "$${SETTINGS_FILE}" - else - printf '%s\n' "$${SETTINGS_JSON}" > "$${SETTINGS_FILE}" - fi + echo -n "$${SETTINGS_B64}" | base64 -d > "$${ZED_DIR}/settings.json" EOT } diff --git a/registry/coder/modules/zed/zed.tftest.hcl b/registry/coder/modules/zed/zed.tftest.hcl index 508b6550..339f6876 100644 --- a/registry/coder/modules/zed/zed.tftest.hcl +++ b/registry/coder/modules/zed/zed.tftest.hcl @@ -5,6 +5,20 @@ run "default_output" { agent_id = "foo" } + override_data { + target = data.coder_workspace.me + values = { + name = "default" + } + } + + override_data { + target = data.coder_workspace_owner.me + values = { + name = "default" + } + } + assert { condition = output.zed_url == "zed://ssh/default.coder" error_message = "zed_url did not match expected default URL" @@ -19,6 +33,20 @@ run "adds_folder" { folder = "/foo/bar" } + override_data { + target = data.coder_workspace.me + values = { + name = "default" + } + } + + override_data { + target = data.coder_workspace_owner.me + values = { + name = "default" + } + } + assert { condition = output.zed_url == "zed://ssh/default.coder/foo/bar" error_message = "zed_url did not include provided folder path" @@ -33,8 +61,54 @@ run "adds_agent_name" { agent_name = "myagent" } + override_data { + target = data.coder_workspace.me + values = { + name = "default" + } + } + + override_data { + target = data.coder_workspace_owner.me + values = { + name = "default" + } + } + assert { condition = output.zed_url == "zed://ssh/myagent.default.default.coder" error_message = "zed_url did not include agent_name in hostname" } } + +run "settings_base64_encoding" { + command = apply + + variables { + agent_id = "foo" + settings = jsonencode({ + theme = "dark" + fontSize = 14 + }) + } + + # Verify settings are base64 encoded (eyJ = base64 prefix for JSON starting with {") + assert { + condition = can(regex("SETTINGS_B64='eyJ", coder_script.zed_settings.script)) + error_message = "settings should be base64 encoded in the script" + } +} + +run "empty_settings" { + command = apply + + variables { + agent_id = "foo" + settings = "" + } + + assert { + condition = can(regex("SETTINGS_B64=''", coder_script.zed_settings.script)) + error_message = "empty settings should result in empty SETTINGS_B64" + } +}