Skip to content

fix(ai-chat): gate repo tools by git_repositories:read permission - #58

Closed
bherila wants to merge 10 commits into
mainfrom
codex/propose-fix-for-ai-chat-vulnerability
Closed

bherila wants to merge 10 commits into
mainfrom
codex/propose-fix-for-ai-chat-vulnerability

Conversation

@bherila

@bherila bherila commented Aug 15, 2026

Copy link
Copy Markdown
Owner

Motivation

  • Close an authorization gap where AI chat could advertise and execute Git-backed repository-exploration tools (e.g. read_repo_file) for callers holding only ProjectsRead/ProjectsWrite, allowing repository contents to be fetched with stored provider credentials.
  • Prevent low-privilege callers from prompting the model to exfiltrate repo files via the agentic tool loop by ensuring tool visibility and execution are bounded by repository-read permissions.

Description

  • Require the git_repositories:read permission before advertising repository exploration tools by checking auth.has_permission(&Permission::GitRepositoriesRead) before merging the __repo_tools__ sentinel provider into the per-turn tools list in ConversationService::send_message (crates/temps-ai-chat/src/service.rs).
  • Add a defense-in-depth execution-time guard that rejects or returns a permission error for unadvertised repo tool calls (read_repo_file, list_repo_dir, list_repo_branches, list_repo_tags) when the caller lacks GitRepositoriesRead in the agentic try_tool_loop routing logic (crates/temps-ai-chat/src/service.rs).
  • Import Permission into the chat service and conditionally set the repo_tools provider handle so downstream execution respects the caller's permission scope.
  • Add an Unreleased entry to CHANGELOG.md describing the authorization fix as required by repository conventions.

Testing

  • Ran cargo fmt --check and formatting check passed.
  • Ran cargo check -p temps-ai-chat --lib and the crate type-checked successfully.
  • Ran cargo test -p temps-ai-chat --lib and all unit tests passed (73 passed, 0 failed).

Codex Task

bherila and others added 10 commits August 15, 2026 07:45
Runs cargo check + lib tests on the tempsdev1 self-hosted runner for
pushes to main/feature/**/fix/** and manual dispatch. Scoped to push
and workflow_dispatch only (never pull_request/pull_request_target) so
fork PRs can never execute on the self-hosted host. Uses a persistent,
isolated CARGO_TARGET_DIR outside the runner work dir so the build
cache survives checkout cleans and is reused across runs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A warm CARGO_TARGET_DIR caches the example plugins' build-script fingerprint,
so after actions/checkout git-cleans the source tree the script is skipped and
include_dir!(".../web/dist") panics on the missing dir. Recreate the debug
placeholder assets up front so the check is independent of build-script cache.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
temps-cli embeds crates/temps-cli/dist via include_dir!; like the example
plugins its build.rs placeholder is skipped on a warm cache, so generalize the
prep step to cover every include_dir! dist target, not just the plugins.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…vy crates

cargo test --lib --workspace pulled in temps-domains' real-Pebble ACME tests,
which need a challenge-responder network this box can't provide. Match the
unit-a/unit-b crate list from rust-tests.yml instead (Docker+networking-heavy
crates run in hosted's dedicated integration jobs, not here).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Its local_provider tests launch a real bundled Chromium against live
example.com/github.com. The hosted ubuntu-latest image ships
google-chrome system-wide so headless_chrome reuses it; this self-hosted
box has no system browser, so headless_chrome auto-downloads one and the
parallel tests race on the binary (Text file busy) and debug ports (no
available ports between 8000 and 9000). It is a live-network browser
integration test, not a unit test, and hosted CI already covers it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The tempsdev1 EC2 runner it targeted has been decommissioned (no
self-hosted runners are registered on this repo anymore), so every
push to main/feature/**/fix/** was queuing a job with nothing to pick
it up. Hosted CI (rust-tests.yml) already covers push and PR on
ubuntu-latest.
cargo fetch --locked warms the registry/deps cache synchronously, then
a full `cargo build --workspace --all-targets` kicks off in the
background (nohup, disowned) so opening the codespace isn't blocked
on it -- by the time you start editing, the workspace is already
compiled or close to it.
Full workspace builds/tests can pin every core and several GB of RAM
for minutes on this 51-crate workspace. Check load/memory before
running them locally, and prefer pushing to let hosted CI validate
when the box is already under load.
…ents

A single load check right before launching a build isn't enough when
multiple worktrees or subagents share a box -- builds kicked off
seconds apart can each pass the check and then land together, and a
few concurrent full/release builds is enough to thrash a shared host
(swap, OOM, runaway load). Re-check right before the build starts and
default to assuming something else may already be building.
@bherila
bherila force-pushed the codex/propose-fix-for-ai-chat-vulnerability branch from 9969e6a to 6cb257b Compare August 15, 2026 07:45
@github-actions

Copy link
Copy Markdown

📓 Changelog preview

This is what your commits will add to the generated CHANGELOG.md at release time (via git-cliff). Do not edit CHANGELOG.md by hand — it is generated from your Conventional Commit messages.

## [Unreleased]

### Added

- **devcontainer:** Prefetch cargo deps and background-build on create

### CI

- **self-hosted:** Add build/test workflow on push to bherila fork
- **self-hosted:** Recreate example-plugin web/dist before cargo check
- **self-hosted:** Also recreate temps-cli/dist placeholder
- **self-hosted:** Run hosted unit-test crate set, skip integration-heavy crates
- **self-hosted:** Exclude temps-screenshots live-browser tests

### Documentation

- Add local resource-awareness guidance to CLAUDE.md
- Emphasize local resource awareness with concurrent worktrees/agents

### Fixed

- **ai-chat:** Gate repo tools by git read permission

### Miscellaneous

- **ci:** Remove dead self-hosted-ci workflow

@bherila

bherila commented Oct 11, 2026

Copy link
Copy Markdown
Owner Author

This proposal was superseded by fork PR #54, whose functional changes were included in merged upstream gotempsh#677. Upstream gotempsh#676 explicitly records this supersession.

Fork main has now been fast-forwarded to upstream main at 7250ab59b38d3138014ade7f5d6391c856eaa4f7. Closing this obsolete fork PR as part of the upstream synchronization; its branch history is retained.

@bherila bherila closed this Oct 11, 2026
@bherila
bherila deleted the codex/propose-fix-for-ai-chat-vulnerability branch October 11, 2026 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant