Skip to content

fix: release workflow version parsing#60

Merged
haofeif merged 1 commit into
mainfrom
fix/release-workflow-version
Feb 2, 2026
Merged

fix: release workflow version parsing#60
haofeif merged 1 commit into
mainfrom
fix/release-workflow-version

Conversation

@haofeif

@haofeif haofeif commented Feb 2, 2026

Copy link
Copy Markdown
Contributor

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@haofeif haofeif merged commit 31f0f11 into main Feb 2, 2026
2 checks passed
@haofeif haofeif deleted the fix/release-workflow-version branch February 2, 2026 12:45
haofeif added a commit that referenced this pull request May 2, 2026
… rebuild

Previous hardening commit wrote sanitisers that CodeQL didn't recognise as
taint-kills because the checks sat *after* Path() construction and
requests.get() received the caller-controlled source string.

- _SAFE_URL_PATH_RE validates parsed.path *before* the fetch; the URL handed
  to requests.get() is rebuilt as f"https://{safe_host}{parsed.path}" where
  safe_host is pulled from the allowlist literal. Reject query/fragment/
  userinfo which have no place in a static .md fetch.
- _FILE_PATH_RE validates the source string *before* Path(source).resolve()
  and Path(source).exists() — the fullmatch regex sits on the data-flow
  edge into each Path() sink.
- Add a CodeQL job to ci.yml (python + js/ts, security-and-quality suite)
  so future SSRF/path-injection regressions fail CI instead of trickling
  in as post-merge alerts.
- Add scripts/security-scan.sh for local trivy + codeql runs mirroring CI.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
haofeif added a commit that referenced this pull request May 2, 2026
#226)

* fix(install): harden agent-profile install against SSRF and path injection

Closes CodeQL py/full-ssrf and py/path-injection alerts on the install
path added in #166.

- URL downloads restricted to https:// with a host allowlist
  (github.com, raw.githubusercontent.com by default; extend via
  CAO_PROFILE_ALLOWED_HOSTS env var).
- Redirects disabled; explicit is_redirect rejection.
- (5, 30)s connect/read timeout to bound worker exposure.
- Filename / profile-name regex [A-Za-z0-9_-]{1,64} on every sink.
- New allow_file_source kwarg on install_agent(); HTTP API and
  (transitively) ops-MCP install_profile pass False so remote callers
  cannot coerce the server into reading arbitrary local files.

CLI behaviour unchanged.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(install): close CodeQL #60/#61/#62 with pre-Path validation + URL rebuild

Previous hardening commit wrote sanitisers that CodeQL didn't recognise as
taint-kills because the checks sat *after* Path() construction and
requests.get() received the caller-controlled source string.

- _SAFE_URL_PATH_RE validates parsed.path *before* the fetch; the URL handed
  to requests.get() is rebuilt as f"https://{safe_host}{parsed.path}" where
  safe_host is pulled from the allowlist literal. Reject query/fragment/
  userinfo which have no place in a static .md fetch.
- _FILE_PATH_RE validates the source string *before* Path(source).resolve()
  and Path(source).exists() — the fullmatch regex sits on the data-flow
  edge into each Path() sink.
- Add a CodeQL job to ci.yml (python + js/ts, security-and-quality suite)
  so future SSRF/path-injection regressions fail CI instead of trickling
  in as post-merge alerts.
- Add scripts/security-scan.sh for local trivy + codeql runs mirroring CI.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(install): close CodeQL #63 + drop conflicting workflow CodeQL job

Two follow-ups to the previous hardening commit:

1. Alert #63 (py/path-injection, install_service.py:235)
   The `elif allow_file_source and _FILE_PATH_RE.fullmatch(source)
   and Path(source).exists()` guard still tripped the scanner because
   CodeQL doesn't thread the regex sanitiser through the compound
   boolean into the Path() call. Fix: dispatch by pure string suffix
   (`source.endswith(".md")`) — no Path() in install_agent() at all.
   All path construction happens inside _download_agent(), which already
   regex-validates before `.resolve()`.

2. The workflow-based `codeql` job conflicted with the repo's existing
   default-setup CodeQL ("CodeQL analyses from advanced configurations
   cannot be processed when the default setup is enabled"). Dropped the
   job and left a comment in ci.yml explaining why; default setup already
   runs the Analyze (python) / Analyze (js-ts) checks on every PR.

3. SECURITY.md — documented CodeQL coverage, the host allowlist behaviour
   (`CAO_PROFILE_ALLOWED_HOSTS`), and the scripts/security-scan.sh wrapper.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(install): reject `..` segments in _FILE_PATH_RE (closes CodeQL #61)

The previous regex used a character class that included `.` and `/`, so
`../../etc/passwd.md` matched and passed into `Path(source).resolve()`.
CodeQL was right to flag it — the sanitiser was weaker than advertised.

- Add a leading negative lookahead `(?!.*\.\.)` to the file-path regex so
  any `..` anywhere in the string rejects the source before Path() is
  constructed. Legitimate `./foo.md`, `/abs/foo.md`, `~/foo.md`, and
  `sub/dir/foo.md` all still work.
- Two new regression tests cover `../../etc/passwd.md` and embedded
  `/tmp/foo/../etc/passwd.md` traversal shapes.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(install): move file-path handling out of install_service (closes CodeQL #64)

Earlier rounds kept `Path(user_input)` reachable inside `install_service` behind
a regex sanitiser. Every regex shape that still admitted a legitimate CLI path
like `./foo.md` also admitted `../../etc/passwd.md` without an unacceptable
normalise+prefix-check — so CodeQL kept correctly flagging the `.resolve()`
sink.

Structural fix: the shared service doesn't need a file-path branch at all.

- `install_service.install_agent()` now accepts only a bare profile name
  (`_PROFILE_NAME_RE`) or an https:// URL on the host allowlist.
- `cli/commands/install.py` grows a `_copy_local_profile_to_store()` helper
  that does the file reading, stem validation, and copy-into-store itself,
  then calls the service with the bare validated stem.
- `api/main.py` drops the `allow_file_source=False` kwarg — the parameter
  is gone; the service refuses filesystem paths for every caller.
- Tests: remove the file-path branches from the service suite, move that
  coverage to the CLI suite (`TestCopyLocalProfileToStore` + integration
  tests on file-source `cao install` invocations).

Full test suite (`test/ --ignore=test/e2e -m "not integration"`): 1581/1581
pass. End-to-end smoke of `cao install /tmp/smoke-agent.md --provider kiro_cli`
verified.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* style: black reformat test_install.py (extra blank line)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant