3 Commits

Include renames

Author	SHA1	Message	Date
euxaristiaandeuxaristia	275e248669	fix(oauth): Split an oversized keyring token blob across entries (#1007) ... * fix(oauth): split an oversized keyring token blob across entries Store every provider and MCP token in one keyring entry and the store stops working once the logins outgrow it. On macOS the secret rides inside a `security -i` command line capped at 4095 bytes, which leaves 3027 bytes of JSON for all logins combined, so a second OIDC login fails to save and every write after it fails too. Split a blob that does not fit across numbered entries and put a manifest in the anchor account. Chunks live in two alternating generations: a write fills the one that is not live, then replaces the manifest, so that single write is the commit point and a crash partway through still reads the previous generation. `zc1:` cannot prefix base64, so an entry written by an existing build is still recognised and read without a migration step. Reserve the range a write will occupy before occupying it. Without that, a write interrupted while filling a longer generation leaves chunks above the count the manifest records, and no later cleanup knows to delete them: a fragment of a token blob would stay in the keychain for good. Expose the per-entry budget from internal/keyring rather than hardcoding the macOS figure in the oauth store, sharing one line builder with Set so the budget and the boundary it describes cannot drift. Backends with no limit report so and keep the single-entry layout, so Linux is untouched. Refs #937 * fix(oauth): lock keyring reads, validate manifest digest, and harden chunk tests * test(oauth): assert Status blocks during keyring lock regression test * fix(oauth): sweep both chunk generations when a store regrows A shrink writes the blob back under the anchor, which replaces the manifest and takes the per-generation chunk counts with it. From then on nothing can name the chunks a failed cleanup left behind, so the growth branch's sweep of the target generation is the only one that will ever reach them — and it only ever targets family A. A keychain that refused one delete during the shrink therefore kept a superseded generation of access, ID and refresh tokens indefinitely, with no way for the user to know. Sweep the other generation alongside the target, and document that the reclaim waits for the next growth: a store that shrinks once and never grows again keeps the residue, which sweeping on every whole write would close at a cost of 128 `security` invocations per save on macOS. Also record why Load and Status take the cross-process lock, and give the missing-chunk error the same "log in again" advice the digest failure carries. Refs #937 * fix(oauth): serialize keyring operations under user-level lock Derive the keyring backend lock file path from the user's home directory rather than file store configuration, ensuring processes with distinct store paths share the same lock domain for the OS keychain. Refs #938 * Address review feedback on chunked keyring token store and recovery Refs #938 * Refine auth reset message and chunk format notation Refs #938 * Harden OAuth store reset lifecycle, keyring migration recovery, and completions. Refs #938 * Validate keyring cleanup markers, propagate lock resolution errors, and test reader concurrency. Refs #938 * Preserve live OAuth credentials when stale cleanup markers survive migration. Refs #937 * Verify JSON output from OAuth reset to cover its recovery response. Refs #937 * Verify that JSON OAuth reset actually removes stored logins. Refs #937 * Require confirmation before resetting shared OAuth credentials. Refs #937 --------- Co-authored-by: euxaristia <[email protected]>	2026-09-12 08:33:44 +08:00
euxaristia	91ea6ded75	fix(keyring): pass generic password via stdin on macOS (#574) ... * fix(keyring): pass generic password via stdin on macOS Pass the generic password secret to security add-generic-password via stdin instead of passing it as a command-line argument. This prevents the secret from leaking to the local process list (visible via ps) and aligns the macOS keyring implementation with the Linux secret-tool implementation. * fix(keyring): pipe secret twice for macOS security password+retype prompt The macOS security add-generic-password command prompts for both a password and a retype confirmation when -w has no trailing value. The previous stdin approach only piped the secret once, causing a passwords don't match failure. Write the secret twice separated by newlines to satisfy both prompts. * fix(keyring): reject multiline secrets on macOS instead of corrupting them Addresses jatmn's review: - security's -w password+retype prompt reads stdin line by line, so a secret containing its own newline can't be distinguished from the password/retype separator: the first line read back would silently truncate the stored value. Set now rejects a secret containing \r or \n on darwin before invoking security, rather than storing a corrupted value. Linux's secret-tool has no such restriction (reads the whole stdin payload), so it is unaffected. - Updated the package doc comment, which still described the old argv-based (process-list-exposed) macOS path this PR replaced. Added TestKeyringSetRejectsMultilineSecretOnDarwin and TestKeyringSetAllowsMultilineSecretOnLinux. * fix(keyring): drive security -i for macOS Set instead of the -w prompt The trailing -w prompt uses getpass(3), which reads from /dev/tty whenever the process has a controlling terminal, so the piped secret was ignored in any interactive session, and getpass's small fixed buffer would truncate long secrets (like the base64 OAuth blob) when the fallback did engage. Interactive mode reads whole commands from stdin, keeping argv at just -i. Arguments are quoted for SecurityTool's split_line parser (backslash and double quote escaped inside double quotes). The parser is line-based with a 4096-byte buffer, so Set rejects newlines and oversized payloads up front rather than corrupting the stored value.	2026-07-14 14:46:09 +08:00
KRATOS	c38444157c	Audit backlog: tui mouse API, notify webhook, swarm scheduling, daemon-remote bundles, MCP/oauth store unification, OS keyring, dead-code prune (#216) ... * tui: migrate off the deprecated Bubble Tea mouse Type API (SA1019) The mouse classification helpers checked both the current Button/Action pair and the deprecated msg.Type / tea.Mouse* constants. Bubble Tea's parser always populates Button+Action and only DERIVES Type from them, so the Type fallbacks were pure redundancy (a left-button drag is Action==Motion, already covered by mouseMotion). Drop them — clears all 11 SA1019 findings in internal/tui with no behavior change. The left-drag test now asserts the Action==Motion path directly. Gates: gofmt/vet/build(host+linux+windows)/test -race/staticcheck(SA1019 0)/ deadcode(no new) all pass. * notify: wire the webhook sink behind an opt-in env var The WebhookSink (Slack / generic JSON POST on turn completion) was fully implemented, redaction-safe, and tested — but never attached to a Notifier, so deadcode flagged its entire machinery (NewWebhookSink, Emit, text, redactLinks, log, eventType, readSnippet) as unreachable. Wire it via a small, testable helper, MaybeAddWebhookSink(n, env, logf): - reads ZERO_NOTIFY_WEBHOOK_URL (+ optional ZERO_NOTIFY_WEBHOOK_SUMMARY) - no-op when the URL is unset/blank, so it is safe to call unconditionally - sourcing the URL from the environment keeps the secret token out of any on-disk config file Attached at both notifier construction sites: - headless exec: failed deliveries log to stderr (never stdout); the sink redacts before logging - TUI: logf=nil so a delivery failure can't corrupt the alt-screen The sink stays gated by the existing Mode/focus policy (verified by TestNotifierOffSuppressesSinks), so a webhook only fires when notifications are enabled (e.g. --notify both) — no change to default behavior. Opt-in and fail-closed: with the env var unset nothing is attached. deadcode: 100 -> 93 unreachable funcs (the 7 WebhookSink entries become reachable); no new dead code. gofmt/vet/build(host+linux+windows)/test -race/ staticcheck(no new)/govulncheck(0) all pass. * swarm: add opt-in recurring-spawn scheduler (wakeup + daily) Adds a dependency-free Scheduler to internal/swarm so a member can be spawned on a recurring schedule, plus a swarm_schedule tool to drive it. - Schedule is interval-based ("wakeup", every >= 1s) with an optional first delay and a MaxRuns cap; a daily "HH:MM" ("cron") time is expressed as a 24h interval whose first delay lands on the next occurrence. - Each fire spawns a fresh member through the existing Swarm.Spawn path, so members inherit the recorded policy and are bounded by the team slot cap + queue. Non-overlapping by default: a fire is skipped while the job's previous spawn is still running, preventing queue pile-up. - Scheduler is lazily created (Swarm.Scheduler()), opt-in (nothing runs until a job is added), and tied to the Swarm's base context — Swarm.Close() stops the scheduler first so no spawn fires during shutdown. - swarm_schedule tool: action add|list|cancel; prompt-gated like swarm_spawn; add validates schedule + agent type up front (fail fast). The timing loop takes an injectable ticker, so the tests drive fires deterministically with no real time (fire/skip/cap/cancel/close, plus pure nextDailyDelay/parseClock/swarmInt). All swarm tests pass 3x under -race. Gates: gofmt/vet/build(host+linux+windows)/test/staticcheck(clean)/govulncheck(0)/ deadcode(no new) all pass. * daemon/remote: SessionLink + git-bundle upload over the bridge Adds an opt-in way to ship a local repo's git history to a remote daemon and link a remote working tree to it — without a shared filesystem or a network git remote. The daemon core (protocol/server) is untouched; all changes live in the remote package plus a CLI hook. Wire: - The auth handshake gains an optional Mode field ("" / "session" => a daemon session as before; "bundle" => a one-shot git-bundle upload). Unknown modes, or bundle mode when uploads are disabled, are denied (fail closed). - Bundle transfer reuses the daemon frame codec: a header frame (link id + exact size), then the bundle bytes as KindData frames, then a result frame. Server (Bridge): - BridgeOptions.BundleDir enables uploads (empty => refused). A received bundle is size-capped (default 256 MiB), staged to a temp file, `git bundle verify`-d, then `git clone`-d into a staging dir and atomically renamed over <BundleDir>/<link-id>. Link ids are sanitized to a single path component and containment is re-checked, so an upload can never escape the bundle dir. Client: - UploadRepoBundle creates `git bundle create --all` of a work tree, hashes it (sha256), streams it over an authenticated bundle-mode TLS connection, and returns a SessionLink {address, link id, remote path, bundle sha}. SessionLink persists as a 0600 atomic JSON file (token never stored) with Save/Load/Validate. CLI: - `daemon serve-remote --bundle-dir <d>` enables uploads. - `daemon link --remote <addr> --repo <dir> --id <name> [--out <file>]` uploads and prints the remote path (+ a ready run command); `daemon link --show <file>` prints a saved link. Tests: git bundle create/verify/clone roundtrip; link-id sanitization + path containment; SessionLink save/load/validate (0600); and a full bridge upload E2E over real TLS (extract + content check + re-upload-replaces + disabled + bad token). All remote tests pass 2x under -race; CLI smoke verified via the binary. Gates: gofmt/vet/build(host+linux+windows)/test/staticcheck(no new)/ govulncheck(0)/deadcode(no new) all pass. * mcp: unify OAuth token storage onto the shared oauth store MCP server tokens lived in their own mcp-oauth-tokens.json with a bespoke codec/lock, separate from the provider-login store (internal/oauth). The oauth store already reserved the "mcp:" key namespace for exactly this. Unify them. - mcp.TokenStore now delegates to internal/oauth.Store, keying each server's token as "mcp:<server>". Its public API (StoredToken/TokenStatus/Save/Load/ Delete/Status/FilePath) is unchanged, so callers (network client, CLI) are untouched. The bespoke read/write/lock code is removed in favor of the shared, cross-process-locked, atomic store. - Transparent, non-destructive migration on first construction: a legacy mcp-oauth-tokens.json is imported into the unified store (only entries not already present — a newer unified token is never overwritten), then renamed to a ".migrated" backup so it is imported at most once and stays recoverable. A missing / unreadable / unknown-schema legacy file is left untouched and never blocks startup. An explicit FilePath suppresses the default-path migration so tests/overrides can't touch the real user config. - Server names that can't form a valid namespaced key (oauth.ValidateKey) are rejected on save and skipped during migration. Added oauth.Store.FilePath(). The token format was already identical (access/refresh/type/scopes/expires), so this is a key rename, not a format change. Existing TokenStore tests pass unchanged (they exercise the API); added migration + namespacing + newer-wins tests. Local run check: `zero mcp oauth status` migrates a seeded legacy file into oauth-tokens.json under mcp:demo, renames the legacy to .migrated, and leaks no token material. Gates: gofmt/vet/build(host+linux+windows)/test -race/staticcheck(clean)/ govulncheck(0)/deadcode(no new) all pass. * oauth: add an opt-in OS keyring storage backend Introduces a pluggable storage backend for the unified oauth token store and a dependency-free OS keyring implementation, selected with ZERO_OAUTH_STORAGE=keyring (default stays the 0600 file). - internal/keyring: a small, third-party-dep-free secret store backed by the OS tooling — `security` on macOS, `secret-tool` (libsecret) on Linux; other platforms report unsupported. On Linux the secret is passed via stdin (never the argv); on macOS `security` takes it as an argument (briefly visible in the process list) — documented, and a reason to prefer the file backend there. A runner seam makes the per-OS command logic fully testable without a real keychain. - internal/oauth: Store now persists its JSON blob through a blobStore backend. fileBlob preserves the exact prior behavior (0600, atomic write, cross-process lock file). keyringBlob stores the blob as one base64 entry (keeps multi-line JSON a single control-char-free value) via a KeyringClient. NewStore resolves the backend from StoreOptions.Storage or ZERO_OAUTH_STORAGE; unknown storage and an unavailable keyring fail closed with a clear error. Because both `zero auth` and the (now unified) MCP token store construct their store through oauth.NewStore, the keyring backend is honored by both with no call-site changes. Added oauth.Store.FilePath() returns a "keyring:" identifier for that backend. No new module dependency (go.mod unchanged). Tests: keyring round-trip/not-found/ unsupported/missing-binary/stdin-not-argv via an injected runner; store keyring round-trip + base64-at-rest + storage selection; all existing file-backend tests pass unchanged. Local run check (macOS): validated the exact `security` add/find/delete shapes and the not-found exit code (44) against the real binary, and `mcp oauth status` with ZERO_OAUTH_STORAGE=keyring reads the real keychain cleanly. Gates: gofmt/vet/build(host+linux+windows)/test -race/staticcheck(no new)/ govulncheck(0)/deadcode(no new) all pass. * all: prune unused unexported functions (dead-code sweep) Removes 15 unexported functions/types/consts that are unreachable even from tests (every one flagged by staticcheck U1000), clearing the U1000 noise the v16 audit surfaced. Conservative by design: only unexported, test-unused symbols are removed — exported internal API (which may be an intentional surface or used only by tests) is deliberately left in place. Removed: - agenteval: validateExpectedChangedFiles - providers/gemini: const providerName - tui: stripMarkdownInline, removeTrailingAtToken, noopModelSwitchCompactionPolicy (+ BeforeModelSwitch), deleteComposerLineBefore, deleteComposerLineAfter, deleteCompletedFileMentionBefore, mcpViewStatus, mcpServerLines, fitCommandOutput, (*commandPicker).clearQuery, tuiTheme.onPanel2, indentText No behavior change (the symbols had no callers). staticcheck U1000 is now empty; deadcode drops accordingly with no genuinely-new unreachable functions (verified by a name-based diff vs origin/main). gofmt/vet/build(host+linux+windows)/full test all pass. * Address review feedback on the audit backlog Resolve CodeRabbit review comments on this PR: - keyring isNotFound: match the specific not-found exit codes (macOS `security` 44 = errSecItemNotFound; `secret-tool` 1) instead of treating any non-zero exit as "no entry", so a real failure (locked keychain, permissions) surfaces instead of being masked as absent. - swarm scheduler daily_at: recompute each fire as the next local HH:MM rather than adding a fixed 24h, so a wall-clock daily schedule does not drift across DST. Adds Daily/Hour/Minute to Schedule and a clock to the Scheduler. - swarm scheduler: reject Add when the scheduler context is already canceled (not just when closed); re-check cancellation after a tick so a tick+cancel race can't spawn one extra member after Cancel/Close. - swarm schedule_tool swarmInt: reject non-integer / non-finite JSON numbers (max_runs=1.9 errors instead of silently truncating to 1). - mcp migrateLegacy: resolve a relative LegacyPath to absolute before the same-file guard so it can't be bypassed by a relative spelling. - oauth keyring store: serialize the keyring read-modify-write with a cross-process lock file (best-effort, beside the config dir) so concurrent processes don't clobber each other's tokens; previously withLock was a no-op. Adds tests for the non-not-found keyring error, the daily delay recompute, the Add-after-cancel guard, and non-integer swarmInt. * swarm: make nextDailyDelay roll the day, not add 24h (DST) CodeRabbit follow-up: the run loop already recomputes a daily job's delay each cycle, but nextDailyDelay itself rolled "passed today" to the next occurrence with Add(24*time.Hour). On a DST changeover day (23h/25h) that fires an hour early/late. Use time.Date(..., day+1, hour, minute, ...) so Go normalizes the date and applies the correct local offset, holding the wall-clock HH:MM across DST. Adds an America/New_York spring-forward test (skips if tzdata is absent).	2026-06-16 14:48:12 +05:30