fix: kiro error pattern for rate limit detection#82
Open
changhansung wants to merge 286 commits into
Open
Conversation
…dcoded 1 Telegram General Topic has thread_id=1 by convention, but Discord channels have snowflake IDs. When general_channel_id is set in fleet.yaml options, use it as the general instance's topic_id. Fixes 'Unknown Channel' error on fleet start with Discord.
- ClassicBotYaml.defaults gains allowed_guilds?: string[] - isGuildAllowed(): empty/unset list allows all guilds (backward compat) - Discord adapter slash_command events now include guildId - handleClassicStart checks guild whitelist before registering channel
After fleet.yaml is written, Discord quickstart now asks: - Set up ClassicBot? (allows /start in any channel) [Y/n] - If yes: collects allowed_guilds (primary guild pre-filled, can add more) - Asks default backend (defaults to fleet backend) - Writes ~/.agend/classicBot.yaml matching ClassicBotYaml interface
When fleet.yaml exists, quickstart now offers 3 options: 1. Add allowed users - load existing config, show current users, collect new IDs, append and write back 2. Overwrite - start fresh (previous behavior) 3. Skip - skip fleet.yaml changes After fleet.yaml handling, if classicBot.yaml exists, offers to add allowed guilds (load/show/collect/append/write back). If classicBot.yaml doesn't exist and Discord, runs the existing creation flow.
…apter /start in a non-primary guild was silently dropped because the channel wasn't in openChannels yet (chicken-and-egg: /start creates the classic channel, but the channel must exist to pass the openChannels check). Fix: let slash commands (isChatInputCommand) through regardless of guild. The guild whitelist check in fleet-manager's handleClassicStart handles authorization. Only block non-slash interactions (buttons) from unknown channels.
Covers setup, slash commands, server whitelist (allowed_guilds), per-channel backend override, and quickstart supplement mode.
Previously any local process knowing AGEND_PORT could POST to /agent with any instance name and impersonate that instance. Now each spawn writes a fresh 32-byte token to <instanceDir>/agent.token (mode 0600); agent-cli reads it and sends it in X-Agend-Instance-Token; the endpoint verifies with timingSafeEqual against the on-disk value for the claimed instance. Also propagates AGEND_HOME to the child so agent-cli finds the token file when the daemon uses a non-default data dir. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
POST /ui/tasks, /ui/schedules, /ui/teams, /ui/config and /ui/send previously cast fields out of JSON with `body.x as string`, which allowed unknown-field injection and unchecked types. outbound-handlers already validates agent-facing calls via zod; web-api now follows the same pattern with strict, public-only schemas: - TaskCreateSchema / TaskUpdateSchema - ScheduleCreateSchema - TeamCreateSchema (name restricted to [A-Za-z0-9._-]) - ConfigUpdateSchema (.strict() at every level) - SendMessageSchema (caps message length at 16 KB; early-aborts oversized bodies) Unknown fields now return 400 instead of silently passing through into internal structures. Added tests/web-api.test.ts covering the reject cases. Addresses ultrareview Phase 1 item P1.3.
importConfig previously streamed every archive entry straight to tar xzf,
trusting the uploader. A malicious tarball with `../etc/passwd` or an
absolute path would write outside dataDir.
Added:
- 500 MB hard cap on input file size (zip-bomb guard)
- Pre-extraction listing via `tar tzf`; reject entries with `..`, absolute
paths, or whose first segment doesn't match the dataDir basename
- After path resolution, verify each entry lands inside dataDir
- Unit test that builds a hostile tarball and confirms import aborts
Addresses ultrareview Phase 1 item P1.4.
…P1.5) The launchd plist and systemd unit templates interpolated ServiceVars with ejs <%= %>, which HTML-escapes but does not strip newlines. A newline in logPath, workingDirectory, or execPath would inject additional systemd directives (e.g. ExecStartPost=rm -rf ~) or break out of plist <string> elements. Added a validator that runs on every render/install call: - rejects control characters (\x00-\x1f, \x7f) in any interpolated value - rejects `]]>` (plist CDATA terminator) - requires execPath / workingDirectory / logPath to be absolute - restricts label (filename component) to [A-Za-z0-9._-]+ Unit tests cover newline injection, NUL, relative path, and label shenanigans. Addresses ultrareview Phase 1 item P1.5.
Previously the project_roots boundary check used path.resolve() which only performs string normalization. An attacker with write access to a directory under an allowed root could create a symlink to /etc (or elsewhere) and then pass that symlinked path to create_instance; the prefix check would pass because the string was still under the allowed root. Switched both the candidate directory and each allowed root to fs.realpathSync(), and use path.sep for the boundary separator so the check is correct across platforms. Roots that don't exist on disk are rejected. Addresses ultrareview Phase 1 item P1.6.
…(P1.7) Telegram/Discord adapters' confirmPairing previously discarded the caller identity before invoking AccessManager.confirmCode, disabling the per-user rate-limit (10 failures / 60s) entirely. Brute-force surface reduced to the 5-attempts-per-pending-code cap only. - Extended ChannelAdapter.confirmPairing signature with optional callerUserId - Telegram + Discord adapters forward the id to AccessManager.confirmCode - Unit test updated to assert argument is forwarded Addresses ultrareview Phase 1 item P1.7.
daemon.handleCheckoutRepo:
- Reject branch names starting with `-` or `+` so execFile-passed values
cannot be reinterpreted by git as option flags (e.g. --upload-pack=).
- Insert `--` before branch in `git rev-parse --verify` as defense in depth.
tmux-manager.pipeOutput:
- Reject logPath containing control characters (\x00-\x1f). tmux runs the
pipe command via /bin/sh, so a newline in the path would break the single
quote escape and let an attacker inject arbitrary shell after the redirect.
Addresses ultrareview Phase 1 item P1.8.
resolveAccessPathFromConfig used the instance argument as a literal path
segment under <dataDir>/instances/. Callers (CLI, tool router) already
constrain instance names, but a defence-in-depth check here prevents any
future caller — or a config-file edit — from escaping the data dir via
"..", "/", "\", or NUL.
Topic mode is unaffected because it doesn't embed the instance name in
the resolved path.
Test: tests/resolve-access-path.test.ts adds 3 cases covering rejection
patterns ("..", "../etc", "a/b", "a\\b", "", "a\\0b"), confirms topic
mode tolerates anything, and confirms conventional names still pass.
Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Telegram bot API requests embed the bot token in the URL path (`/bot<token>/...`). A misconfigured or malicious `telegram_api_root` would silently leak the token to whatever host is configured. Validate at startup: only api.telegram.org (https) and loopback (localhost / 127.0.0.1 / ::1, for E2E mock servers) are accepted. Anything else throws — fail fast at config load rather than after the first API call sends credentials to the attacker. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
The 409 conflict retry loop was unbounded — a permanently stuck conflict would keep retrying forever every 15s. Cap at 30 attempts (~7 min total) and emit a final error so the operator can intervene. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
The previous 10 MB ceiling let a runaway producer (or hostile peer sharing the socket) buffer 10 MB per client before being dropped. Real IPC payloads — tool calls/responses, decision/task lists — are well under 100 KB; 1 MB stays comfortably above any legitimate message while shrinking the DoS window 10x. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
The flood-control branch in MessageQueue.runWorker had a dead comment
("reset backoff now that we've cleaned up") but never actually reset the
backoff. As a result, once sustained 429s drove backoffMs above
FLOOD_CONTROL_THRESHOLD_MS, the queue would shed all status_update items
yet still wait the full ~30s exponential delay between every retry — even
though the queue was now small and the rate-limit may already have eased.
Fix:
- After dropping items, reset backoffMs to INITIAL_BACKOFF_MS and clear
backoffUntil, so the next retry happens within ~1s instead of ~30s.
- Emit a warn log noting how many items were dropped, so operators can
see flood control activity instead of silent drops.
Test: tests/channel/message-queue.test.ts asserts that after sustained
429s push backoff past the threshold, status_updates are mostly dropped,
the warn fires, and the surviving content message still gets delivered.
Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
When control mode disconnected and reconnected, paneToWindow and lastOutputAt kept stale paneId → windowId mappings from the previous tmux server. After a tmux restart pane ids are recycled, so waitForIdle / isIdle could consult a pane that no longer belongs to the expected window (or has been recycled to a different one) — returning false "idle" and letting us paste into the wrong target. Track every registered windowId in a Set; on (re)connect clear the transient paneId caches and re-resolve panes for all registered windows so upper layers don't need to know about the reconnect. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
setInterval keeps firing the poll callback even when the previous invocation is still awaiting stat/open/read. With slow disk or a large transcript two concurrent runs would race on byteOffset — both could read the same byte range and emit duplicate tool_use / assistant_text events to downstream consumers. A simple boolean guard makes the second tick a no-op until the first finishes; setInterval itself stays unchanged. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
If the daemon was down across a scheduled fire time, the schedule would silently skip until the next regular trigger. Now `init()` walks every enabled schedule and, if its most recent expected run falls in the past 24h, fires it once. The 24h cap avoids dumping a backlog of pings on the user after a long outage, while still recovering from short crashes/restarts. At most one catch-up per schedule — `* * * * *` doesn't replay 59 missed minutes. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
When a daily limit fires, the limit handler pauses the instance. If the user manually restarts that instance, the new session causes a cost-rotation event but `limitEmitted` stayed sticky for the rest of the day — so the new session could blow past the daily cap again without ever re-pausing. Same shape for the warn flag. Reset both flags on `snapshotAndReset` so the next cost report re-evaluates against the (still-elevated) accumulated total and re-emits if the threshold is still crossed. Within a single session each event still fires at most once. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Old impl used `setHours(24, 0, 0, 0)` (a local-tz operation) on a Date constructed by reinterpreting the target-tz wall-clock as local-tz time. The two reinterpretations cancel for normal days, but on DST days the target tz's day length differs from local arithmetic by an hour: - Spring-forward day in target tz (23h day): scheduler fires ~1h LATE. - Fall-back day in target tz (25h day): scheduler fires ~1h EARLY. Either way the daily reset misaligns with the user's calendar day twice a year — exactly when daily-budget alerts most need to be predictable. Replaced with an Intl.DateTimeFormat-based binary search that finds the first instant where the date *as observed in the target tz* changes. This naturally handles 23h, 24h, and 25h day lengths regardless of local-vs-target tz mismatches. Tests cover the two specific buggy moments (LA pre-jump 01:30 PST and LA pre-fallback 01:30 PDT) plus a 26h upper bound and positivity invariant.
The archived Set lived in memory only — after a daemon restart the poller would re-archive every previously-closed idle topic and re-issue closeForumTopic, fighting users who had reopened topics manually. Persist to <dataDir>/archived-topics.json on add/remove and load on construct. Adds dataDir to ArchiverContext (FleetManager already exposes it). Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
The 6-hex suffix on getTmuxSessionName/getTmuxSocketName is not cryptographic — it just disambiguates two custom AGEND_HOME values on the tmux namespace. md5 was fine functionally but trips FIPS-mode Node and routine security scanners. Switching to sha256 (still slice(0,6)) removes those false positives. Behaviour change: users with a custom AGEND_HOME will see their tmux session/socket name change once on first run after the upgrade. A daemon restart resyncs cleanly; any orphan tmux session under the old name can be killed manually. Default AGEND_HOME (which uses the literal string "agend" with no suffix) is unaffected. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
- src/scheduler/scheduler.ts no longer defines its own validateTimezone; it imports the canonical implementation from src/config.ts. This was the only duplicate left after PR #34 — error messages now consistently read "timezone: invalid timezone \"X\". Use IANA format ...". - src/quickstart.ts and src/setup-wizard.ts now write the .env file with mode 0o600 and chmod after the write (writeFileSync's mode flag is ignored for existing files). The .env contains the bot token and optionally a Groq API key — restricting to owner-only read/write prevents other local users / sibling processes from reading them. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
feat: daily update check, /update detached restart, doctor expansion
feat: doctor checks for service Restart policy and daemon-reload
1. sd-notify: READY=1 after generals start, WATCHDOG=1 every 30s, STOPPING=1 on shutdown. Zero dependencies (uses net.createConnection). 2. systemd template: Type=notify, NotifyAccess=main, WatchdogSec=60. systemd kills+restarts fleet if no watchdog ping for 60s. 3. General instance startup failure now sends notification to the general topic channel with error details.
feat: systemd watchdog + general startup failure notification
…ollab log - Replace all user-facing 'agend fleet start' with 'agend start' - Collab mode: skip logging empty bot messages (no text, no attachments)
fix: update message uses 'agend start' + skip empty bot messages in collab log
- Upgrade grammy to 1.44.0 - Auto-detect rich content (tables, code blocks, headings, HR, details) - Use sendRichMessage API for rich content, fallback to sendText on failure
feat: Telegram Rich Messages support (Bot API 10.1)
docs: add tmux Ctrl+C unstick procedure to fleet-health skill
feat: add /update and /doctor to TG bot menu
feat: add /update and /doctor Discord slash commands (admin only)
fix: /doctor uses agend backend doctor with fleet default
feat: notify General when non-admin tries /start or /stop
docs: add kiro-cli session storage path to skill
fix: agend update auto-removes stale npm link
fix: systemd NotifyAccess=all + TimeoutStartSec=0
feat: /status + /sysinfo markdown tables (Rich Message compatible)
fix: install.sh removes npm-linked old version via readlink
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds 'having trouble responding' pattern to kiro backend. /sysinfo table format already correct (verified).