fix(config): require absolute goose path roots (#10454)
This commit is contained in:
@@ -1,12 +1,12 @@
|
||||
use etcetera::{choose_app_strategy, AppStrategy, AppStrategyArgs};
|
||||
use std::ffi::OsString;
|
||||
use std::path::PathBuf;
|
||||
|
||||
pub struct Paths;
|
||||
|
||||
impl Paths {
|
||||
fn get_dir(dir_type: DirType) -> PathBuf {
|
||||
if let Ok(test_root) = std::env::var("GOOSE_PATH_ROOT") {
|
||||
let base = PathBuf::from(test_root);
|
||||
if let Some(base) = Self::path_root() {
|
||||
match dir_type {
|
||||
DirType::Config => base.join("config"),
|
||||
DirType::Data => base.join("data"),
|
||||
@@ -37,6 +37,14 @@ impl Paths {
|
||||
}
|
||||
}
|
||||
|
||||
fn path_root() -> Option<PathBuf> {
|
||||
Self::validated_path_root(std::env::var_os("GOOSE_PATH_ROOT"))
|
||||
}
|
||||
|
||||
fn validated_path_root(value: Option<OsString>) -> Option<PathBuf> {
|
||||
value.map(PathBuf::from).filter(|path| path.is_absolute())
|
||||
}
|
||||
|
||||
pub fn config_dir() -> PathBuf {
|
||||
Self::get_dir(DirType::Config)
|
||||
}
|
||||
@@ -86,3 +94,27 @@ enum DirType {
|
||||
Agents,
|
||||
AgentsHome,
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::Paths;
|
||||
use std::ffi::OsString;
|
||||
|
||||
#[test]
|
||||
fn path_root_requires_an_absolute_path() {
|
||||
assert_eq!(Paths::validated_path_root(None), None);
|
||||
assert_eq!(Paths::validated_path_root(Some(OsString::new())), None);
|
||||
assert_eq!(
|
||||
Paths::validated_path_root(Some(OsString::from("relative/root"))),
|
||||
None
|
||||
);
|
||||
|
||||
let absolute = std::env::current_dir()
|
||||
.unwrap()
|
||||
.join("nonexistent-goose-root");
|
||||
assert_eq!(
|
||||
Paths::validated_path_root(Some(absolute.clone().into_os_string())),
|
||||
Some(absolute)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,12 +1,12 @@
|
||||
use etcetera::{choose_app_strategy, AppStrategy, AppStrategyArgs};
|
||||
use std::ffi::OsString;
|
||||
use std::path::PathBuf;
|
||||
|
||||
pub struct Paths;
|
||||
|
||||
impl Paths {
|
||||
fn get_dir(dir_type: DirType) -> PathBuf {
|
||||
if let Ok(test_root) = std::env::var("GOOSE_PATH_ROOT") {
|
||||
let base = PathBuf::from(test_root);
|
||||
if let Some(base) = Self::path_root() {
|
||||
match dir_type {
|
||||
DirType::Config => base.join("config"),
|
||||
DirType::Data => base.join("data"),
|
||||
@@ -37,6 +37,14 @@ impl Paths {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn path_root() -> Option<PathBuf> {
|
||||
Self::validated_path_root(std::env::var_os("GOOSE_PATH_ROOT"))
|
||||
}
|
||||
|
||||
fn validated_path_root(value: Option<OsString>) -> Option<PathBuf> {
|
||||
value.map(PathBuf::from).filter(|path| path.is_absolute())
|
||||
}
|
||||
|
||||
pub fn config_dir() -> PathBuf {
|
||||
Self::get_dir(DirType::Config)
|
||||
}
|
||||
@@ -86,3 +94,27 @@ enum DirType {
|
||||
Agents,
|
||||
AgentsHome,
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::Paths;
|
||||
use std::ffi::OsString;
|
||||
|
||||
#[test]
|
||||
fn path_root_requires_an_absolute_path() {
|
||||
assert_eq!(Paths::validated_path_root(None), None);
|
||||
assert_eq!(Paths::validated_path_root(Some(OsString::new())), None);
|
||||
assert_eq!(
|
||||
Paths::validated_path_root(Some(OsString::from("relative/root"))),
|
||||
None
|
||||
);
|
||||
|
||||
let absolute = std::env::current_dir()
|
||||
.unwrap()
|
||||
.join("nonexistent-goose-root");
|
||||
assert_eq!(
|
||||
Paths::validated_path_root(Some(absolute.clone().into_os_string())),
|
||||
Some(absolute)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3,7 +3,7 @@ use std::path::{Path, PathBuf};
|
||||
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use crate::config::Config;
|
||||
use crate::config::{paths::Paths, Config};
|
||||
use crate::plugins::plugin_install_dir;
|
||||
|
||||
const PLUGINS_CONFIG_KEY: &str = "plugins";
|
||||
@@ -192,9 +192,9 @@ fn load_all_settings(project_root: Option<&Path>) -> Vec<(SettingsScope, PluginS
|
||||
}
|
||||
|
||||
fn user_settings_path() -> Option<PathBuf> {
|
||||
if let Ok(test_root) = std::env::var("GOOSE_PATH_ROOT") {
|
||||
if let Some(path_root) = Paths::path_root() {
|
||||
return Some(
|
||||
PathBuf::from(test_root)
|
||||
path_root
|
||||
.join(".config")
|
||||
.join("goose")
|
||||
.join("settings.json"),
|
||||
|
||||
+2
-10
@@ -30,7 +30,7 @@ import { checkBackendStatus } from './backendStatus';
|
||||
import { startGooseServe } from './gooseServe';
|
||||
import { GooseServeLeaseRegistry, type GooseServeLease } from './gooseServeLeaseRegistry';
|
||||
import { acpWebSocketUrlFromHttpBase, normalizeAcpHttpBaseUrl } from './acp/url';
|
||||
import { expandTilde } from './utils/pathUtils';
|
||||
import { expandTilde, sanitizeGoosePathRoot } from './utils/pathUtils';
|
||||
import log from './utils/logger';
|
||||
import { ensureWinShims } from './utils/winShims';
|
||||
import { addRecentDir, loadRecentDirs } from './utils/recentDirs';
|
||||
@@ -861,14 +861,6 @@ const getBundledConfig = (): BundledConfig => {
|
||||
|
||||
const { defaultProvider, defaultModel, predefinedModels, version } = getBundledConfig();
|
||||
|
||||
const resolveGoosePathRoot = (): string | undefined => {
|
||||
const pathRoot = process.env.GOOSE_PATH_ROOT?.trim();
|
||||
if (pathRoot) {
|
||||
return expandTilde(pathRoot);
|
||||
}
|
||||
return undefined;
|
||||
};
|
||||
|
||||
const GENERATED_SECRET = crypto.randomBytes(32).toString('hex');
|
||||
|
||||
interface ExternalBackend {
|
||||
@@ -954,7 +946,7 @@ let appConfig = {
|
||||
GOOSE_DEFAULT_PROVIDER: defaultProvider,
|
||||
GOOSE_DEFAULT_MODEL: defaultModel,
|
||||
GOOSE_PREDEFINED_MODELS: predefinedModels,
|
||||
GOOSE_PATH_ROOT: resolveGoosePathRoot(),
|
||||
GOOSE_PATH_ROOT: sanitizeGoosePathRoot(process.env),
|
||||
GOOSE_WORKING_DIR: '',
|
||||
// Start with the env-var override; the OS region locale is filled in after app.ready
|
||||
// (see updateLocaleFromSystem below) since getSystemLocale() cannot be called earlier.
|
||||
|
||||
@@ -0,0 +1,35 @@
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { isAbsoluteGoosePath, resolveGoosePathRoot, sanitizeGoosePathRoot } from './pathUtils';
|
||||
|
||||
describe('resolveGoosePathRoot', () => {
|
||||
it('rejects empty and relative values', () => {
|
||||
expect(resolveGoosePathRoot(undefined)).toBeUndefined();
|
||||
expect(resolveGoosePathRoot(' ')).toBeUndefined();
|
||||
expect(resolveGoosePathRoot('relative/root')).toBeUndefined();
|
||||
});
|
||||
|
||||
it('retains absolute paths without requiring them to exist', () => {
|
||||
const absolute = path.resolve('nonexistent-goose-root');
|
||||
expect(resolveGoosePathRoot(` ${absolute} `)).toBe(absolute);
|
||||
});
|
||||
|
||||
it('expands a home-relative root before validation', () => {
|
||||
expect(resolveGoosePathRoot('~')).toBe(os.homedir());
|
||||
});
|
||||
|
||||
it('removes a rejected value from the child-process environment', () => {
|
||||
const env = { GOOSE_PATH_ROOT: 'relative/root' };
|
||||
expect(sanitizeGoosePathRoot(env)).toBeUndefined();
|
||||
expect(env).not.toHaveProperty('GOOSE_PATH_ROOT');
|
||||
});
|
||||
|
||||
it('matches Rust absolute-path handling on Windows', () => {
|
||||
expect(isAbsoluteGoosePath('C:\\goose\\root', 'win32')).toBe(true);
|
||||
expect(isAbsoluteGoosePath('\\\\server\\share\\goose', 'win32')).toBe(true);
|
||||
expect(isAbsoluteGoosePath('C:goose\\root', 'win32')).toBe(false);
|
||||
expect(isAbsoluteGoosePath('\\goose\\root', 'win32')).toBe(false);
|
||||
expect(isAbsoluteGoosePath('/goose/root', 'win32')).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -23,3 +23,35 @@ export function expandTilde(filePath: string): string {
|
||||
}
|
||||
return filePath;
|
||||
}
|
||||
|
||||
export function resolveGoosePathRoot(value: string | undefined): string | undefined {
|
||||
const trimmed = value?.trim();
|
||||
if (!trimmed) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const expanded = expandTilde(trimmed);
|
||||
return isAbsoluteGoosePath(expanded) ? expanded : undefined;
|
||||
}
|
||||
|
||||
export function isAbsoluteGoosePath(
|
||||
filePath: string,
|
||||
platform: 'win32' | 'posix' = process.platform === 'win32' ? 'win32' : 'posix'
|
||||
): boolean {
|
||||
if (platform !== 'win32') {
|
||||
return path.posix.isAbsolute(filePath);
|
||||
}
|
||||
|
||||
const root = path.win32.parse(filePath).root;
|
||||
return path.win32.isAbsolute(filePath) && root.length > 1;
|
||||
}
|
||||
|
||||
export function sanitizeGoosePathRoot(env: { GOOSE_PATH_ROOT?: string }): string | undefined {
|
||||
const pathRoot = resolveGoosePathRoot(env.GOOSE_PATH_ROOT);
|
||||
if (pathRoot) {
|
||||
env.GOOSE_PATH_ROOT = pathRoot;
|
||||
} else {
|
||||
delete env.GOOSE_PATH_ROOT;
|
||||
}
|
||||
return pathRoot;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user