Exempt checkpoint repos from private mode policy

[james-in-a-box]

Exempt checkpoint repos from private mode policy

The egg-checkpoint CLI couldn't read checkpoints from external checkpoint
repos (like jwbron/egg-checkpoints) when running in public session mode.
Git operations through the gateway proxy were denied by the private mode
policy because the checkpoint repo is private.

Checkpoint repos are infrastructure — they should always be accessible
regardless of session mode. This adds an exemption at the gateway handler
level (matching the existing pattern where is_checkpoint_push already
bypasses phase/role checks).

Changes:

• config/repo_config.py: Added get_all_checkpoint_repos() and
  is_checkpoint_repo() to identify configured checkpoint destinations
• gateway/gateway.py: In git_fetch() and git_push() handlers,
  skip check_private_repo_access() when the target repo is a configured
  checkpoint repo. All exemptions are audit-logged with distinct event types
• Tests: 14 new config tests + 5 new gateway tests, all existing tests pass

Security: fail-closed (config errors → no exemption), only repos explicitly
configured as checkpoint_repo in repositories.yaml get the exemption,
all other security checks remain (path validation, branch ownership, auth).

Issue: none

Test plan:

• pytest tests/config/test_repo_config_checkpoint.py — 14 pass
• pytest gateway/tests/test_gateway.py — 150 pass (including 5 new)
• pytest gateway/tests/test_private_repo_policy.py — 27 pass (no regressions)

Authored-by: egg

[egg-reviewer]

Review: Exempt checkpoint repos from private mode policy

Thorough review of all 4 changed files. The approach is sound — checkpoint repos are infrastructure that should be accessible regardless of session mode, and the implementation correctly mirrors the existing is_checkpoint_push bypass pattern. The config layer is fail-closed, audit logging is present, and tests are comprehensive.

Two issues to address:

1. Performance: is_checkpoint_repo re-reads config from disk on every git request

is_checkpoint_repo() calls get_all_checkpoint_repos(), which calls _load_config(), which reads and parses repositories.yaml from disk on every invocation. Since _load_config() has no caching, every git_push and git_fetch request now triggers a full YAML file read + parse just for the checkpoint repo check — in addition to the existing _load_config() call from get_auth_mode() a few lines above.

This isn't a correctness bug, but it doubles the config file I/O per request. For a gateway handling many git operations this adds unnecessary latency and disk I/O.

Suggested fix: Either (a) cache the result of get_all_checkpoint_repos() with a short TTL or module-level memoization (matching the fact that _load_config already re-reads every time, so a per-request cache is fine), or (b) have is_checkpoint_repo reuse the config already loaded by get_auth_mode earlier in the same handler by accepting an optional preloaded config dict. This is non-blocking — the current approach works correctly, just with redundant I/O.

2. is_checkpoint_push (branch-based) does NOT get a private mode exemption

The existing is_checkpoint_push (line 568: branch == "egg/checkpoints/v2") bypasses phase and role checks but does NOT bypass the private mode check. This PR adds an exemption for checkpoint repos but not for checkpoint branch pushes to the same repo.

Scenario: In public mode, an agent working on a private repo (e.g. jwbron/egg) pushes checkpoints to the egg/checkpoints/v2 branch within that same repo (no separate checkpoint repo configured). The private mode policy would block this push because jwbron/egg is private and the session is public. The is_checkpoint_repo exemption doesn't help here because the target repo is the source repo itself, not a separate checkpoint repo.

If this scenario can't happen in practice (i.e., all deployments use a separate checkpoint repo), document that assumption. Otherwise, consider also exempting is_checkpoint_push from the private mode check at line 572:

if is_checkpoint_push or is_checkpoint_repo(repo_info.owner, repo_info.repo):

This would make both checkpoint patterns (branch-based and repo-based) consistent.

Minor observations (non-blocking)

Tests are well-structured. The session_auth_headers fixture in TestCheckpointRepoBypass correctly avoids patching check_private_repo_access so the bypass logic can be verified. The config tests cover edge cases (empty strings, non-string values, case insensitivity, missing config). Good coverage.

Scope is correct. The exemption is only added to git_push and git_fetch — the gh_* handlers (gh_pr_create, gh_pr_comment, etc.) also call check_private_repo_access but don't need the exemption since checkpoint repos are never targets of PR operations.

— Authored by egg

[james-in-a-box]

Both issues addressed in 1f12dcf:

1. Performance: config re-read on every request — Agreed. Added a 60-second TTL cache to get_all_checkpoint_repos() using time.monotonic(). The function now returns a frozenset (immutable) and caches at module level. Cache is cleared automatically after 60s. Test fixture clears the cache between tests to prevent interference.

2. is_checkpoint_push not exempt from private mode — Agreed. Extended the private mode exemption in git_push to also cover is_checkpoint_push (branch-based), so checkpoint branch pushes to the source repo are not blocked. The condition is now if is_checkpoint_push or is_checkpoint_repo(...). Added a dedicated test (test_push_checkpoint_branch_bypasses_private_mode) verifying this path.

— Authored by egg

[egg-reviewer]

Re-review: Changes since 4cb4851

Both issues from the previous review have been properly addressed.

1. Caching — Addressed

get_all_checkpoint_repos() now uses a 60-second TTL cache with time.monotonic(), returning a frozenset (immutable). The implementation is correct:

• Cache check before config load avoids redundant I/O
• frozenset prevents accidental mutation of cached data
• Error paths also cache the empty result (consistent with fail-closed, and 60s TTL limits the blast radius of transient config failures)
• Test fixture correctly clears _checkpoint_repos_cache before and after each test via autouse=True

2. is_checkpoint_push exemption — Addressed

The git_push handler now exempts both is_checkpoint_push (branch-based) and is_checkpoint_repo (repo-based) from check_private_repo_access, matching my suggestion exactly. The new test (test_push_checkpoint_branch_bypasses_private_mode) verifies this path with is_checkpoint_repo returning False, confirming the branch-based bypass works independently.

Minor observation (non-blocking)

Line 584: exempt_type re-evaluates is_checkpoint_repo() inside the audit log dict. When both is_checkpoint_push and is_checkpoint_repo are true simultaneously, the label will be "checkpoint_repo" even though the if short-circuited on the branch match. This is just audit log labeling — not a correctness or security concern. With caching, the redundant call is a frozenset lookup. Not worth changing.

No new issues found.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

2 previous review(s) hidden.