From f41e0cc6d47fb36570fcd40641a26f6eb5d2e52a Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Fri, 6 Nov 2026 04:29:51 +0100 Subject: [PATCH] fix: dot segments and doubled slashes cannot spell around the data folder refusals (release review) The refusals of $HOME, ~/.codeman and the checkout compare strings, so `~/../me` or `/../repo/data` passed them: the first shares state with a native install, the second lands in the build context, where `COPY . .` would bake CLI logins into the image. Doubled slashes are collapsed and any `.` or `..` segment is refused. Co-Authored-By: Claude Opus 5.5 --- docker/Start-Codeman.sh | 11 +++++++++++ test/docker-start-setup.test.ts | 13 +++++++++++++ 2 files changed, 24 insertions(+) diff --git a/docker/Start-Codeman.sh b/docker/Start-Codeman.sh index 0109b69a..41fa2678 100644 --- a/docker/Start-Codeman.sh +++ b/docker/Start-Codeman.sh @@ -230,6 +230,9 @@ accept_appdata_path() { '~') p=$HOME ;; '~/'*) p="$HOME/${p#\~/}" ;; esac + while [[ "$p" == *//* ]]; do + p=${p//\/\//\/} + done while [[ "$p" == */ && "$p" != / ]]; do p=${p%/} done @@ -247,6 +250,14 @@ accept_appdata_path() { printf ' Use an absolute path, one that starts with /.\n' >&2 return 1 fi + # The refusals below compare strings, so a dot segment could spell any folder + # they exist to keep out. + case "$p/" in + */./* | */../*) + printf ' Spell the path out, without . or .. in it.\n' >&2 + return 1 + ;; + esac # The folder becomes the container's home directory, so its `.codeman` is # the server's state directory: $HOME itself would share state.json with a # Codeman installed directly on this machine. diff --git a/test/docker-start-setup.test.ts b/test/docker-start-setup.test.ts index 25d517eb..f207b286 100644 --- a/test/docker-start-setup.test.ts +++ b/test/docker-start-setup.test.ts @@ -343,6 +343,13 @@ describe('Start-Codeman.sh first run (no docker/.env yet)', () => { ['a relative path', () => 'codeman-data', /absolute path/], ['the checkout, which is the image build context', ({ repo }: { repo: string }) => repo, /copied into the image/], ['a folder inside the checkout', ({ repo }: { repo: string }) => join(repo, 'data'), /copied into the image/], + // join() would resolve these, which is the whole point: the refusals above compare strings. + ['HOME spelled through ..', ({ home }: { home: string }) => `${home}/../home`, /without \. or \.\./], + ['the checkout spelled through ..', ({ home }: { home: string }) => `${home}/../repo/data`, /without \. or \.\./], + ['a path ending in ..', ({ home }: { home: string }) => `${home}/data/..`, /without \. or \.\./], + ['a native install spelled through .', ({ home }: { home: string }) => `${home}/./.codeman`, /without \. or \.\./], + ['a native install behind a doubled slash', ({ home }: { home: string }) => `${home}//.codeman`, /installed directly/], + ['the checkout behind a doubled slash', ({ repo }: { repo: string }) => `${repo}//data`, /copied into the image/], ])('refuses %s as the data folder and writes nothing', (_name, appdata, reason) => { const r = runStart(['--setup-only'], { appdata }); expect(r.status).toBe(1); @@ -402,6 +409,12 @@ describe('Start-Codeman.sh first run (no docker/.env yet)', () => { expect(r.envKind).toBe('symlink'); }); + it('keeps a folder name that merely starts with a dot or dots', () => { + const r = runStart(['--setup-only'], { appdata: ({ home }) => `${home}/..data/.state` }); + expect(r.status, r.stderr).toBe(0); + expect(envValue(r.env as string, 'CODEMAN_APPDATA_PATH')).toBe(`${r.home}/..data/.state`); + }); + it('never rewrites an existing docker/.env', () => { const existing = '# hand-written\nCODEMAN_PASSWORD=mine-and-only-mine\nCODEMAN_APPDATA_PATH=/x\n'; const r = runStart(['--setup-only'], { existingEnv: existing });