Cat A fork-prep fixes: console hardcode, vault dir, vault secret#10
Merged
Conversation
The Windows PowerShell parent-process check compared parent.Executable()
against the literal "cdt"/"cdt.exe". After the binary rename the
comparison silently fails, breaking the ANSI-detection heuristic for any
launcher invoked via itself (hooks, package commands) under PowerShell —
ANSI sequences get emitted as raw text.
filepath.Base(os.Executable()) returns the same shape parent.Executable()
already returns ("cola"/"cola.exe", or whatever the binary is named at
runtime), with no context dependency — important since this runs at
package-init time before InitContext().
The hardcoded ~/.file-vault/ directory was shared across every binary built from this codebase, so two launchers with different names (e.g. cola and mydt) collided on every vault entry — silent data corruption between binaries. Move the vault under the per-launcher tree (mirrors the existing config / dropins / logs layout). Inline AppDir resolution rather than importing config to avoid a circular import (config → helper → gvault). Migration: at init time, if no entry exists at the new path but one exists at the legacy ~/.file-vault/<name> path, copy the legacy file in place. Single-binary users keep their data; multi-binary users need to re-login on the second binary, which was already the implicit contract. The legacy-path fallback is intended to be removed one release after this lands.
readSecret() previously fell through to hashing ~/.ssh/id_rsa when neither <APPNAME>_VAULT_SECRET nor <APPNAME>_VAULT_SECRET_FILE was set. That coupling was implicit and load-bearing: users who never asked for it had their vault recoverability tied to an SSH key they might rotate, and CI runners with no ~/.ssh saw silent failures that only surfaced as cascading test errors much higher up the stack. Drop the SSH default. If neither env var is set, return an error that names both env vars so the caller knows exactly what to set. Existing users on a non-default vault secret are unaffected; existing users on the SSH default need to either set <APPNAME>_VAULT_SECRET_FILE=~/.ssh/id_rsa explicitly to keep current behaviour, or pick a new secret and re-store credentials. Surfaced during the env-var split PR (#9) where four integration suites failed on CI for this exact reason and passed locally because dev machines have an ~/.ssh.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three independent Category-A fork-prep fixes, each its own commit. Closes the bulk of the structural-correctness cleanup left over from the cdt → cola rename.
8354677internal/console/console.go: Windows parent-process detection comparedparent.Executable()against the literal"cdt"/"cdt.exe". After the rename the comparison silently fails and the ANSI heuristic breaks for any launcher invoked via itself (hooks, package commands) under PowerShell. Derive the name fromfilepath.Base(os.Executable())so it tracks whatever the binary is called at runtime. Runs at package-init time, beforeInitContext(), so context isn't an option here.445afafinternal/gvault/file-vault.go: the vault directory was hardcoded to~/.file-vault/, shared across every binary built from this codebase. Two launchers with different names collide on every vault entry — silent data corruption. Scope under<AppDir>/file-vault/(mirrors the existing config / dropins / logs layout). One-release migration: at init, if no entry exists at the new path but one exists at the legacy path, copy it in place; new writes always go to the new path. AppDir resolution is inlined to avoidconfig → helper → gvaultcircular import.6132319internal/gvault/file-vault.goreadSecret(): dropped the silent fallback to hashing~/.ssh/id_rsaas the vault key. That coupling was load-bearing — vault recoverability was tied to an SSH key the user might rotate, and CI runners with no~/.sshsaw cascading failures (this exact issue surfaced during PR #9). Now requires an explicit<APPNAME>_VAULT_SECRET(literal) or<APPNAME>_VAULT_SECRET_FILE(path); error message names both env vars when neither is set. Existing users on the SSH default need to either set<APPNAME>_VAULT_SECRET_FILE=~/.ssh/id_rsato preserve current behaviour or pick a new secret.Behaviour change
The third commit is a breaking change for any user who was implicitly relying on the SSH-key default to encrypt their file vault. They'll see an explicit error pointing at the two env vars they can set. Worth a release note when this lands.
Test plan
Verified locally before push:
./build.shcleango build ./... && go vet ./...cleango test ./...all packages pass./test/integration.sh19/19 suites pass