-
Notifications
You must be signed in to change notification settings - Fork 589
[AGE-4000] fix(runner): rebuild agent mounts before STS credentials expire #5520
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
d311dfc
acaa86e
290cfc2
533eed5
9ee103e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,12 +20,23 @@ | |
| # STS responses are SOAP-ish XML under the 2011-06-15 namespace; strip it for tag lookups. | ||
| _STS_NS = "{https://sts.amazonaws.com/doc/2011-06-15/}" | ||
|
|
||
| # Mirrors the `maxSessionLength: 12h` we configure in the compose SeaweedFS IAM config; the two | ||
| # must stay in sync. | ||
| _SEAWEEDFS_MAX_SESSION_SECONDS = 43200 | ||
|
|
||
| # AWS GetFederationToken accepts DurationSeconds in [900, 129600]. | ||
| _AWS_MAX_FEDERATION_SECONDS = 129600 | ||
|
|
||
|
|
||
| def _parse_sts_credentials(xml_text: str) -> Credentials: | ||
| """Parse an STS XML response (`AssumeRoleWithWebIdentity`) into a miniopy Credentials. | ||
|
|
||
| Returns the scoped access/secret/session triple plus expiry; raises if the response | ||
| carried no Credentials block (e.g. an STS error document slipped past the status check). | ||
|
|
||
| Fails closed on a missing or unparsable `Expiration`: the runner reads a missing expiry as | ||
| "never expires", so one malformed response would silently disable the reuse bound for the | ||
| lifetime of a pooled environment. | ||
| """ | ||
| root = ElementTree.fromstring(xml_text) | ||
| creds_el = root.find(f".//{_STS_NS}Credentials") | ||
|
|
@@ -36,13 +47,15 @@ def _text(tag: str) -> Optional[str]: | |
| el = creds_el.find(f"{_STS_NS}{tag}") | ||
| return el.text if el is not None else None | ||
|
|
||
| expiration = None | ||
| raw_exp = _text("Expiration") | ||
| if raw_exp: | ||
| try: | ||
| expiration = datetime.fromisoformat(raw_exp.replace("Z", "+00:00")) | ||
| except ValueError: | ||
| expiration = None | ||
| if not raw_exp: | ||
| raise MountStorageUnavailable("STS response carried no credential expiry.") | ||
| try: | ||
| expiration = datetime.fromisoformat(raw_exp.replace("Z", "+00:00")) | ||
| except ValueError as exc: | ||
| raise MountStorageUnavailable( | ||
| "STS response carried an unparsable credential expiry." | ||
| ) from exc | ||
|
|
||
| return Credentials( | ||
| access_key=_text("AccessKeyId"), | ||
|
|
@@ -229,14 +242,21 @@ async def _sign_web_identity( | |
| """SeaweedFS path: unauthenticated `AssumeRoleWithWebIdentity` against the store endpoint.""" | ||
| host, secure = self._host_secure() | ||
| endpoint = f"{'https' if secure else 'http'}://{host}" | ||
| # SeaweedFS validates DurationSeconds in [900, 43200], but the EFFECTIVE lifetime is | ||
| # min(DurationSeconds, web-identity token expiry, maxSessionLength). The token-expiry cap | ||
| # has no floor, so minting the token at the capped TTL is what lets a sub-900 TTL take | ||
| # effect (the QA lever); DurationSeconds only has to stay inside the validated range. | ||
| capped = min(duration_seconds, _SEAWEEDFS_MAX_SESSION_SECONDS) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The core of the SeaweedFS half: the token now carries the real requested lifetime (capped at the 12h session ceiling), while |
||
| body = urlencode( | ||
| { | ||
| "Action": "AssumeRoleWithWebIdentity", | ||
| "Version": "2011-06-15", | ||
| "RoleArn": webidentity.role_arn(), | ||
| "RoleSessionName": webidentity.STORE_SUBJECT, | ||
| "WebIdentityToken": webidentity.mint_web_identity_token(), | ||
| "DurationSeconds": str(duration_seconds), | ||
| "WebIdentityToken": webidentity.mint_web_identity_token( | ||
| ttl_seconds=capped | ||
| ), | ||
| "DurationSeconds": str(max(capped, 900)), | ||
| "Policy": scope_policy, | ||
| } | ||
| ) | ||
|
|
@@ -261,9 +281,9 @@ async def _sign_federation_token( | |
| """Remote-S3 path: SigV4-signed `GetFederationToken` against the store's STS endpoint. | ||
|
|
||
| The STS endpoint defaults to the S3 endpoint (MinIO co-locates STS there); AWS splits | ||
| it onto `sts.<region>.amazonaws.com`, set via AGENTA_STORE_STS_ENDPOINT_URL. Duration is | ||
| clamped to STS's 900s floor. The request is signed with the store's master keys; the | ||
| returned credentials carry only the inline `Policy`'s permissions. | ||
| it onto `sts.<region>.amazonaws.com`, set via AGENTA_STORE_STS_ENDPOINT_URL. Duration | ||
| is clamped into STS's [900, 129600] range. The request is signed with the store's | ||
| master keys; the returned credentials carry only the inline `Policy`'s permissions. | ||
| """ | ||
| endpoint = (self._sts_endpoint_url or self._endpoint_url or "").rstrip("/") | ||
| if not endpoint: | ||
|
|
@@ -275,7 +295,10 @@ async def _sign_federation_token( | |
| "Action": "GetFederationToken", | ||
| "Version": "2011-06-15", | ||
| "Name": webidentity.STORE_SUBJECT, | ||
| "DurationSeconds": str(max(duration_seconds, 900)), | ||
| # AWS rejects anything outside its own range; clamp rather than fail the sign. | ||
| "DurationSeconds": str( | ||
| min(max(duration_seconds, 900), _AWS_MAX_FEDERATION_SECONDS) | ||
| ), | ||
| "Policy": scope_policy, | ||
| } | ||
| ) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1117,6 +1117,15 @@ def enabled(self) -> bool: | |
| class MountsConfig(BaseModel): | ||
| """Mounts-domain config. Store credentials live in StoreConfig.""" | ||
|
|
||
| # Lifetime of signed mount credentials, in seconds. The store backends clamp it to their | ||
| # own STS bounds. | ||
| credentials_ttl_seconds: int = Field( | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| default_factory=lambda: ( | ||
| _parse_optional_positive_int_env("AGENTA_MOUNTS_CREDENTIALS_TTL_SECONDS") | ||
| or 3600 | ||
| ) | ||
| ) | ||
|
|
||
| model_config = ConfigDict(extra="ignore") | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| # Fix: agent mounts return EACCES after STS credentials expire | ||
|
|
||
| Planning workspace for GitHub issue Agenta-AI/agenta#5516. Agent file mounts go | ||
| "Permission denied" for many minutes at a time because the temporary store credentials | ||
| behind them expire and nothing reacts. This workspace holds the investigation and the | ||
| proposed fix. | ||
|
|
||
| ## What each file answers | ||
|
|
||
| | File | Question it answers | | ||
| | --- | --- | | ||
| | [context.md](context.md) | What breaks for the user today, and why this work exists. | | ||
| | [research.md](research.md) | How credentials flow end to end, what actually expires and when, where credentials are installed into mounts, which refresh mechanisms geesefs supports, and why the fix is safe on both SeaweedFS and real AWS. | | ||
| | [plan.md](plan.md) | The chosen fix, the exact code changes per file, the test plan, and the live QA plan. | | ||
| | [decisions.md](decisions.md) | Every decision the fix embeds, its trade-off, and how to back it out. | | ||
| | [status.md](status.md) | Where this work stands right now. | | ||
|
|
||
| ## Domain nouns, one sentence each | ||
|
|
||
| - **Runner**: the Node sidecar (`services/runner/`) that executes one agent turn per | ||
| HTTP `/run` request. | ||
| - **Sandbox**: the isolated environment a turn runs in; "local" means the runner's own | ||
| host, "remote" means a Daytona cloud VM. | ||
| - **Turn**: one user-message-to-agent-reply exchange; the runner's unit of work. | ||
| - **Turn boundary**: the moment between two turns, when the runner decides whether to | ||
| reuse a live environment or build a fresh one. | ||
| - **Session pool**: the runner's in-memory table of live environments parked between | ||
| turns so the next turn can continue without a rebuild. | ||
| - **Mount**: a directory in the sandbox whose contents live in the object store, so | ||
| files survive sandbox teardown. | ||
| - **Durable agent volume**: the per-agent mount (`agent-files/`) shared across all | ||
| sessions of one agent; long-lived by design. | ||
| - **Session scratch**: the per-session working directory mount (`cwd`); re-signed at | ||
| every turn boundary. | ||
| - **Mount credentials**: the temporary, prefix-scoped S3 key, secret, and session | ||
| token a mount's geesefs daemon holds; they carry an expiry and cannot be renewed | ||
| in place. | ||
| - **Installed lease**: the expiry of the credentials a live environment's daemons | ||
| are actually running on, as opposed to the expiry of credentials signed later and | ||
| never given to them. | ||
| - **geesefs**: the FUSE filesystem binary that presents an S3 prefix as a local | ||
| directory; each mount is one geesefs daemon process. | ||
| - **S3**: the object-storage HTTP protocol; Agenta speaks it to whichever store backs | ||
| the deployment. | ||
| - **SeaweedFS**: the S3-compatible store bundled with self-hosted Agenta. | ||
| - **STS**: the "Security Token Service" protocol for minting temporary, scoped | ||
| S3 credentials; both SeaweedFS and AWS implement it. | ||
| - **EACCES**: the "Permission denied" error code; geesefs returns it for every file | ||
| operation once the store answers 403. | ||
| - **ENOTCONN**: the "Transport endpoint is not connected" error code; the kernel | ||
| returns it when a FUSE daemon has died but its mount point is still registered. | ||
|
|
||
| ## Related work | ||
|
|
||
| - `docs/design/mount-file-viewer/`: the mounts file-listing UI; shares the same sign | ||
| endpoints, no credential-lifetime logic. | ||
| - Draft PR #5247 and design issue #5215 introduced the durable agent mount this bug | ||
| hits hardest; its design workspace lives on that PR's branch, not on main. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fail closed on purpose: the runner reads a missing expiry as "never expires", so one malformed STS response used to disable the reuse bound silently for the lifetime of a pooled environment. A refused sign degrades the turn to mount-less (the existing handling for any sign failure); it does not fail the turn. Both signing branches are STS, so there is no legitimate no-expiry path.