Restrict agent container autopull to latest tag#46
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
Greptile SummaryThis PR restricts Docker image auto-pull to only images tagged as Key changes:
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[CLI command invoked\nrun / proxy start / logs start] --> B{auto_pull enabled\nor --pull flag?}
B -- "--pull flag" --> D[Pull image unconditionally]
B -- "auto_pull enabled" --> C{_is_latest_tag?}
C -- "Yes\nexplicit :latest or no tag" --> D
C -- "No\npinned version/digest" --> E[Skip pull]
D --> F[Start container]
E --> F
A2[run --paste-images] --> G{DISPLAY set?}
G -- "No" --> H[Warn & skip X11 forwarding]
G -- "Yes" --> I[Mount /tmp/.X11-unix\nSet DISPLAY env]
I --> J[⚠️ Xauthority NOT forwarded\nX11 auth may fail]
J --> F2[Start agent container]
H --> F2
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def _x11_volumes_and_env(display: str) -> tuple[list[tuple[str, str, str]], dict[str, str]]: | ||
| """Return X11 socket volumes and DISPLAY env for paste-image support.""" | ||
| volumes: list[tuple[str, str, str]] = [("/tmp/.X11-unix", "/tmp/.X11-unix", "rw")] | ||
| env: dict[str, str] = {"DISPLAY": display} | ||
| return volumes, env |
There was a problem hiding this comment.
X11 authentication cookie not forwarded
The _x11_volumes_and_env helper mounts the X11 socket and sets DISPLAY, but standard X11 security (MIT-MAGIC-COOKIE-1) also requires the authorization cookie file (~/.Xauthority). Without mounting that file and setting the XAUTHORITY env var, any X11 client inside the container will fail with Authorization required, use MIT-MAGIC-COOKIE-1 — unless the host happens to have xhost +local: enabled, which is an insecure configuration.
To make --paste-images work in the default case, the host's Xauthority file needs to be forwarded:
def _x11_volumes_and_env(display: str) -> tuple[list[tuple[str, str, str]], dict[str, str]]:
"""Return X11 socket volumes and DISPLAY env for paste-image support."""
xauth_path = os.environ.get("XAUTHORITY", os.path.expanduser("~/.Xauthority"))
volumes: list[tuple[str, str, str]] = [("/tmp/.X11-unix", "/tmp/.X11-unix", "rw")]
env: dict[str, str] = {"DISPLAY": display}
if os.path.exists(xauth_path):
volumes.append((xauth_path, xauth_path, "ro"))
env["XAUTHORITY"] = xauth_path
return volumes, env| if _is_latest_tag(proxy_image): | ||
| manager.pull_if_newer(proxy_image) |
There was a problem hiding this comment.
Missing user-facing info message for proxy update check
In proxy.py the equivalent block emits info("Checking for proxy image updates…") before calling pull_if_newer, so the user can see what is happening. This call site in run.py silently pulls in the background, which can look like a hang for larger images.
| if _is_latest_tag(proxy_image): | |
| manager.pull_if_newer(proxy_image) | |
| if _is_latest_tag(proxy_image): | |
| info("Checking for proxy image updates…") | |
| manager.pull_if_newer(proxy_image) |
| def _is_latest_tag(image: str) -> bool: | ||
| """Return True when *image* uses the ``latest`` tag (explicitly or by omission).""" | ||
| name = image.split("/")[-1] | ||
| return ":" not in name or name.endswith(":latest") |
There was a problem hiding this comment.
Private-name function imported as a public API
_is_latest_tag carries a leading underscore, which in Python signals "private to this module / implementation detail". However it is now imported explicitly in three separate command modules (logs.py, proxy.py, run.py), making it a de-facto public utility. Either rename it to is_latest_tag to signal the intended public use, or document why the single-underscore prefix is intentional (e.g. "not part of the stable API"). As-is, tools like linters, documentation generators, and other contributors will reasonably assume this symbol should not be imported externally.
1 participant
Eensures that Docker images tagged as
latestare checked for updates before container startup for agents