Skip to content

fix: harden remote publication safety - #153

Merged
delete-cloud merged 2 commits into
mainfrom
fix/remote-publication-safety
May 14, 2026
Merged

delete-cloud merged 2 commits into
mainfrom
fix/remote-publication-safety

Conversation

@delete-cloud

@delete-cloud delete-cloud commented May 13, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • make workspace diff/patch use a temporary index with git add -A so review output includes untracked files without mutating the real index
  • validate publication branch remote URL and Git token before git add/commit/push, preserving inspectable workspace state on config failures
  • require Git source clone URLs to pass a configured remote_sources.git.allowed_hosts allowlist before git clone
  • strip query and fragment from publication remote_url responses

Verification

  • uv run pytest tests/coding_agent/environment/test_docker_workspace_provider.py -q
  • uv run pytest tests/ui/test_http_server.py tests/ui/test_http_server_workspace_transfer.py tests/cli/test_remote_client.py -q
  • uv run pytest tests/ui/test_http_server.py tests/ui/test_http_server_workspace_transfer.py tests/cli/test_remote_client.py tests/coding_agent/environment/test_docker_workspace_provider.py -q -k "git_workspace or session_result or workspace_diff or workspace_patch or publish or remote_run or create_session_provisions_git_workspace_source"
  • uv run basedpyright --level error src/coding_agent/environment/docker_workspace_provider.py src/coding_agent/ui/http_server.py
  • uv run ruff format --check src/coding_agent/environment/docker_workspace_provider.py src/coding_agent/ui/http_server.py tests/coding_agent/environment/test_docker_workspace_provider.py tests/ui/test_http_server.py
  • git diff --check

Summary by CodeRabbit

  • New Features

    • Added configuration support for specifying allowed Git source hosts and injecting it into cloud workspace config.
  • Improvements

    • Git source URLs are validated against the allowlist and required to use HTTPS before cloning.
    • Diff/patch generation uses an isolated temp index for more reliable reporting.
    • Publication responses redact sensitive URL components (query/fragment) and avoid leaking tokens.
  • Tests

    • Added tests covering pre-clone validation, diff/patch behavior, and non-mutating rejection scenarios.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 13, 2026 •

Copy link
Copy Markdown
Contributor

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR hardens Git-backed workspace provisioning and publishing by introducing a remote source allowlist configuration, validating Git clone URLs before execution, refactoring diff/patch to use isolated temporary Git indexes, and improving publication URL redaction and environment wiring.

Changes

Remote sources validation and Git workspace control flow

Layer / File(s) Summary
Configuration loading for remote sources
src/coding_agent/ui/http_server.py, tests/ui/test_http_server.py, docs/remote-sandbox-production.md
_load_remote_sources_config() reads the optional [remote_sources] section from agent.toml and injects it into cloud_workspace_config when present. Test verifies the allowlist is captured and passed to provisioning. Production config example documents the new [remote_sources.git].allowed_hosts section.
Git source URL validation before clone
src/coding_agent/environment/docker_workspace_provider.py, tests/coding_agent/environment/test_docker_workspace_provider.py
_validate_git_source_remote_url() and _remote_source_string_list() enforce HTTPS-only transport, reject embedded credentials/query/fragment, and verify the remote hostname against remote_sources.git.allowed_hosts before any git clone invocation. Cloning now passes config to enable validation. Tests assert ValueError is raised and no git operations occur when the host is unallowlisted or transport is unsafe (e.g., file://).
Git diff/patch with isolated temporary indexes
src/coding_agent/environment/docker_workspace_provider.py, tests/coding_agent/environment/test_docker_workspace_provider.py
_git_workspace_diff() and _git_workspace_patch() now use GIT_INDEX_FILE to operate on isolated temporary Git indexes, running git read-tree HEAD or copying .git/index, staging changes with git add -A, and executing git diff --cached for both name-status and numstat. Temp indexes are always cleaned up in finally blocks. _parse_git_name_status() is adjusted to handle malformed output with missing tab-separated fields. Tests verify untracked files are included in both diff and patch output.
Git publication URL redaction and push environment wiring
src/coding_agent/environment/docker_workspace_provider.py, tests/coding_agent/environment/test_docker_workspace_provider.py, docs/remote-sandbox-production.md
Publication flow is reordered to fetch and validate remote.origin.url earlier, derive push environment via _git_publication_push_env(), and compute pushed_ref after validation. _redact_git_remote_url() now removes query/fragment in addition to credentials/host. Tests mock remote.origin.url with token query parameters and assert no git mutations (add, commit) occur when validation fails (unsafe URL, missing allowlist, missing token env var). Documentation clarifies that allowed_hosts is checked before token injection and git push.

Sequence Diagram(s)

Not applicable. While the PR introduces significant validation and workflow changes, the core interactions are covered by the layer diagrams above: configuration file → loader → provisioning, URL validation before clone, temporary index operations, and publication validation → push flow.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • delete-cloud/Coding-Agent-Loop#146: Both PRs modify Git workspace diff/patch functionality in DockerWorkspaceProvider, with this PR refactoring to use temporary indexes while #146 focuses on diff/patch export.
  • delete-cloud/Coding-Agent-Loop#149: Both PRs implement Git workspace cloning/publishing validation, with this PR adding allowed_hosts enforcement and HTTPS/credential/token checks that complement #149's branch publishing work.
  • delete-cloud/Coding-Agent-Loop#145: Both PRs extend the kind="git" provisioning path in docker_workspace_provider.py, with this PR adding remote_sources.git.allowed_hosts validation to #145's Git workspace cloning implementation.

"I'm a rabbit in the code den, I check each host by hand,
I seed a temp index in the sand, keep diffs tidy and grand,
I refuse unsafe clones and keep tokens out of sight,
Docs now list allowed hosts so everything stays right,
A little hop, a careful patch — secure workspaces take flight!"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: harden remote publication safety' directly and clearly describes the main objective of the changeset, which centers on hardening safety for remote Git operations through validation and security improvements.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/remote-publication-safety

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@coding-agent/tests/coding_agent/environment/test_docker_workspace_provider.py`:
- Line 980: Update the pytest.raises(...) calls in
coding_agent/tests/coding_agent/environment/test_docker_workspace_provider.py so
their match= patterns are raw string literals with regex metacharacters escaped;
specifically change the string "host is not in remote_sources.git.allowed_hosts"
to a raw, escaped regex like r"host is not in
remote_sources\.git\.allowed_hosts" (and apply the same fix for the other two
match= occurrences), ensuring each dot is escaped so the test matches the
literal message rather than any character.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c83c86bb-72dc-4ee4-9ab5-8af96a394ea6

📥 Commits

Reviewing files that changed from the base of the PR and between 77cc89a and ef97361.

📒 Files selected for processing (5)
  • coding-agent/docs/remote-sandbox-production.md
  • coding-agent/src/coding_agent/environment/docker_workspace_provider.py
  • coding-agent/src/coding_agent/ui/http_server.py
  • coding-agent/tests/coding_agent/environment/test_docker_workspace_provider.py
  • coding-agent/tests/ui/test_http_server.py

monkeypatch.setattr(subprocess, "run", fake_run)

with pytest.raises(
ValueError, match="host is not in remote_sources.git.allowed_hosts"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Escape regex metacharacters in pytest.raises(... match=...) patterns.

The match= arguments at lines 980, 1022, and 1494 contain unescaped . characters that are regex metacharacters. They currently match any character at those positions, so a test could silently pass on a slightly different error message. Use raw strings with escaped dots (Ruff RUF043).

🔧 Proposed fix
-    with pytest.raises(
-        ValueError, match="host is not in remote_sources.git.allowed_hosts"
-    ):
+    with pytest.raises(
+        ValueError, match=r"host is not in remote_sources\.git\.allowed_hosts"
+    ):
-    with pytest.raises(ValueError, match="workspace_source.remote_url must use https"):
+    with pytest.raises(ValueError, match=r"workspace_source\.remote_url must use https"):
-    with pytest.raises(ValueError, match="remote_publication.git_token_env is not set"):
+    with pytest.raises(ValueError, match=r"remote_publication\.git_token_env is not set"):

Also applies to: 1022-1022, 1494-1494

🧰 Tools
🪛 Ruff (0.15.12)

[warning] 980-980: Pattern passed to match= contains metacharacters but is neither escaped nor raw

(RUF043)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@coding-agent/tests/coding_agent/environment/test_docker_workspace_provider.py`
at line 980, Update the pytest.raises(...) calls in
coding_agent/tests/coding_agent/environment/test_docker_workspace_provider.py so
their match= patterns are raw string literals with regex metacharacters escaped;
specifically change the string "host is not in remote_sources.git.allowed_hosts"
to a raw, escaped regex like r"host is not in
remote_sources\.git\.allowed_hosts" (and apply the same fix for the other two
match= occurrences), ensuring each dot is escaped so the test matches the
literal message rather than any character.

@delete-cloud
delete-cloud merged commit 3555b90 into main May 14, 2026
1 check was pending
@delete-cloud
delete-cloud deleted the fix/remote-publication-safety branch May 14, 2026 01:22
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