Skip to content

[Hackathon] fix(project): quote environment recovery commands to preserve the target #560

Open

Description

@walter-w-white

Environment recovery commands can update a different environment when a name contains spaces.

Verified against commit d61e6dfc400f5e88ee34ac799c5496ac5c5a7751, CLI 0.15.0, Linux, Node 22.23.3 and Bash 5.2. All requests below use a loopback compatibility fixture, never TestSprite services.

When an environment create/update is acknowledged but --clear-credentials or --var is not applied, confirmRequestedEffects() builds an actionable recovery command. It inserts the acknowledged environment name without shell quoting. This breaks a legitimate multiword name; the CLI accepts excess positionals and can actually write to a different existing environment.

For example, a PATCH to old renames it to QA West but leaves its credentials unchanged. The initial CLI correctly exits 7/UNSUPPORTED and recommends:

testsprite project env update proj_lab QA West --clear-credentials

With environments QA West and QA present, copying that command sends PATCH to /projects/proj_lab/env/QA, clears the QA sibling, and exits 0. The intended QA West credentials remain unchanged. Quoting 'QA West' sends the intended request and succeeds. With no QA sibling, the suggested command fails with NOT_FOUND. Owner's QA instead causes a Bash syntax error before CLI startup. These are one command-construction defect.

Expected: the generated recovery command preserves the acknowledged project/environment identity as literal arguments, repairs only the ignored effect, and leaves sibling environments alone. Existing UNSUPPORTED classification and the partial-write receipt should remain unchanged.

Reproduction (Python standard library, built CLI, local Node22 and Bash; creates only a disposable output directory):

python3 reproduce.py --minimal --node /absolute/path/to/node22 --cli /absolute/path/to/testsprite-cli/dist/index.js --out ./recovery-repro

Expected regression result on the tested SHA: script exits 1; plain-name control succeeds, multiword suggestion exits0 while modifying the wrong sibling, apostrophe suggestion exits2; every manually quoted correction and fresh GET succeeds. Exact argv, emitted command, HTTP request/response ledger, stdout/stderr and before/after state are saved under the output directory. The fixture's initial ignored-field behavior is explicit and documented as a compatibility path; this is not a claim that current hosted servers ignore the flag.

Self-contained reproduction script
"""Native CLI -> emitted shell command -> native CLI -> independently stored state.
No HOME override, live endpoints, real credentials, timers or production edits.
"""
import argparse, copy, hashlib, json, os, pathlib, random, re, shlex, subprocess, threading, time
from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
from urllib.parse import unquote, urlsplit
ROOT=pathlib.Path('/root/testsprite-cli-lab'); HERE=pathlib.Path(__file__).resolve().parent
NODE=ROOT/'.runtime/node_modules/.bin/node'; CLI=ROOT/'upstream/dist/index.js'; GUARD=None
GUARD_TEXT="const os = require('node:os');\nos.homedir = () => process.env.LAB_FIXTURE_HOME;\nrequire('node:module').syncBuiltinESMExports();\nconst nativeFetch = globalThis.fetch;\nglobalThis.fetch = (input, init) => {\n  const url = new URL(typeof input === 'string' || input instanceof URL ? input : input.url);\n  if (url.origin !== process.env.LAB_ORIGIN) throw new Error('Fixture denied non-loopback origin');\n  return nativeFetch(input, init);\n};\n"
def dump(p,d): p.parent.mkdir(parents=True,exist_ok=True);p.write_text(json.dumps(d,ensure_ascii=False,indent=2)+'\n')
def envrow(name,ident):
 return dict(id=ident,name=name,url='https://app.example.test',isDefault=False,authMode='account',hasCredentials=True,username='fixture-user',enableOtp=False,updatedAt='2026-10-18T01:00:00Z',variables={})
class Fixture:
 def __init__(self,cfg,path):
  self.cfg=cfg;self.path=path;self.phase='initial';self.requests=[];self.store={'old':envrow('old','env-target'),'baseline':envrow('baseline','env-base')};self.counter=0
  if cfg['operation']=='create':del self.store['old']
  if cfg.get('sibling'): self.store[cfg['sibling']]=envrow(cfg['sibling'],'env-sibling')
  self.initial=copy.deepcopy(self.store)
 def handle(self,method,raw,body,headers):
  parts=[unquote(p) for p in urlsplit(raw).path.split('/') if p]
  row=dict(method=method,path=raw,decoded_parts=parts,phase=self.phase,body=body,headers={k:v for k,v in headers.items() if k.lower() in ['idempotency-key','x-request-id']},monotonic=time.monotonic())
  status=200
  error=lambda code,msg:dict(error=dict(code=code,message=msg,nextAction='Inspect fixture inventory.',requestId='fixture-recovery',details={'resource':'environment'}))
  if parts[:5]!=['api','cli','v1','projects','proj_lab'] or len(parts)<6 or parts[5]!='env':status,res=404,error('NOT_FOUND','Unknown route')
  elif method=='GET':
   if len(parts)==6:res={'environments':list(self.store.values())}
   elif parts[6] in self.store:res={'environment':self.store[parts[6]]}
   else:status,res=404,error('NOT_FOUND','Environment not found')
  elif method in ('POST','PATCH'):
   self.counter+=1
   if self.phase=='initial' and self.cfg.get('retry') and self.counter==1: status,res=503,error('UNAVAILABLE','One transient fixture refusal')
   elif method=='PATCH' and parts[6] not in self.store:status,res=404,error('NOT_FOUND','Environment not found')
   else:
    old=parts[6] if method=='PATCH' else body['name']
    e=copy.deepcopy(self.store[old]) if method=='PATCH' else envrow(old,'env-target')
    if 'rename' in body:e['name']=body['rename']
    if 'url' in body:e['url']=body['url']
    ignore=self.cfg['ignore'] if self.phase=='initial' else []
    if body.get('clearCredentials') and 'clear' not in ignore:e.update(hasCredentials=False,username=None,authMode='public')
    if 'variables' in body and 'var' not in ignore:e['variables'].update(body['variables'])
    if method=='PATCH':del self.store[old]
    self.store[e['name']]=e;res={'environment':e}
    if method=='POST':res['created']=True;status=201
  else:status,res=405,error('NOT_FOUND','Wrong method')
  row.update(status=status,response=copy.deepcopy(res),state_after=copy.deepcopy(self.store));self.requests.append(row)
  return status,res
class Handler(BaseHTTPRequestHandler):
 def log_message(self,*args):pass
 def do_GET(self):self.serve()
 def do_POST(self):self.serve()
 def do_PATCH(self):self.serve()
 def serve(self):
  raw=self.rfile.read(int(self.headers.get('Content-Length','0')));body=json.loads(raw) if raw else None
  code,data=self.server.fixture.handle(self.command,self.path,body,self.headers)
  content=json.dumps(data,ensure_ascii=False).encode();self.send_response(code);self.send_header('Content-Type','application/json');self.send_header('Content-Length',str(len(content)));self.end_headers();self.wfile.write(content)
def run_process(path,stage,argv,env,fixture):
 p=path/stage;p.mkdir();start=time.monotonic();before=copy.deepcopy(fixture.store);r=subprocess.run(argv,env=env,cwd=path,capture_output=True,timeout=20)
 (p/'stdout.txt').write_bytes(r.stdout);(p/'stderr.txt').write_bytes(r.stderr)
 selected={k:v for k,v in env.items() if k.startswith(('TESTSPRITE_','LAB_')) or k in ['NODE_OPTIONS','CI','NO_COLOR']}
 dump(p/'command.json',dict(argv=argv,cwd=str(path),env=selected,exit_code=r.returncode,elapsed=time.monotonic()-start))
 dump(p/'state-before.json',before);dump(p/'state-after.json',fixture.store)
 return dict(exit=r.returncode,stdout=r.stdout.decode(),stderr=r.stderr.decode(),argv=argv)
def scenario(cfg,out):
 p=out/cfg['id'];p.mkdir(parents=True);dump(p/'fixture.json',cfg);home=p/'synthetic-home';home.mkdir()
 f=Fixture(cfg,p);server=ThreadingHTTPServer(('127.0.0.1',0),Handler);server.fixture=f;thread=threading.Thread(target=server.serve_forever,daemon=True);thread.start();origin=f'http://127.0.0.1:{server.server_port}'
 env={k:v for k,v in os.environ.items() if k not in ['NODE_OPTIONS','HTTP_PROXY','HTTPS_PROXY','ALL_PROXY','http_proxy','https_proxy','all_proxy'] and not k.startswith(('TESTSPRITE_','LAB_'))}
 env.update(TESTSPRITE_API_KEY='sk-user-lab-environment-recovery',TESTSPRITE_API_URL=origin,TESTSPRITE_NO_TELEMETRY='1',TESTSPRITE_NO_UPDATE_NOTIFIER='1',CI='1',NO_COLOR='1',LAB_ORIGIN=origin,LAB_FIXTURE_HOME=str(home),LAB_NODE=str(NODE),LAB_CLI=str(CLI),LAB_ARGV_FILE=str(p/'shell-argv.bin'),NODE_OPTIONS='--require='+str(GUARD))
 base=[str(NODE),str(CLI),'--output',cfg.get('mode','json')]
 flags=[]
 if cfg['operation']=='create':args=['project','env','create','proj_lab','--name',cfg['name'],'--url','https://app.example.test'];flags+=['--var','REGION=west']
 else:
  args=['project','env','update','proj_lab','old','--rename',cfg['name']]
  if cfg.get('clear',True):flags+=['--clear-credentials']
  if cfg.get('var'):flags+=['--var','REGION=west']
 try:
  initial=run_process(p,'initial',base+args+flags,env,f)
  assert initial['exit']==7,(cfg['id'],initial)
  assert cfg['name'] in f.store,'initial write not acknowledged'
  if cfg.get('mode','json')=='json':
   err=json.JSONDecoder().raw_decode(initial['stderr'][initial['stderr'].index('{'):])[0]['error'];assert err['code']=='UNSUPPORTED';next_action=err['nextAction']
  else:
   line=next(l for l in initial['stderr'].splitlines() if 'After the server is upgraded, run:' in l);next_action=line
  command=next_action.split('After the server is upgraded, run: ',1)[1].strip()
  command=command.replace('KEY=VALUE','REGION=west')
  # Only this deliberately inert corpus is permitted for shell consumption.
  assert not any(c in command for c in [';','|','&','<','>','`','$','\n','\r']),command
  (p/'generated-command.txt').write_text(command+'\n');(p/'next-action.txt').write_text(next_action+'\n')
  f.phase='suggested';f.counter=0
  wrapper='testsprite() { printf "%s\\0" "$@" > "$LAB_ARGV_FILE"; "$LAB_NODE" "$LAB_CLI" --output json "$@"; }\n'
  shell=run_process(p,'suggested',['bash','--noprofile','--norc','-c',wrapper+command],env,f)
  argv_file=p/'shell-argv.bin';shell_args=argv_file.read_bytes().decode().split('\0')[:-1] if argv_file.exists() else None
  dump(p/'shell-argv.json',shell_args)
  expected=['project','env','update','proj_lab',cfg['name']]
  if 'clear' in cfg['ignore']:expected+=['--clear-credentials']
  if 'var' in cfg['ignore']:expected+=['--var','REGION=west']
  literal=shell_args==expected
  state_suggested=copy.deepcopy(f.store)
  f.phase='corrected';quoted='testsprite '+' '.join(shlex.quote(x) for x in expected)
  corrected=run_process(p,'corrected',['bash','--noprofile','--norc','-c',wrapper+quoted],env,f)
  assert corrected['exit']==0,(cfg['id'],corrected)
  f.phase='readback';read=run_process(p,'readback',[str(NODE),str(CLI),'--output','json','project','env','get','proj_lab',cfg['name']],env,f)
  assert read['exit']==0
  actual=json.loads(read['stdout'])['environment'];assert actual['id']=='env-target'
  if 'clear' in cfg['ignore']:assert actual['hasCredentials'] is False and actual['username'] is None
  if 'var' in cfg['ignore']:assert actual['variables']['REGION']=='west'
  writes=[r for r in f.requests if r['phase']=='initial' and r['method']!='GET']
  if cfg.get('retry'):
   assert len(writes)==2 and writes[0]['status']==503 and writes[1]['status'] in [200,201]
   assert writes[0]['body']==writes[1]['body'] and writes[0]['headers']['idempotency-key']==writes[1]['headers']['idempotency-key']
  sibling_changed=bool(cfg.get('sibling') and state_suggested[cfg['sibling']]!=f.initial[cfg['sibling']])
  result=dict(id=cfg['id'],name=cfg['name'],expected_args=expected,actual_args=shell_args,identity_preserved=literal,initial_exit=initial['exit'],suggested_exit=shell['exit'],corrected_exit=corrected['exit'],readback_exit=read['exit'],sibling_changed=sibling_changed,requests=len(f.requests),fixture_assertions='passed',contract_passed=literal and shell['exit']==0 and not sibling_changed)
  dump(p/'result.json',result);return result
 finally:
  dump(p/'requests.json',f.requests);dump(p/'state-final.json',f.store);server.shutdown();server.server_close();thread.join()
def main():
 global NODE,CLI,GUARD
 parser=argparse.ArgumentParser();parser.add_argument('--out',default=str(HERE/'results'));parser.add_argument('--minimal',action='store_true');parser.add_argument('--node',default=str(NODE));parser.add_argument('--cli',default=str(CLI));a=parser.parse_args();NODE=pathlib.Path(a.node).resolve();CLI=pathlib.Path(a.cli).resolve();out=pathlib.Path(a.out).resolve();out.mkdir(parents=True,exist_ok=True);GUARD=out/'guard.cjs';GUARD.write_text(GUARD_TEXT)
 cases=[dict(id='plain-control',name='staging',operation='update',ignore=['clear']),dict(id='space-rename',name='QA West',sibling='QA',operation='update',ignore=['clear']),dict(id='apostrophe-rename',name="Owner's QA",operation='update',ignore=['clear'])]
 if not a.minimal:
  cases += [dict(id='space-sibling',name='QA West',sibling='QA',operation='update',ignore=['clear']),dict(id='unicode-control',name='预发布',operation='update',ignore=['clear']),dict(id='double-space',name='QA  West',operation='update',ignore=['clear']),dict(id='tab-name',name='QA\tWest',operation='update',ignore=['clear']),dict(id='double-quote-sibling',name='QA"West"',sibling='QAWest',operation='update',ignore=['clear']),dict(id='backslash-sibling',name='QA\\West',sibling='QAWest',operation='update',ignore=['clear']),dict(id='create-space-var',name='New Stage',operation='create',ignore=['var']),dict(id='create-plain-control',name='new-stage',operation='create',ignore=['var']),dict(id='text-space',name='QA East',mode='text',operation='update',ignore=['clear']),dict(id='combined-retry-partial',name='QA West',operation='update',ignore=['var'],clear=True,var=True,retry=True),dict(id='both-effects-apostrophe',name="Owner's Stage",operation='update',ignore=['clear','var'],clear=True,var=True)]
  rng=random.Random(2026101803)
  for i in range(8):
   name=rng.choice(['QA','测试','Stage'])+rng.choice([' ',"'",'\\','"','-','  '])+rng.choice(['North','West','East'])
   cases.append(dict(id=f'seed-{i}',name=name,operation='update',ignore=['clear']))
 dump(out/'scenario-inputs.json',cases);res=[]
 for c in cases:
  r=scenario(c,out);res.append(r);dump(out/'summary.json',res);print(json.dumps(r,ensure_ascii=False),flush=True)
 dump(out/'execution.json',dict(cases=len(res),contract_failures=sum(not r['contract_passed'] for r in res),fixture_failures=0,node=subprocess.check_output([str(NODE),'--version'],text=True).strip(),sha=subprocess.check_output(['git','-C',str(CLI.parent.parent),'rev-parse','HEAD'],text=True).strip(),cli_version=json.loads((CLI.parent.parent/'package.json').read_text())['version']))
 # Reproducer exits red if the independently expected shell identity contract fails.
 return 1 if any(not r['contract_passed'] for r in res) else 0
if __name__=='__main__':raise SystemExit(main())

The full local matrix covered 22 partial-write recovery scenarios (including initial503 replay, rename+clear with ignored variable, create+ignored variable, text/JSON and eight seeded names), plus a fully applied normal update control. Nineteen commands lost target identity or failed shell parsing; four changed a sibling. Independent raw-evidence audits passed. Linux/Bash only; no PowerShell/Windows or hosted occurrence claim, and no shell-injection payloads were used.

Root cause: src/commands/project-env.ts confirmRequestedEffects directly interpolates created.name/updated.name into nextAction. Existing tests assert the error includes flags but never consume the command. The separate quoteEnvironmentName helper in src/lib/environment-tunnel.ts already handles POSIX literal names for other recovery hints. Proposed scope: consistently quote identifier arguments in this builder, with a native command-consumer regression proving exact destination and unchanged siblings. No dependency or backend behavior change is needed.

Duplicate review: refreshed the 532 OPEN+CLOSED issue/PR snapshot and re-read current comments on #385/#386 and #331/#76, retaining the earlier #25 thread/review comparison. #385/#386 validate blank project IDs; #331/#76 concern project defaults; #25 was superseded by #22 for parser error JSON. They do not fix recovery-command argument identity. Prior local variable-map tests covered response confirmation, and a separate result hint already uses quoting. No separate report per name spelling or per symptom is proposed.

I intend to implement this after maintainer acceptance and actual bot assignment to @walter-w-white. Please contact @walter-w-white on this GitHub thread; no Discord identity is provided. Eligibility and rewards remain the maintainers' decision.

Revalidated on the exact commit above on 2026-10-20. The complete native CLI matrix still reproduces the reported behavior; normal controls and independent fixture checks pass. Refreshed duplicate review covers 552 open and closed Issue/PR records plus current related comments.

I intend to implement this fix after maintainer acceptance and assignment.

Please contact @walter-w-white on this GitHub issue or PR; no Discord identity is provided.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions