diff --git a/TODO.md b/TODO.md index 3150ac0..f233d2d 100644 --- a/TODO.md +++ b/TODO.md @@ -7,7 +7,7 @@ - [ ] Check semantic retrieval ownership and links before copying or deleting its directory. An existing owned runtime's source files are overwritten without comparing recorded hashes; a replaced target symlink is followed on enable, and disabling the last client removes foreign files inside the runtime. (`scripts/lib/semantic-retrieval.py:82`, `scripts/lib/semantic-retrieval.py:100`) - [ ] Preserve pre-existing MCPorter server entries. `ensure_mcporter()` adds a server and claims ownership without checking existing configuration; if the add succeeds for an existing name, a later removal deletes that user's entry. (`scripts/lib/managed-extensions.py:395`, `scripts/lib/managed-extensions.py:263`) - [ ] Preserve pre-existing global CLI packages. `ensure_cli()` and `ensure_qmd()` install and claim npm packages without checking whether they were already installed; later removal can uninstall a user's package. (`scripts/lib/managed-extensions.py:342`, `scripts/lib/managed-extensions.py:378`) -- [ ] Fetch skill archives by an immutable commit and record that same commit. The archive uses GitHub `HEAD`, while `resolvedRevision` comes from a separate later request, so the recorded revision may not identify the installed payload. (`scripts/lib/managed-extensions.py:94`, `scripts/lib/managed-extensions.py:140`, `scripts/lib/managed-extensions.py:409`) +- [x] Fetch skill archives by an immutable commit and record that same commit. The archive uses GitHub `HEAD`, while `resolvedRevision` comes from a separate later request, so the recorded revision may not identify the installed payload. (`scripts/lib/managed-extensions.py:94`, `scripts/lib/managed-extensions.py:140`, `scripts/lib/managed-extensions.py:409`) - [ ] Protect the semantic retrieval daemon's IPC protocol. It uses a hardcoded authentication key and `multiprocessing.connection.recv()`, which deserializes messages; a process that can reach its socket or pipe can submit arbitrary roots and potentially crafted objects. Use a private per-install credential and a non-pickle wire format, or restrict the endpoint to the current user. (`shared/retrieval/server.py:47`, `shared/retrieval/server.py:306`, `shared/retrieval/server.py:314`) - [x] Write semantic retrieval ownership state atomically. `save_state()` replaces the JSON with a direct write, unlike the extension and selection ledgers; interruption can leave it unreadable and block later reconciliation. (`scripts/lib/semantic-retrieval.py:63`) diff --git a/scripts/lib/managed-extensions.py b/scripts/lib/managed-extensions.py index df2f3fa..e797d23 100644 --- a/scripts/lib/managed-extensions.py +++ b/scripts/lib/managed-extensions.py @@ -98,7 +98,10 @@ def fetch_skill(entry): skill = entry['codex_skill'] if not re.fullmatch(r'[A-Za-z0-9_-]+', skill): raise ValueError('Skill name is invalid.') - request = urllib.request.Request(f'https://codeload.github.com/{source}/zip/HEAD', headers={'User-Agent': 'MyAiConfiguration'}) + revision = resolve_revision(source) + if revision is None: + raise ValueError('Cannot resolve an immutable skill revision.') + request = urllib.request.Request(f'https://codeload.github.com/{source}/zip/{revision}', headers={'User-Agent': 'MyAiConfiguration'}) with urllib.request.urlopen(request, timeout=60) as response: if not response.url.startswith('https://codeload.github.com/'): raise ValueError('Unexpected skill download redirect.') @@ -134,7 +137,7 @@ def fetch_skill(entry): if relative in files: raise ValueError('Skill archive contains duplicate paths.') files[relative] = (archive.read(item), bool(item.external_attr >> 16 & 0o111)) - return skill, files + return skill, files, revision def resolve_revision(source): @@ -143,7 +146,7 @@ def resolve_revision(source): with urllib.request.urlopen(request, timeout=10) as response: payload = json.loads(response.read(1024 * 1024)) revision = payload.get('sha') if isinstance(payload, dict) else None - return revision if isinstance(revision, str) and re.fullmatch(r'[0-9a-fA-F]{7,64}', revision) else None + return revision if isinstance(revision, str) and re.fullmatch(r'[0-9a-fA-F]{40}', revision) else None except (OSError, ValueError, json.JSONDecodeError): return None @@ -428,7 +431,7 @@ class Manager: return fetched = fetch_skill(entry) _, files = fetched[:2] - revision = fetched[2] if len(fetched) > 2 else resolve_revision(entry['codex_source']) + revision = fetched[2] if len(fetched) > 2 else None if record is None: record = dict(client='codex', name=entry['name'], kind='skill', directory=directory, source=entry['codex_source'], files={}, complete=False) self.state['resources'].append(record)