Compare commits

..
62 changed files with 2283 additions and 5697 deletions

No files matched your search

+2 -3
View File
@@ -302,9 +302,8 @@ Conceptually, one requested tool goes through these phases:
the model can inspect it and decide what to do next.
6. **Render and persist** — the result is surfaced to the TUI, stream-JSON
writer, or ACP client. TUI and exec record rich tool/session events for
resume/history; ACP persists conversational messages and completed tool-call
starts/results for load replay. Live streaming details and permission
exchanges remain client notifications rather than durable history.
resume/history; ACP currently persists conversational messages and streams
tool/permission updates to the client.
Tool success and failure are both informative context. A successful `read_file`
result gives the model file contents; a failed `edit_file` result gives the model
+54 -546
View File
@@ -2,14 +2,10 @@ package acp
import (
"context"
"crypto/sha256"
"encoding/base64"
"encoding/json"
"errors"
"fmt"
"log"
"os"
"path/filepath"
"strings"
"sync"
@@ -44,12 +40,8 @@ type Deps struct {
// ResolveWorkspaceRoot validates + normalizes a client-supplied cwd (must be an
// existing directory; never the bare root). It is the file-tool confinement root.
ResolveWorkspaceRoot func(cwd string) (string, error)
// PersistEvents is an optional test seam for injecting event-store failures.
// It receives a COMPLETED TURN as one batch, never one event at a time: the
// batch is the unit the shared store keeps contiguous (see persistTurn).
PersistEvents func(sessionID string, inputs []sessions.AppendEventInput) error
Store *sessions.Store
AgentInfo Implementation
Store *sessions.Store
AgentInfo Implementation
}
// Workspace is the per-turn execution environment passed to the agent. ACP does
@@ -114,8 +106,6 @@ func NewAgent(conn *Conn, deps Deps) *Agent {
conn.Handle(MethodInitialize, a.handleInitialize)
conn.Handle(MethodSessionNew, a.handleSessionNew)
conn.Handle(MethodSessionLoad, a.handleSessionLoad)
conn.Handle(MethodSessionList, a.handleSessionList)
conn.Handle(MethodSessionResume, a.handleSessionResume)
conn.Handle(MethodSessionPrompt, a.handleSessionPrompt)
conn.Handle(MethodSessionSetMode, a.handleSetMode)
conn.Handle(MethodSessionSetConfigOption, a.handleSetConfigOption)
@@ -144,11 +134,11 @@ func (a *Agent) handleInitialize(_ context.Context, params json.RawMessage) (any
return InitializeResult{
ProtocolVersion: negotiated,
AgentCapabilities: AgentCapabilities{
// ACP v1 optional methods are advertised as empty capability objects;
// clients must gate session/list and session/resume on their presence.
LoadSession: true,
PromptCapabilities: PromptCapabilities{Image: true},
SessionCapabilities: &SessionCapabilities{List: &struct{}{}, Resume: &struct{}{}},
// Only advertise what ZERO actually implements: session/load (loadSession)
// and image prompts. session/resume + the session-capability sub-object
// are intentionally omitted since there is no resume handler yet.
LoadSession: true,
PromptCapabilities: PromptCapabilities{Image: true},
},
AgentInfo: &info,
// ZERO owns credentials (BYOK) and does not delegate auth to the editor.
@@ -158,41 +148,12 @@ func (a *Agent) handleInitialize(_ context.Context, params json.RawMessage) (any
// ---- session lifecycle ----
// requestedWorkspace is the single rule for a cwd that arrived from the client,
// applied before the value can reach ResolveWorkspaceRoot.
//
// THE REQUEST'S OWN CWD IS THE ONLY THING ALLOWED TO PICK A ROOT. Every params
// type spells cwd as a plain string, so an absent field and an empty one decode
// to exactly the same "", and the resolver treats "" as "use the directory this
// process was started in" and joins a relative cwd onto it. Those two defaults
// meet in the middle: `{"sessionId":"known"}` used to activate a persisted
// session, and `{}` used to create one, both rooted at wherever the editor
// happened to spawn `zero acp` — which then becomes the sandbox and file-tool
// confinement root. The client never named that directory, so it is a root the
// server invented, and ACP requires an absolute cwd on every method carrying
// one. Rejecting here is what keeps a missing field from being answered with a
// guess.
func requestedWorkspace(cwd string) (string, error) {
trimmed := strings.TrimSpace(cwd)
if trimmed == "" {
return "", RPCError(codeInvalidParams, "cwd is required and must be an absolute path")
}
if !filepath.IsAbs(trimmed) {
return "", RPCError(codeInvalidParams, "cwd must be an absolute path: "+trimmed)
}
return trimmed, nil
}
func (a *Agent) handleSessionNew(ctx context.Context, params json.RawMessage) (any, error) {
var p NewSessionParams
if err := json.Unmarshal(params, &p); err != nil {
return nil, RPCError(codeInvalidParams, "invalid session/new params")
}
cwd, err := requestedWorkspace(p.Cwd)
if err != nil {
return nil, err
}
root, err := a.deps.ResolveWorkspaceRoot(cwd)
root, err := a.deps.ResolveWorkspaceRoot(p.Cwd)
if err != nil {
return nil, RPCError(codeInvalidParams, err.Error())
}
@@ -204,7 +165,7 @@ func (a *Agent) handleSessionNew(ctx context.Context, params json.RawMessage) (a
if err != nil {
return nil, RPCError(codeInternalError, "create session: "+err.Error())
}
sess, _ := a.registerSession(meta.SessionID, root, nil, model, models, restrictModels)
sess := a.registerSession(meta.SessionID, root, nil, model, models, restrictModels)
return NewSessionResult{
SessionID: sess.id,
ConfigOptions: a.configOptions(sess),
@@ -217,90 +178,22 @@ func (a *Agent) handleSessionLoad(ctx context.Context, params json.RawMessage) (
if err := json.Unmarshal(params, &p); err != nil {
return nil, RPCError(codeInvalidParams, "invalid session/load params")
}
return a.activatePersistedSession(ctx, p, persistedSessionLoad)
}
func (a *Agent) handleSessionResume(ctx context.Context, params json.RawMessage) (any, error) {
var p ResumeSessionParams
if err := json.Unmarshal(params, &p); err != nil {
return nil, RPCError(codeInvalidParams, "invalid session/resume params")
}
return a.activatePersistedSession(ctx, p, persistedSessionResume)
}
type persistedSessionOperation uint8
const (
persistedSessionLoad persistedSessionOperation = iota
persistedSessionResume
)
// activatePersistedSession restores the agent's internal conversation context
// for both lifecycle methods. session/load additionally replays user-visible
// history as ordered session/update notifications; session/resume deliberately
// does not, which makes it safe for an already-rendered desktop reconnect.
func (a *Agent) activatePersistedSession(ctx context.Context, p LoadSessionParams, operation persistedSessionOperation) (any, error) {
// BOTH METHODS, BEFORE ANYTHING IS LOOKED UP. session/resume shares this
// params type with session/load, so the wire cannot tell an omitted cwd from
// an empty one; the blank case used to be answered with the persisted cwd,
// which made {"sessionId":"known"} a complete, successful activation request.
// Validating here rather than in one handler is what keeps the two entry
// points from drifting apart.
cwd, err := requestedWorkspace(p.Cwd)
if err != nil {
return nil, err
}
meta, err := a.deps.Store.Get(p.SessionID)
if err != nil || meta == nil {
return nil, RPCError(codeInvalidParams, "session not found: "+p.SessionID)
}
if operation == persistedSessionResume && !sessions.IsResumableKind(meta.SessionKind) {
return nil, RPCError(codeInvalidParams, "session is not resumable: "+p.SessionID)
cwdInput := p.Cwd
if strings.TrimSpace(cwdInput) == "" {
cwdInput = meta.Cwd
}
if strings.TrimSpace(meta.Cwd) == "" {
return nil, RPCError(codeInvalidParams, "session has no persisted workspace: "+p.SessionID)
}
// SAME RULE ON THE WAY IN. Omitting a relative entry from the listing is not
// enough: a stored "." still resolves against this process's directory, so a
// client that hands back that same directory as its cwd would be sold the
// invented root as a match. A workspace that cannot be identified is not one
// this session can be restored into.
if !filepath.IsAbs(meta.Cwd) {
return nil, RPCError(codeInvalidParams, "session workspace is not an absolute path, so it cannot be identified: "+p.SessionID)
}
persistedRoot, err := a.deps.ResolveWorkspaceRoot(meta.Cwd)
if err != nil {
return nil, RPCError(codeInvalidParams, "persisted session workspace is unavailable: "+err.Error())
}
root, err := a.deps.ResolveWorkspaceRoot(cwd)
root, err := a.deps.ResolveWorkspaceRoot(cwdInput)
if err != nil {
return nil, RPCError(codeInvalidParams, err.Error())
}
// ACP session cwd is immutable. Loading history under a different root
// would give a conversation from one workspace access to another
// workspace's configuration, files, and tools.
if !sameWorkspace(root, persistedRoot) {
return nil, RPCError(codeInvalidParams, "session cwd does not match its persisted workspace")
}
// Load history BEFORE publishing the session so no concurrent prompt observes
// a half-initialized session (registerSession sets history under the lock and
// reuses an already-live session rather than orphaning its in-flight turn).
requireHistoryLog := operation == persistedSessionResume && meta.EventCount > 0
history, messages, historyWarning, historyErr := a.loadHistory(meta.SessionID, requireHistoryLog)
// RESUME PROMISES RESTORED CONTEXT, SO A FAILED RESTORATION IS A FAILED
// RESUME. historyErr used to do nothing but suppress replay and raise a
// warning: the session was registered and reported ready regardless, so a
// corrupt record or an unreadable events file left the caller holding a live,
// promptable session ID whose next prompt ran as a brand-new conversation
// under the old identity. Nothing in the response said so.
//
// Publication is now gated on restoration for resume. LOAD keeps the
// best-effort policy deliberately rather than by inheriting this helper: it
// replays what it has and warns about the rest, which is the right trade for
// a client rebuilding a transcript it can still scroll. Reported by @jatmn.
if operation == persistedSessionResume && historyErr != nil {
return nil, RPCError(codeInternalError, "restore session history: "+historyErr.Error())
}
history, historyErr := a.loadHistory(meta.SessionID)
model, models, restrictModels, err := a.resolveModelChoices(ctx, root)
if err != nil {
return nil, RPCError(codeInternalError, "config: "+err.Error())
@@ -311,36 +204,12 @@ func (a *Agent) activatePersistedSession(ctx context.Context, p LoadSessionParam
models = append(models, SessionConfigOptionValue{Value: persistedModel, Name: persistedModel})
}
}
sess, existed := a.registerSession(meta.SessionID, root, history, model, models, restrictModels)
if existed {
_, messages, historyWarning, historyErr = a.refreshSessionHistory(sess, requireHistoryLog)
if operation == persistedSessionResume && historyErr != nil {
return nil, RPCError(codeInternalError, "restore session history: "+historyErr.Error())
}
}
note := &notifier{conn: a.conn, sessionID: sess.id}
if operation == persistedSessionLoad && historyErr == nil {
for _, message := range messages {
if message.tool != nil {
note.send(*message.tool)
continue
}
note.send(replayMessageChunk(message.role, replayMessageID(message.eventID), message.content))
}
}
sess := a.registerSession(meta.SessionID, root, history, model, models, restrictModels)
a.warnPersistence(
note,
"rehydrate session compaction",
"Could not apply session compaction. Raw session history was restored instead.",
historyWarning,
operation == persistedSessionLoad,
)
a.warnPersistence(
note,
&notifier{conn: a.conn, sessionID: sess.id},
"load session history",
"Could not load session history. The session is open, but earlier turns may be missing until storage recovers.",
historyErr,
operation == persistedSessionLoad,
)
return LoadSessionResult{
ConfigOptions: a.configOptions(sess),
@@ -348,80 +217,6 @@ func (a *Agent) activatePersistedSession(ctx context.Context, p LoadSessionParam
}, nil
}
func (a *Agent) handleSessionList(_ context.Context, params json.RawMessage) (any, error) {
var p ListSessionsParams
if len(params) > 0 {
if err := json.Unmarshal(params, &p); err != nil {
return nil, RPCError(codeInvalidParams, "invalid session/list params")
}
}
if p.Cursor != "" {
return nil, RPCError(codeInvalidParams, "invalid session/list cursor")
}
items, err := a.deps.Store.ListResumable()
if err != nil {
return nil, RPCError(codeInternalError, "list sessions: "+err.Error())
}
// The filter is the one cwd ACP leaves optional, so a blank one means "no
// filter" rather than an error — but a relative one is rebased onto this
// process's directory exactly like the others, and would then silently answer
// about a workspace the client never named.
var cwd string
if strings.TrimSpace(p.Cwd) != "" {
filter, err := requestedWorkspace(p.Cwd)
if err != nil {
return nil, err
}
cwd, err = a.deps.ResolveWorkspaceRoot(filter)
if err != nil {
return nil, RPCError(codeInvalidParams, err.Error())
}
}
result := ListSessionsResult{Sessions: make([]SessionInfo, 0, len(items))}
for _, item := range items {
// EVERY ENTRY IS RESOLVED, FILTER OR NO FILTER. Listing is a menu, and
// every item on it has to be orderable: activatePersistedSession
// resolves the persisted workspace and refuses what it cannot reach, so
// anything this loop cannot resolve is something the client would be
// offered and then denied.
//
// Resolving only when a cwd filter was supplied left two shapes through —
// a session whose workspace has since been deleted, and a legacy entry
// holding a relative path, which was then reported as cwd "." even though
// ACP requires SessionInfo.cwd to be absolute.
if strings.TrimSpace(item.Cwd) == "" {
continue
}
// A RELATIVE PERSISTED CWD HAS NO RECOVERABLE IDENTITY. Resolving one
// rebases it onto wherever this ACP server happens to be running and
// advertises that invented absolute path as the session's workspace — so a
// conversation created for one project could be resumed against another
// project's configuration, files and tools. The original base is not
// knowable from the metadata, so the honest answer is to omit the entry
// rather than to guess at it.
if !filepath.IsAbs(item.Cwd) {
continue
}
itemRoot, err := a.deps.ResolveWorkspaceRoot(item.Cwd)
if err != nil {
continue
}
if cwd != "" && !sameWorkspace(itemRoot, cwd) {
continue
}
result.Sessions = append(result.Sessions, SessionInfo{
SessionID: item.SessionID,
Title: item.Title,
// The RESOLVED root, not the stored string: absolute as ACP
// requires, and the same value the client will hand back on resume.
Cwd: itemRoot,
UpdatedAt: item.UpdatedAt,
Meta: &SessionInfoMeta{ModelID: item.ModelID, CreatedAt: item.CreatedAt},
})
}
return result, nil
}
// ---- prompt turn ----
func (a *Agent) handleSessionPrompt(ctx context.Context, params json.RawMessage) (any, error) {
@@ -498,15 +293,6 @@ func (a *Agent) runTurn(ctx context.Context, sess *acpSession, userText string,
note.text("\n\n[zero warning] " + text + "\n")
}
}
// Buffer the complete turn until RunAgent has a valid ACP outcome. A hard run
// failure is an aborted turn: neither live history nor durable history may
// retain its user/tool prefix under an RPC error. StopCancelled is a valid
// outcome and commits through the same boundary below.
pendingEvents := make([]sessions.AppendEventInput, 0, 4)
queue := func(input sessions.AppendEventInput) {
pendingEvents = append(pendingEvents, input)
}
queue(messageEvent("user", userText))
opts := agent.Options{
Cwd: sess.cwd,
@@ -521,12 +307,8 @@ func (a *Agent) runTurn(ctx context.Context, sess *acpSession, userText string,
Images: images,
OnText: note.text,
OnReasoning: note.thought,
OnToolCall: func(call agent.ToolCall) {
queue(toolCallEvent(call))
note.toolCall(call)
},
OnToolCall: note.toolCall,
OnToolResult: func(result agent.ToolResult) {
queue(toolResultEvent(result))
note.toolResult(result)
if result.Name == "update_plan" {
a.emitPlan(registry, note)
@@ -539,36 +321,19 @@ func (a *Agent) runTurn(ctx context.Context, sess *acpSession, userText string,
agentPrompt := buildPrompt(sess.snapshotHistory(), userText)
result, runErr := a.deps.RunAgent(ctx, agentPrompt, provider, opts)
if result.FinalAnswer != "" {
queue(messageEvent("assistant", result.FinalAnswer))
}
reason, stopErr := stopReasonFor(result, runErr)
if stopErr != nil {
return "", RPCError(codeInternalError, stopErr.Error())
}
// ONE BATCH, ONE LOCK. The outcome gate above decides WHETHER the turn is
// committed; the store batch decides WHICH RECORDS STAY TOGETHER relative to
// other writers, and these are different guarantees. Persisting the buffer
// one event per call took and released the store's session lock between
// events, so a second ACP instance holding the same session — its turnMu is
// its own, only the store is shared — could append its whole turn in the
// gap: user A, user B, answer B, answer A. Every write was individually
// locked and the log was valid, but a fresh load pairs prose by order and
// reconstructed three wrong turns from two right ones. Before this branch,
// persistTurn handed the user and assistant events to AppendEvents together;
// buffering until the outcome is known was correct, splitting the commit
// into singleton writes was not. The store writes a batch under one lock and
// one file write, which is the contiguity restored here — not a disk
// transaction, and not a promise about crashes mid-fsync. Reported by @jatmn.
persistenceErr := a.persistTurn(sess.id, pendingEvents)
sess.appendHistory(turnRecord{user: userText, assistant: result.FinalAnswer})
a.warnPersistence(
note,
"save session history",
"Could not save session history. This turn is available in memory, but future resume may miss it until storage recovers.",
persistenceErr,
true,
)
if err := a.persistTurn(sess, userText, result.FinalAnswer); err != nil {
a.warnPersistence(
note,
"save session history",
"Could not save session history. This turn is available in memory, but future resume may miss it until storage recovers.",
err,
)
}
return reason, nil
}
@@ -827,159 +592,39 @@ func (a *Agent) configOptions(s *acpSession) []SessionConfigOption {
// ---- persistence + continuity ----
// persistTurn commits a completed turn's buffered events as one store batch, so
// no other writer's records can land between them. A failure is the failure of
// the whole batch: nothing after a failed prerequisite is appended on its own.
func (a *Agent) persistTurn(sessionID string, inputs []sessions.AppendEventInput) error {
if len(inputs) == 0 {
return nil
}
if a.deps.PersistEvents != nil {
return a.deps.PersistEvents(sessionID, inputs)
}
func (a *Agent) persistTurn(sess *acpSession, user, assistant string) error {
defer sess.appendHistory(turnRecord{user: user, assistant: assistant})
if a.deps.Store == nil {
return nil
}
_, err := a.deps.Store.AppendEvents(sessionID, inputs)
events := []sessions.AppendEventInput{
{
Type: sessions.EventMessage,
Payload: map[string]any{"role": "user", "content": user},
},
}
if assistant != "" {
events = append(events, sessions.AppendEventInput{
Type: sessions.EventMessage,
Payload: map[string]any{"role": "assistant", "content": assistant},
})
}
_, err := a.deps.Store.AppendEvents(sess.id, events)
return err
}
func messageEvent(role, content string) sessions.AppendEventInput {
return sessions.AppendEventInput{
Type: sessions.EventMessage,
Payload: map[string]any{"role": role, "content": content},
}
}
func toolCallEvent(call agent.ToolCall) sessions.AppendEventInput {
return sessions.AppendEventInput{
Type: sessions.EventToolCall,
Payload: map[string]any{
"toolCallId": call.ID,
"name": call.Name,
"arguments": call.Arguments,
},
}
}
func toolResultEvent(result agent.ToolResult) sessions.AppendEventInput {
return sessions.AppendEventInput{
Type: sessions.EventToolResult,
Payload: map[string]any{
"toolCallId": result.ToolCallID,
"name": result.Name,
"status": result.Status,
"output": result.Output,
"changedFiles": append([]string(nil), result.ChangedFiles...),
},
}
}
// persistedMessage is one entry of the restored transcript in stored order. It
// is either a message (role/content) or a tool-call notification (tool), never
// both: keeping them in ONE ordered slice is what preserves the interleaving a
// client needs to redraw the conversation as it originally happened.
type persistedMessage struct {
eventID string
role string
content string
// tool is set for a replayed tool call or its result, and is already in wire
// shape — it comes from the same toolCallStart/toolCallResult mapping the
// live turn uses, so a replayed tool call renders identically to a fresh one
// and there is no second mapping to drift out of sync.
tool *ToolCallUpdate
}
func (a *Agent) loadHistory(sessionID string, requireHistoryLog bool) ([]turnRecord, []persistedMessage, error, error) {
func (a *Agent) loadHistory(sessionID string) ([]turnRecord, error) {
if a.deps.Store == nil {
return nil, nil, nil, nil
return nil, nil
}
// THE EFFECTIVE CONVERSATION, NOT THE RAW LOG. A compacted session keeps its
// original prefix on disk alongside an EventCompaction that names the events
// it replaced and carries their durable summary. ReadEvents hands back both,
// so restoring from it replayed superseded turns AND dropped the summary that
// replaced them — resume then seeded the next turn with context the
// conversation had already moved past, and load visibly resurrected
// transcript entries the user had seen compacted away.
//
// ReadRehydratedEvents is the projection the TUI and exec paths already use
// (internal/tui/session.go, internal/sessions/exec_session.go), so all three
// now agree on what the conversation IS. Switching readers alone is not
// enough: rehydration substitutes the compaction event in place of the events
// it replaced, so a loop that skips everything but EventMessage would drop the
// summary exactly as before. It is projected below. Reported by @jatmn.
events, eventLogPresent, err := a.deps.Store.ReadRehydratedEventsWithPresence(sessionID)
var rehydrateWarning error
events, err := a.deps.Store.ReadEvents(sessionID)
if err != nil {
rehydrateWarning = err
events, eventLogPresent, err = a.deps.Store.ReadEventsWithPresence(sessionID)
if err != nil {
return nil, nil, nil, err
}
}
if requireHistoryLog && !eventLogPresent {
return nil, nil, nil, fmt.Errorf("zero session %s event log is missing", sessionID)
return nil, err
}
var records []turnRecord
var messages []persistedMessage
activeToolCalls := make(map[string]string)
var pendingUser string
havePending := false
for _, e := range events {
if e.Type == sessions.EventCompaction {
// The summary stands in for the turns it replaced, so it enters the
// history as an assistant message: it is prior context the next turn
// must see, and the wire has no separate role for it.
raw, marshalErr := json.Marshal(e.Payload)
if marshalErr != nil {
continue
}
var payload struct {
Summary string `json:"summary"`
}
if json.Unmarshal(raw, &payload) != nil || strings.TrimSpace(payload.Summary) == "" {
continue
}
messages = append(messages, persistedMessage{
eventID: persistedMessageIdentity(sessionID, e),
role: "assistant",
content: payload.Summary,
})
records = append(records, turnRecord{user: pendingUser, assistant: payload.Summary})
pendingUser = ""
havePending = false
continue
}
// TOOL ACTIVITY IS PART OF THE TRANSCRIPT, NOT DECORATION. A restored
// conversation that shows only the prose reads as if the agent asserted
// its edits and command output from nowhere: the user sees "I updated
// scope.go" with no record that any tool ran. These are collected for
// replay only. They deliberately do NOT enter turnRecord, so the prompt
// context a resumed turn receives stays byte-identical to what it was —
// load and resume continue to consume the SAME effective history, and
// only the wire rendering differs. Reported by @jatmn.
if e.Type == sessions.EventToolCall || e.Type == sessions.EventToolResult {
if upd := replayToolUpdate(e); upd != nil {
rawID := upd.ToolCallID
if e.Type == sessions.EventToolResult {
replayID, ok := activeToolCalls[rawID]
if !ok {
continue
}
upd.ToolCallID = replayID
delete(activeToolCalls, rawID)
} else {
replayID := replayToolCallID(persistedMessageIdentity(sessionID, e))
activeToolCalls[rawID] = replayID
upd.ToolCallID = replayID
}
messages = append(messages, persistedMessage{
eventID: persistedMessageIdentity(sessionID, e),
tool: upd,
})
}
continue
}
if e.Type != sessions.EventMessage {
continue
}
@@ -996,14 +641,12 @@ func (a *Agent) loadHistory(sessionID string, requireHistoryLog bool) ([]turnRec
}
switch msg.Role {
case "user":
messages = append(messages, persistedMessage{eventID: persistedMessageIdentity(sessionID, e), role: msg.Role, content: msg.Content})
if havePending {
records = append(records, turnRecord{user: pendingUser})
}
pendingUser = msg.Content
havePending = true
case "assistant":
messages = append(messages, persistedMessage{eventID: persistedMessageIdentity(sessionID, e), role: msg.Role, content: msg.Content})
records = append(records, turnRecord{user: pendingUser, assistant: msg.Content})
pendingUser = ""
havePending = false
@@ -1012,89 +655,10 @@ func (a *Agent) loadHistory(sessionID string, requireHistoryLog bool) ([]turnRec
if havePending {
records = append(records, turnRecord{user: pendingUser})
}
return records, messages, rehydrateWarning, nil
return records, nil
}
// replayToolUpdate rebuilds the ACP notification for one stored tool event.
//
// The stored toolCallId is returned here as a correlation key. loadHistory maps
// each call occurrence to a stable session-wide replay identity and applies that
// identity to its result. Older records wrote the field as "id", so both
// spellings are accepted — the TUI projection already does the same
// (internal/tui/session.go).
//
// A record that carries no usable id is skipped rather than replayed under a
// synthesized one: an unpairable update is worse than a missing row.
func replayToolUpdate(event sessions.Event) *ToolCallUpdate {
raw, err := json.Marshal(event.Payload)
if err != nil {
return nil
}
var payload struct {
Name string `json:"name"`
ToolCallID string `json:"toolCallId"`
ID string `json:"id"`
Arguments string `json:"arguments"`
Status string `json:"status"`
Output string `json:"output"`
ChangedFiles []string `json:"changedFiles"`
}
if json.Unmarshal(raw, &payload) != nil {
return nil
}
id := payload.ToolCallID
if id == "" {
id = payload.ID
}
if id == "" {
return nil
}
if event.Type == sessions.EventToolCall {
upd := toolCallStart(agent.ToolCall{ID: id, Name: payload.Name, Arguments: payload.Arguments})
return &upd
}
status := tools.Status(payload.Status)
if status == "" {
status = tools.StatusOK
}
upd := toolCallResult(agent.ToolResult{
ToolCallID: id,
Name: payload.Name,
Status: status,
Output: payload.Output,
ChangedFiles: append([]string(nil), payload.ChangedFiles...),
})
return &upd
}
func persistedMessageIdentity(sessionID string, event sessions.Event) string {
if event.ID != "" {
return event.ID
}
return fmt.Sprintf("%s:%d", sessionID, event.Sequence)
}
// replayMessageID maps ZERO's stable event identity to a standards-shaped UUID
// without leaking or parsing the event id on the wire. The same stored message
// receives the same opaque id across loads, which also gives clients an exact
// chunk boundary when two adjacent persisted messages have the same role.
func replayMessageID(eventID string) string {
return replayOpaqueID("message", eventID)
}
func replayToolCallID(eventID string) string {
return replayOpaqueID("tool-call", eventID)
}
func replayOpaqueID(kind, eventID string) string {
sum := sha256.Sum256([]byte("zero-acp-" + kind + ":" + eventID))
b := sum[:16]
b[6] = (b[6] & 0x0f) | 0x50
b[8] = (b[8] & 0x3f) | 0x80
return fmt.Sprintf("%x-%x-%x-%x-%x", b[:4], b[4:6], b[6:8], b[8:10], b[10:16])
}
func (a *Agent) warnPersistence(note *notifier, action string, message string, err error, notify bool) {
func (a *Agent) warnPersistence(note *notifier, action string, message string, err error) {
if err == nil {
return
}
@@ -1103,7 +667,7 @@ func (a *Agent) warnPersistence(note *notifier, action string, message string, e
sessionID = note.sessionID
}
log.Printf("zero acp: failed to %s for session %s: %v", action, sessionID, err)
if note != nil && notify {
if note != nil {
note.text("\n\n[zero warning] " + message + "\n")
}
}
@@ -1150,34 +714,18 @@ func promptImages(blocks []ContentBlock) []zeroruntime.ImageBlock {
// registerSession publishes a session under the agent's lock. If one is already
// registered for id (e.g. a re-load of an in-flight session) the existing live
// session is returned rather than orphaning its turn or resetting its mode/model.
// The bool reports that case so persisted activation can re-read and apply disk
// history under turnMu. history is set BEFORE first publication so no concurrent
// prompt can read a half-initialized session.
func (a *Agent) registerSession(id, cwd string, history []turnRecord, model string, models []SessionConfigOptionValue, restrictModels bool) (*acpSession, bool) {
// session is returned unchanged rather than orphaning its turn or resetting its
// mode/model. history is set BEFORE publishing so no concurrent prompt can read a
// half-initialized session.
func (a *Agent) registerSession(id, cwd string, history []turnRecord, model string, models []SessionConfigOptionValue, restrictModels bool) *acpSession {
a.mu.Lock()
defer a.mu.Unlock()
if existing := a.sessions[id]; existing != nil {
return existing, true
return existing
}
sess := &acpSession{id: id, cwd: cwd, mode: agent.PermissionModeAuto, model: model, models: models, restrictModels: restrictModels, history: history}
a.sessions[id] = sess
return sess, false
}
// refreshSessionHistory is the activation apply boundary for an already-live
// session. The history is read while prompts are excluded, then applied to the
// same object the prompt path uses. Re-reading here avoids overwriting a turn
// that committed between activatePersistedSession's pre-publication read and
// discovering that another activation had already registered the session.
func (a *Agent) refreshSessionHistory(sess *acpSession, requireHistoryLog bool) ([]turnRecord, []persistedMessage, error, error) {
sess.turnMu.Lock()
defer sess.turnMu.Unlock()
history, messages, warning, err := a.loadHistory(sess.id, requireHistoryLog)
if err == nil {
sess.setHistory(history)
}
return history, messages, warning, err
return sess
}
func (a *Agent) session(id string) *acpSession {
@@ -1260,48 +808,8 @@ func (s *acpSession) appendHistory(rec turnRecord) {
s.mu.Unlock()
}
func (s *acpSession) setHistory(history []turnRecord) {
s.mu.Lock()
s.history = append([]turnRecord(nil), history...)
s.mu.Unlock()
}
func (s *acpSession) snapshotHistory() []turnRecord {
s.mu.Lock()
defer s.mu.Unlock()
return append([]turnRecord(nil), s.history...)
}
// sameWorkspace reports whether two resolved roots name the same directory.
//
// STRING EQUALITY IS NOT DIRECTORY IDENTITY. ResolveWorkspaceRoot is abs plus
// filepath.Clean plus a stat: it does not fold case and does not resolve
// junctions, so one directory reached by two spellings produces two different
// roots. A session persisted from the TUI was then unresumable from an editor
// holding a different spelling of the same project folder, and session/list
// filtered by the other spelling returned nothing — not a failed resume but an
// invisible one, on exactly the case this feature exists for.
//
// filepath.EvalSymlinks is NOT the fix on Windows: it normalises a drive letter
// but returns a junction path unchanged, so the alias survives it. Junctions need
// no privilege, so this is ordinary rather than exotic. os.SameFile compares the
// filesystem's own identity for the two directories, which is the question being
// asked.
//
// The string comparison stays as the fast path, and a stat failure falls back to
// it rather than widening the match — this gate refuses access to another
// workspace's files and configuration, so an unanswerable comparison denies.
func sameWorkspace(left, right string) bool {
if left == right {
return true
}
leftInfo, err := os.Stat(left)
if err != nil {
return false
}
rightInfo, err := os.Stat(right)
if err != nil {
return false
}
return os.SameFile(leftInfo, rightInfo)
}
+13 -1684
View File
File diff suppressed because it is too large. Load diff
+10 -43
View File
@@ -89,13 +89,6 @@ type Conn struct {
handlers map[string]HandlerFunc
notifiers map[string]NotifyFunc
// Notifications for one method are an ordered stream: session/update in
// particular is stateful, so dispatching every frame in an unrelated
// goroutine can make a later chunk overtake an earlier one. Different
// methods retain independent tails, keeping session/cancel responsive while
// an update handler is busy.
notifyMu sync.Mutex
notifyTails map[string]chan struct{}
mu sync.Mutex
nextID int64
@@ -113,12 +106,11 @@ type Conn struct {
// lifetime.
func NewConn(r io.Reader, w io.Writer) *Conn {
return &Conn{
rawReader: r,
w: w,
handlers: make(map[string]HandlerFunc),
notifiers: make(map[string]NotifyFunc),
notifyTails: make(map[string]chan struct{}),
pending: make(map[int64]chan rpcMessage),
rawReader: r,
w: w,
handlers: make(map[string]HandlerFunc),
notifiers: make(map[string]NotifyFunc),
pending: make(map[int64]chan rpcMessage),
}
}
@@ -311,7 +303,11 @@ func (c *Conn) handleLine(ctx context.Context, line []byte) {
}(msg)
case msg.isNotify():
if fn := c.notifiers[msg.Method]; fn != nil {
c.dispatchNotification(ctx, msg, fn)
c.wg.Add(1)
go func(m rpcMessage) {
defer c.wg.Done()
fn(ctx, m.Params)
}(msg)
}
default:
// Malformed frame; reply only if we can identify a request id.
@@ -321,35 +317,6 @@ func (c *Conn) handleLine(ctx context.Context, line []byte) {
}
}
// dispatchNotification preserves wire order within one method without making
// unrelated notification methods wait for each other. The read loop installs
// each tail before starting its goroutine, so goroutine scheduling cannot
// reorder the chain it observes.
func (c *Conn) dispatchNotification(ctx context.Context, msg rpcMessage, fn NotifyFunc) {
c.notifyMu.Lock()
previous := c.notifyTails[msg.Method]
done := make(chan struct{})
c.notifyTails[msg.Method] = done
c.notifyMu.Unlock()
c.wg.Add(1)
go func() {
defer c.wg.Done()
defer func() {
close(done)
c.notifyMu.Lock()
if c.notifyTails[msg.Method] == done {
delete(c.notifyTails, msg.Method)
}
c.notifyMu.Unlock()
}()
if previous != nil {
<-previous
}
fn(ctx, msg.Params)
}()
}
func (c *Conn) dispatchRequest(ctx context.Context, msg rpcMessage) {
fn := c.handlers[msg.Method]
if fn == nil {
-80
View File
@@ -360,86 +360,6 @@ func TestConnNotification(t *testing.T) {
}
}
func TestConnNotificationsForOneMethodStayInWireOrder(t *testing.T) {
a, b, stop := connPair(t)
defer stop()
firstStarted := make(chan struct{})
releaseFirst := make(chan struct{})
delivered := make(chan int, 2)
b.HandleNotify("update", func(_ context.Context, params json.RawMessage) {
var in struct{ Sequence int }
_ = json.Unmarshal(params, &in)
if in.Sequence == 1 {
close(firstStarted)
<-releaseFirst
}
delivered <- in.Sequence
})
if err := a.Notify("update", map[string]int{"Sequence": 1}); err != nil {
t.Fatalf("first notify: %v", err)
}
select {
case <-firstStarted:
case <-time.After(2 * time.Second):
t.Fatal("first notification did not start")
}
if err := a.Notify("update", map[string]int{"Sequence": 2}); err != nil {
t.Fatalf("second notify: %v", err)
}
select {
case sequence := <-delivered:
t.Fatalf("notification %d overtook the blocked first notification", sequence)
case <-time.After(25 * time.Millisecond):
}
close(releaseFirst)
for want := 1; want <= 2; want++ {
select {
case got := <-delivered:
if got != want {
t.Fatalf("notification order = ...,%d; want %d", got, want)
}
case <-time.After(2 * time.Second):
t.Fatalf("notification %d was not delivered", want)
}
}
}
func TestConnDifferentNotificationMethodsStayConcurrent(t *testing.T) {
a, b, stop := connPair(t)
defer stop()
updateStarted := make(chan struct{})
releaseUpdate := make(chan struct{})
cancelDelivered := make(chan struct{})
b.HandleNotify("update", func(context.Context, json.RawMessage) {
close(updateStarted)
<-releaseUpdate
})
b.HandleNotify("cancel", func(context.Context, json.RawMessage) {
close(cancelDelivered)
})
if err := a.Notify("update", nil); err != nil {
t.Fatalf("update notify: %v", err)
}
select {
case <-updateStarted:
case <-time.After(2 * time.Second):
t.Fatal("update notification did not start")
}
if err := a.Notify("cancel", nil); err != nil {
t.Fatalf("cancel notify: %v", err)
}
select {
case <-cancelDelivered:
case <-time.After(2 * time.Second):
t.Fatal("different notification method was blocked behind update")
}
close(releaseUpdate)
}
func TestConnMethodNotFound(t *testing.T) {
a, _, stop := connPair(t)
defer stop()
+43 -13
View File
@@ -3,6 +3,7 @@ package acp
import (
"encoding/json"
"net/url"
"path/filepath"
"strings"
"unicode"
"unicode/utf8"
@@ -19,14 +20,6 @@ func agentMessageChunk(delta string) ContentChunk {
return ContentChunk{SessionUpdate: UpdateAgentMessageChunk, Content: TextBlock(delta)}
}
func replayMessageChunk(role, messageID, text string) ContentChunk {
update := UpdateAgentMessageChunk
if role == "user" {
update = UpdateUserMessageChunk
}
return ContentChunk{SessionUpdate: update, MessageID: messageID, Content: TextBlock(text)}
}
func agentThoughtChunk(delta string) ContentChunk {
return ContentChunk{SessionUpdate: UpdateAgentThoughtChunk, Content: TextBlock(delta)}
}
@@ -246,22 +239,59 @@ func toolCallResult(result agent.ToolResult) ToolCallUpdate {
}
func toolResultContent(result agent.ToolResult) []ToolCallContent {
content := make([]ToolCallContent, 0, 1+len(result.FileDiffs))
text := strings.TrimRight(result.Output, "\n")
if text == "" {
text = result.Display.Summary
}
if text == "" {
return nil
return appendToolResultDiffs(content, result.FileDiffs)
}
return []ToolCallContent{ToolContent(TextBlock(text))}
content = append(content, ToolContent(TextBlock(text)))
return appendToolResultDiffs(content, result.FileDiffs)
}
func appendToolResultDiffs(content []ToolCallContent, diffs []tools.FileDiff) []ToolCallContent {
for _, diff := range diffs {
if !filepath.IsAbs(diff.Path) || (!diff.OldExists && !diff.NewExists) {
continue
}
var oldText *string
if diff.OldExists {
old := diff.OldText
oldText = &old
}
var newText *string
if diff.NewExists {
updated := diff.NewText
newText = &updated
}
content = append(content, ToolCallContent{Type: "diff", Path: diff.Path, OldText: oldText, NewText: newText})
}
return content
}
func toolResultLocations(result agent.ToolResult) []ToolCallLocation {
locs := make([]ToolCallLocation, 0, len(result.ChangedFiles))
for _, f := range result.ChangedFiles {
if strings.TrimSpace(f) == "" {
locs := make([]ToolCallLocation, 0, len(result.FileDiffs)+len(result.ChangedFiles))
seen := make(map[string]bool, len(result.FileDiffs)+len(result.ChangedFiles))
for _, diff := range result.FileDiffs {
path := diff.Path
if path == "" || seen[path] {
continue
}
seen[path] = true
locs = append(locs, ToolCallLocation{Path: path})
}
// FileDiff.Path is canonical absolute path data while ChangedFiles is
// normally workspace-relative. Without the trusted workspace root these
// coordinate systems cannot be correlated safely: a suffix match would let
// /workspace/sub/a.go consume the fallback for a distinct root a.go. The
// shared seen set deduplicates only identities already exactly comparable.
for _, f := range result.ChangedFiles {
if f == "" || seen[f] {
continue
}
seen[f] = true
locs = append(locs, ToolCallLocation{Path: f})
}
return locs
+193 -3
View File
@@ -2,6 +2,7 @@ package acp
import (
"encoding/json"
"path/filepath"
"strings"
"testing"
"unicode"
@@ -300,21 +301,26 @@ func TestToolCallStart(t *testing.T) {
}
func TestToolCallResult(t *testing.T) {
path := filepath.Join(t.TempDir(), "a.go")
ok := toolCallResult(agent.ToolResult{
ToolCallID: "tc1",
Name: "edit_file",
Status: tools.StatusOK,
Output: "applied\n",
ChangedFiles: []string{"a.go", ""},
FileDiffs: []tools.FileDiff{{Path: path, OldExists: true, NewExists: true, OldText: "before\n", NewText: "after\n"}},
})
if ok.SessionUpdate != UpdateToolCallUpdate || ok.Status != ToolStatusCompleted {
t.Fatalf("unexpected ok result: %+v", ok)
}
if len(ok.Content) != 1 || ok.Content[0].Type != "content" || ok.Content[0].Content.Text != "applied" {
if len(ok.Content) != 2 || ok.Content[0].Type != "content" || ok.Content[0].Content.Text != "applied" {
t.Fatalf("unexpected content: %+v", ok.Content)
}
if len(ok.Locations) != 1 || ok.Locations[0].Path != "a.go" {
t.Fatalf("blank changed files should be dropped, got %+v", ok.Locations)
if diff := ok.Content[1]; diff.Type != "diff" || diff.Path != path || diff.OldText == nil || *diff.OldText != "before\n" || diff.NewText == nil || *diff.NewText != "after\n" {
t.Fatalf("unexpected diff content: %+v", diff)
}
if len(ok.Locations) != 2 || ok.Locations[0].Path != path || ok.Locations[1].Path != "a.go" {
t.Fatalf("unproven absolute/relative aliases must both remain visible, got %+v", ok.Locations)
}
failed := toolCallResult(agent.ToolResult{ToolCallID: "tc2", Status: tools.StatusError, Output: "boom"})
@@ -323,6 +329,190 @@ func TestToolCallResult(t *testing.T) {
}
}
func TestToolCallDiffJSONDistinguishesEmptyFilesAndDeletion(t *testing.T) {
path := filepath.Join(t.TempDir(), "empty.txt")
content := appendToolResultDiffs(nil, []tools.FileDiff{
{Path: path, OldExists: false, NewExists: true, NewText: ""},
{Path: path, OldExists: true, NewExists: true, OldText: "before", NewText: ""},
{Path: path, OldExists: true, NewExists: false, OldText: "before"},
})
if len(content) != 3 {
t.Fatalf("diff content = %#v", content)
}
for index, diff := range content {
encoded, err := json.Marshal(diff)
if err != nil {
t.Fatal(err)
}
var wire map[string]any
if err := json.Unmarshal(encoded, &wire); err != nil {
t.Fatal(err)
}
if wire["path"] != path {
t.Fatalf("wire diff %d = %s", index, encoded)
}
switch index {
case 0:
oldText, present := wire["oldText"]
if !present || oldText != nil || wire["newText"] != "" {
t.Fatalf("create diff = %s, want null oldText and empty newText", encoded)
}
case 1:
if wire["oldText"] != "before" || wire["newText"] != "" {
t.Fatalf("empty replacement diff = %s", encoded)
}
case 2:
newText, present := wire["newText"]
if wire["oldText"] != "before" || !present || newText != nil {
t.Fatalf("deletion diff = %s, want oldText and null newText", encoded)
}
}
}
}
func TestToolResultLocationsPreserveDistinctPathIdentities(t *testing.T) {
root := t.TempDir()
rootPath := filepath.Join(root, "a.go")
nestedPath := filepath.Join(root, "sub", "a.go")
diff := func(path string) tools.FileDiff {
return tools.FileDiff{Path: path, OldExists: true, NewExists: true, OldText: "before", NewText: "after"}
}
for _, tc := range []struct {
name string
diffs []tools.FileDiff
want []string
}{
{name: "both rich", diffs: []tools.FileDiff{diff(rootPath), diff(nestedPath)}, want: []string{rootPath, nestedPath, "a.go", filepath.Join("sub", "a.go")}},
{name: "root rich", diffs: []tools.FileDiff{diff(rootPath)}, want: []string{rootPath, "a.go", filepath.Join("sub", "a.go")}},
{name: "nested rich", diffs: []tools.FileDiff{diff(nestedPath)}, want: []string{nestedPath, "a.go", filepath.Join("sub", "a.go")}},
} {
t.Run(tc.name, func(t *testing.T) {
locations := toolResultLocations(agent.ToolResult{
ChangedFiles: []string{"a.go", filepath.Join("sub", "a.go")},
FileDiffs: tc.diffs,
})
if len(locations) != len(tc.want) {
t.Fatalf("locations = %#v, want %#v", locations, tc.want)
}
for index := range tc.want {
if locations[index].Path != tc.want[index] {
t.Fatalf("locations = %#v, want %#v", locations, tc.want)
}
}
})
}
}
func TestToolCallResultPreservesWhitespaceInFilePaths(t *testing.T) {
relativePath := " report.txt "
absolutePath := filepath.Join(t.TempDir(), relativePath)
update := toolCallResult(agent.ToolResult{
ChangedFiles: []string{relativePath},
FileDiffs: []tools.FileDiff{{
Path: absolutePath, OldExists: true, NewExists: true, OldText: "before", NewText: "after",
}},
})
if len(update.Content) != 1 || update.Content[0].Path != absolutePath {
t.Fatalf("diff content path = %#v, want %q", update.Content, absolutePath)
}
if len(update.Locations) != 2 || update.Locations[0].Path != absolutePath || update.Locations[1].Path != relativePath {
t.Fatalf("locations = %#v, want exact paths %q and %q", update.Locations, absolutePath, relativePath)
}
}
func TestToolResultLocationsDeduplicateOnlyExactPaths(t *testing.T) {
path := filepath.Join(t.TempDir(), "a.go")
locations := toolResultLocations(agent.ToolResult{
ChangedFiles: []string{path, path},
FileDiffs: []tools.FileDiff{{Path: path, OldExists: true, NewExists: true, OldText: "before", NewText: "after"}},
})
if len(locations) != 1 || locations[0].Path != path {
t.Fatalf("exact duplicate locations = %#v", locations)
}
}
func TestDeletedFileEmitsDiffAndKeepsLocations(t *testing.T) {
relativePath := "deleted.go"
absolutePath := filepath.Join(t.TempDir(), relativePath)
update := toolCallResult(agent.ToolResult{
ChangedFiles: []string{relativePath},
FileDiffs: []tools.FileDiff{{
Path: absolutePath, OldExists: true, NewExists: false, OldText: "before",
}},
})
if len(update.Content) != 1 || update.Content[0].OldText == nil || *update.Content[0].OldText != "before" || update.Content[0].NewText != nil {
t.Fatalf("deleted file diff = %#v, want oldText with null newText", update.Content)
}
if len(update.Locations) != 2 || update.Locations[0].Path != absolutePath || update.Locations[1].Path != relativePath {
t.Fatalf("deleted file locations = %#v", update.Locations)
}
}
func TestToolCallResultEmitsOnlyRedactedFileDiffs(t *testing.T) {
secret := "sk-proj-abcdefghijklmnopqrstuvwxyz"
path := filepath.Join(t.TempDir(), "secret.txt")
scrubbed := tools.ScrubResultSecrets(tools.Result{FileDiffs: []tools.FileDiff{{
Path: path, OldExists: true, NewExists: true, OldText: "token=" + secret, NewText: "safe",
}}})
update := toolCallResult(agent.ToolResult{ToolCallID: "call", Status: tools.StatusError, FileDiffs: scrubbed.FileDiffs})
if len(update.Content) != 1 || update.Content[0].OldText == nil || strings.Contains(*update.Content[0].OldText, secret) {
t.Fatalf("ACP content leaked unredacted diff: %#v", update.Content)
}
}
func TestToolCallResultOmitsSemanticallyUnchangedRedactedDiff(t *testing.T) {
oldSecret := "ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
newSecret := "ghp_9876543210ZYXWVUTSRQPONMLKJIHGFEDCBA"
scrubbed := tools.ScrubResultSecrets(tools.Result{
ChangedFiles: []string{"credentials.txt"},
FileDiffs: []tools.FileDiff{{
Path: filepath.Join(t.TempDir(), "credentials.txt"), OldExists: true, NewExists: true,
OldText: "token=" + oldSecret, NewText: "token=" + newSecret,
}},
})
update := toolCallResult(agent.ToolResult{
ToolCallID: "call", Status: tools.StatusOK,
ChangedFiles: scrubbed.ChangedFiles, FileDiffs: scrubbed.FileDiffs,
})
if len(update.Content) != 0 {
t.Fatalf("ACP emitted semantically unchanged redacted diff: %#v", update.Content)
}
if len(update.Locations) != 1 || update.Locations[0].Path != "credentials.txt" {
t.Fatalf("ACP path fallback = %#v", update.Locations)
}
}
func TestToolCallResultOmitsDefaultIgnorableSplitSecretsOnEitherSide(t *testing.T) {
secret := "sk-ant-api03-AAAABBBBCCCCDDDDEEEEFFFFGGGG"
for name, separator := range map[string]string{
"combining grapheme joiner": "\u034f",
"variation selector": "\ufe0f",
} {
for _, side := range []string{"old", "new"} {
t.Run(name+" "+side, func(t *testing.T) {
obfuscated := secret[:20] + separator + secret[20:]
diff := tools.FileDiff{
Path: filepath.Join(t.TempDir(), "secret.txt"), OldExists: true, NewExists: true,
OldText: "safe old", NewText: "safe new",
}
if side == "old" {
diff.OldText = obfuscated
} else {
diff.NewText = obfuscated
}
scrubbed := tools.ScrubResultSecrets(tools.Result{FileDiffs: []tools.FileDiff{diff}})
if !scrubbed.Redacted || len(scrubbed.FileDiffs) != 0 {
t.Fatalf("registry boundary retained an obfuscated secret: %#v", scrubbed)
}
update := toolCallResult(agent.ToolResult{ToolCallID: "call", Status: tools.StatusOK, FileDiffs: scrubbed.FileDiffs})
if len(update.Content) != 0 {
t.Fatalf("ACP content retained an obfuscated secret: %#v", update.Content)
}
})
}
}
}
func TestPlanUpdateAndStatus(t *testing.T) {
upd := planUpdate([]tools.PlanItem{
{Content: "step a", Status: "completed"},
+29 -57
View File
@@ -12,8 +12,6 @@ const (
MethodAuthenticate = "authenticate"
MethodSessionNew = "session/new"
MethodSessionLoad = "session/load"
MethodSessionList = "session/list"
MethodSessionResume = "session/resume"
MethodSessionPrompt = "session/prompt"
MethodSessionCancel = "session/cancel" // notification
MethodSessionUpdate = "session/update" // notification (agent -> client)
@@ -64,16 +62,8 @@ type PromptCapabilities struct {
}
type AgentCapabilities struct {
LoadSession bool `json:"loadSession"`
PromptCapabilities PromptCapabilities `json:"promptCapabilities"`
SessionCapabilities *SessionCapabilities `json:"sessionCapabilities,omitempty"`
}
// Empty capability objects are presence flags in ACP v1. Pointers preserve
// the wire distinction between an advertised `{}` and an omitted capability.
type SessionCapabilities struct {
List *struct{} `json:"list,omitempty"`
Resume *struct{} `json:"resume,omitempty"`
LoadSession bool `json:"loadSession"`
PromptCapabilities PromptCapabilities `json:"promptCapabilities"`
}
type AuthMethod struct {
@@ -127,14 +117,9 @@ type McpServer struct {
}
type NewSessionParams struct {
Cwd string `json:"cwd"`
McpServers []McpServer `json:"mcpServers"`
// AdditionalDirectories is declared by the protocol and consumed nowhere in
// this repo yet. When it is wired up it must go through requestedWorkspace
// like Cwd does: these are client-supplied paths with no absoluteness rule of
// their own, which is the same shape as the resume-cwd hole that made
// {"sessionId":"known"} activate a session against this process's directory.
AdditionalDirectories []string `json:"additionalDirectories,omitempty"`
Cwd string `json:"cwd"`
McpServers []McpServer `json:"mcpServers"`
AdditionalDirectories []string `json:"additionalDirectories,omitempty"`
}
type NewSessionResult struct {
@@ -155,39 +140,6 @@ type LoadSessionResult struct {
Modes *SessionModeState `json:"modes,omitempty"`
}
// ListSessionsParams and the following types implement ACP v1 session/list.
// Transcript contents stay behind session/load; this method returns metadata
// only and leaves the optional pagination cursor opaque.
type ListSessionsParams struct {
Cwd string `json:"cwd,omitempty"`
Cursor string `json:"cursor,omitempty"`
}
type SessionInfoMeta struct {
ModelID string `json:"modelId,omitempty"`
CreatedAt string `json:"createdAt,omitempty"`
}
type SessionInfo struct {
SessionID string `json:"sessionId"`
Cwd string `json:"cwd"`
Title string `json:"title,omitempty"`
UpdatedAt string `json:"updatedAt,omitempty"`
Meta *SessionInfoMeta `json:"_meta,omitempty"`
}
type ListSessionsResult struct {
Sessions []SessionInfo `json:"sessions"`
NextCursor string `json:"nextCursor,omitempty"`
}
// session/resume takes session/load's wire shape, so its cwd is a plain string
// too and an omitted one decodes exactly like an empty one. Sharing the type is
// safe only because activatePersistedSession requires an absolute cwd from the
// request itself — nothing downstream may supply a default for either method.
type ResumeSessionParams = LoadSessionParams
type ResumeSessionResult = LoadSessionResult
// ---- prompt turn ----
type PromptParams struct {
@@ -222,7 +174,6 @@ type SessionNotification struct {
// ContentBlock under "content"; the variant is set via SessionUpdate.
type ContentChunk struct {
SessionUpdate string `json:"sessionUpdate"`
MessageID string `json:"messageId,omitempty"`
Content ContentBlock `json:"content"`
}
@@ -284,9 +235,30 @@ type ToolCallContent struct {
// type == "content"
Content *ContentBlock `json:"content,omitempty"`
// type == "diff"
Path string `json:"path,omitempty"`
OldText string `json:"oldText,omitempty"`
NewText string `json:"newText,omitempty"`
Path string `json:"path,omitempty"`
OldText *string `json:"oldText,omitempty"`
NewText *string `json:"newText,omitempty"`
}
// MarshalJSON preserves ACP's discriminated content union. A diff always has
// path, while nullable oldText/newText distinguish creation, deletion, and an
// existing file replaced with empty content. Other content variants omit all
// diff fields rather than serializing irrelevant nulls.
func (content ToolCallContent) MarshalJSON() ([]byte, error) {
if content.Type == "diff" {
return json.Marshal(struct {
Type string `json:"type"`
Path string `json:"path"`
OldText *string `json:"oldText"`
NewText *string `json:"newText"`
}{
Type: content.Type, Path: content.Path, OldText: content.OldText, NewText: content.NewText,
})
}
return json.Marshal(struct {
Type string `json:"type"`
Content *ContentBlock `json:"content,omitempty"`
}{Type: content.Type, Content: content.Content})
}
func ToolContent(block ContentBlock) ToolCallContent {
+7
View File
@@ -1527,6 +1527,7 @@ func executeToolCall(ctx context.Context, registry *tools.Registry, call ToolCal
Images: result.Images,
Redacted: result.Redacted,
ChangedFiles: result.ChangedFiles,
FileDiffs: result.FileDiffs,
ChangeSummaries: result.ChangeSummaries,
Display: result.HumanDisplay(),
Outcome: result.Outcome,
@@ -1833,6 +1834,10 @@ func runToolForUnsandboxedRetry(ctx context.Context, registry *tools.Registry, n
}
func toolResultFromPrePermissionReject(call ToolCall, result tools.Result) ToolResult {
// PrePermissionRejecter runs before Registry.RunWithOptions, so it must
// explicitly cross the same transcript/redaction boundary before its result
// can be forwarded through ACP.
result = tools.ScrubResultSecrets(result)
output, outputRedacted := scrubInterceptedOutput(result.Output)
display := result.Display
summary, summaryRedacted := scrubInterceptedOutput(display.Summary)
@@ -1860,6 +1865,7 @@ func toolResultFromPrePermissionReject(call ToolCall, result tools.Result) ToolR
Meta: meta,
Redacted: result.Redacted || outputRedacted || summaryRedacted || metaRedacted,
ChangedFiles: result.ChangedFiles,
FileDiffs: result.FileDiffs,
ChangeSummaries: result.ChangeSummaries,
Display: display,
LoadedTools: loadedToolsFromResult(meta),
@@ -2153,6 +2159,7 @@ func askUserFallbackResult(ctx context.Context, registry *tools.Registry, call T
Meta: result.Meta,
Redacted: result.Redacted,
ChangedFiles: result.ChangedFiles,
FileDiffs: result.FileDiffs,
ChangeSummaries: result.ChangeSummaries,
Display: result.HumanDisplay(),
Outcome: result.Outcome,
+17
View File
@@ -27,6 +27,23 @@ type mockProvider struct {
requests []zeroruntime.CompletionRequest
}
func TestPrePermissionRejectScrubsFileDiffs(t *testing.T) {
secret := "sk-proj-abcdefghijklmnopqrstuvwxyz"
result := toolResultFromPrePermissionReject(ToolCall{ID: "call", Name: "test"}, tools.Result{
Status: tools.StatusError,
FileDiffs: []tools.FileDiff{{
Path: filepath.Join(t.TempDir(), "secret.txt"),
OldExists: true,
NewExists: true,
OldText: "token=" + secret,
NewText: "safe",
}},
})
if len(result.FileDiffs) != 1 || strings.Contains(result.FileDiffs[0].OldText, secret) || !result.Redacted {
t.Fatalf("pre-permission FileDiff = %#v, redacted = %t", result.FileDiffs, result.Redacted)
}
}
func TestTypedExecutionOutcomeOverridesLegacySandboxHeuristics(t *testing.T) {
engine := sandbox.NewEngine(sandbox.EngineOptions{WorkspaceRoot: t.TempDir(), Policy: sandbox.DefaultPolicy()})
call := ToolCall{Name: tools.ExecCommandToolName}
+1
View File
@@ -84,6 +84,7 @@ type ToolResult struct {
Images []zeroruntime.ImageBlock
Redacted bool
ChangedFiles []string
FileDiffs []tools.FileDiff
// ChangeSummaries are non-selectable generated-tree summaries emitted by
// command execution; callers must not schedule per-file work from them.
ChangeSummaries []execution.Change
+6 -1
View File
@@ -7,6 +7,7 @@ import (
"os"
"os/exec"
"os/signal"
"path/filepath"
"strings"
"syscall"
"time"
@@ -146,7 +147,11 @@ func runDaemonStartDetached(paths daemon.Paths, stdout io.Writer, stderr io.Writ
if err != nil {
return writeAppError(stderr, err.Error(), exitCrash)
}
logFile, logPath, err := daemon.OpenRuntimeLog(paths)
if err := os.MkdirAll(filepath.Dir(paths.Socket), 0o700); err != nil {
return writeAppError(stderr, err.Error(), exitCrash)
}
logPath := filepath.Join(filepath.Dir(paths.Socket), "daemon.log")
logFile, err := os.OpenFile(logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o600)
if err != nil {
return writeAppError(stderr, err.Error(), exitCrash)
}
-23
View File
@@ -49,14 +49,6 @@ const (
exitInterrupted = 130
)
// defaultExecMaxTurns is the headless tool-turn budget when no source sets one.
// An interactive run can be continued past the shared interactive default by the
// user; an exec run has nobody to unstick it, so evals and automation would
// truncate mid-task at 80 turns. The ceiling is the already-justified bound —
// still finite against a genuinely runaway loop, which the loop's empty-turn
// guard also cuts independently of this budget.
const defaultExecMaxTurns = config.MaxTurnsCeiling
type execOutputFormat string
type execInputFormat string
@@ -328,7 +320,6 @@ func runExec(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) in
}
return writeExecProviderError(stdout, stderr, options.outputFormat, "provider_error", err.Error())
}
resolved.MaxTurns = execTurnBudget(resolved)
var displacedMaxTurns int
resolved.MaxTurns, displacedMaxTurns = applyProfileTurnBudget(execProfile, options.maxTurns, resolved.MaxTurns)
execScope, err := sandbox.NewScope(workspaceRoot, append(append([]string{}, resolved.Sandbox.AdditionalWriteRoots...), options.addDirs...))
@@ -1231,20 +1222,6 @@ func specProfileEffortFilled(effortFilled bool, specReasoningEffort string) bool
return effortFilled && strings.TrimSpace(specReasoningEffort) == ""
}
// execTurnBudget applies the headless default to the resolved turn budget: an
// interactive run can be continued past the shared default by the user, but an
// exec run has nobody to unstick it, so an unconfigured budget starts at the
// documented ceiling instead of truncating mid-task. Explicit sources
// (--max-turns, mode presets, ZERO_MAX_TURNS, config files) resolve as
// MaxTurnsSet and pass through; an exec-profile budget still wins via
// applyProfileTurnBudget afterward.
func execTurnBudget(resolved config.ResolvedConfig) int {
if resolved.MaxTurnsSet {
return resolved.MaxTurns
}
return defaultExecMaxTurns
}
// applyProfileTurnBudget decides the run's turn budget once config is resolved.
// The profile displaces the RESOLVED value, not the flag: a pinned budget
// always wins and the profile backs off entirely, while an env/config budget
-20
View File
@@ -1975,23 +1975,3 @@ func TestRunExecTopTierDeclineNoSwitch(t *testing.T) {
t.Fatalf("provider model = %q, want claude-opus-4.1", providerModels[0])
}
}
func TestExecTurnBudgetDefaultsToCeilingUnlessSet(t *testing.T) {
cases := []struct {
name string
resolved config.ResolvedConfig
want int
}{
// A headless run has no user to continue past the shared interactive
// default, so an unconfigured budget starts at the ceiling.
{"unconfigured default", config.ResolvedConfig{MaxTurns: 80, MaxTurnsSet: false}, config.MaxTurnsCeiling},
// A user who configured 80 on purpose (e.g. cost control in CI) keeps 80.
{"explicit default value", config.ResolvedConfig{MaxTurns: 80, MaxTurnsSet: true}, 80},
{"configured value passes through", config.ResolvedConfig{MaxTurns: 42, MaxTurnsSet: true}, 42},
}
for _, tc := range cases {
if got := execTurnBudget(tc.resolved); got != tc.want {
t.Fatalf("%s: execTurnBudget = %d, want %d", tc.name, got, tc.want)
}
}
}
-5
View File
@@ -166,7 +166,6 @@ func Resolve(options ResolveOptions) (ResolvedConfig, error) {
Providers: providers,
Provider: active,
MaxTurns: cfg.MaxTurns,
MaxTurnsSet: cfg.maxTurnsSet,
MCP: cfg.MCP,
Sandbox: cfg.Sandbox,
Notify: cfg.Notify,
@@ -235,7 +234,6 @@ func mergeConfig(dst *FileConfig, src FileConfig) {
}
if src.MaxTurns > 0 {
dst.MaxTurns = src.MaxTurns
dst.maxTurnsSet = true
}
for _, provider := range src.Providers {
mergeProvider(dst, provider)
@@ -296,7 +294,6 @@ func mergeProjectConfig(dst *FileConfig, src FileConfig) error {
}
if src.MaxTurns > 0 {
dst.MaxTurns = src.MaxTurns
dst.maxTurnsSet = true
}
for _, provider := range src.Providers {
candidate := providerMergeCandidate(*dst, provider)
@@ -609,7 +606,6 @@ func applyEnv(cfg *FileConfig, env map[string]string) {
n = MaxTurnsCeiling
}
cfg.MaxTurns = n
cfg.maxTurnsSet = true
}
}
@@ -730,7 +726,6 @@ func applyOverrides(cfg *FileConfig, overrides Overrides) {
}
if overrides.MaxTurns > 0 {
cfg.MaxTurns = overrides.MaxTurns
cfg.maxTurnsSet = true
}
if overrides.Sandbox.Enabled != nil {
cfg.Sandbox.Enabled = overrides.Sandbox.Enabled
-42
View File
@@ -2252,45 +2252,3 @@ func TestResolveRejectsInvalidCrossSessionInbound(t *testing.T) {
t.Fatalf("error = %v", err)
}
}
func TestResolveReportsExplicitMaxTurns(t *testing.T) {
const providerJSON = `{
"activeProvider": "p",
"providers": [{"name": "p", "provider_kind": "openai-compatible", "base_url": "https://x.example/v1", "apiKey": "k", "model": "m"}]
}`
providerWithTurns := strings.Replace(providerJSON, `"activeProvider": "p"`, `"activeProvider": "p", "maxTurns": 40`, 1)
cases := []struct {
name string
opts ResolveOptions
set bool
}{
{"built-in default only", ResolveOptions{
Overrides: Overrides{Provider: ProviderProfile{Name: "p", ProviderKind: "openai-compatible", BaseURL: "https://x.example/v1", APIKey: "k", Model: "m"}},
}, false},
{"user config", ResolveOptions{UserConfigPath: writeConfig(t, providerWithTurns)}, true},
{"project config", ResolveOptions{
UserConfigPath: writeConfig(t, providerJSON),
ProjectConfigPath: writeConfig(t, `{"maxTurns": 40}`),
}, true},
{"env", ResolveOptions{
Env: map[string]string{MaxTurnsEnv: "120"},
Overrides: Overrides{Provider: ProviderProfile{Name: "p", ProviderKind: "openai-compatible", BaseURL: "https://x.example/v1", APIKey: "k", Model: "m"}},
}, true},
{"cli override", ResolveOptions{
Overrides: Overrides{
MaxTurns: 42,
Provider: ProviderProfile{Name: "p", ProviderKind: "openai-compatible", BaseURL: "https://x.example/v1", APIKey: "k", Model: "m"},
},
}, true},
}
for _, tc := range cases {
resolved, err := Resolve(tc.opts)
if err != nil {
t.Fatalf("%s: Resolve() error = %v", tc.name, err)
}
if resolved.MaxTurnsSet != tc.set {
t.Fatalf("%s: MaxTurnsSet = %v, want %v (MaxTurns=%d)", tc.name, resolved.MaxTurnsSet, tc.set, resolved.MaxTurns)
}
}
}
+4 -13
View File
@@ -364,10 +364,6 @@ type FileConfig struct {
LocalControl LocalControlConfig `json:"localControl,omitempty"`
STT STTConfig `json:"stt,omitempty"`
CrossSessionInbound string `json:"crossSessionInbound,omitempty"`
// maxTurnsSet records that some merge source supplied a positive maxTurns —
// i.e. the value is configured, not the built-in default. Unexported like
// Tools.deferThresholdSet: merge bookkeeping, not a config key.
maxTurnsSet bool
// Extra preserves top-level fields written by newer Zero versions or
// extensions so a read-modify-write through this version is non-destructive.
Extra map[string]json.RawMessage `json:"-"`
@@ -465,15 +461,10 @@ type Overrides struct {
}
type ResolvedConfig struct {
ActiveProvider string
Providers []ProviderProfile
Provider ProviderProfile
MaxTurns int
// MaxTurnsSet reports whether MaxTurns came from an explicit source (a
// config file, a provider command, ZERO_MAX_TURNS, or CLI overrides) rather
// than the built-in default — callers that need a different default can
// tell the two apart.
MaxTurnsSet bool
ActiveProvider string
Providers []ProviderProfile
Provider ProviderProfile
MaxTurns int
MCP MCPConfig
Sandbox SandboxConfig
Notify NotifyConfig
+2 -31
View File
@@ -30,35 +30,15 @@ var processAlive = osProcessAlive
// only to enrich a contention error with the current holder's PID; the kernel
// lock, not PID metadata, is authoritative.
func acquireLock(path string, isAlive func(pid int) bool) (*fileLock, error) {
return acquireLockWith(isAlive, func() (*lockutil.FileLock, error) {
return lockutil.TryAcquireFileLock(path)
}, func() (int, error) {
return readPidFile(path)
})
}
func acquireLockRoot(root *os.Root, name, displayPath string, isAlive func(pid int) bool) (*fileLock, error) {
return acquireLockWith(isAlive, func() (*lockutil.FileLock, error) {
return lockutil.TryAcquireFileLockRoot(root, name, displayPath)
}, func() (int, error) {
return readPidFileRoot(root, name)
})
}
func acquireLockWith(
isAlive func(pid int) bool,
acquire func() (*lockutil.FileLock, error),
readPID func() (int, error),
) (*fileLock, error) {
if isAlive == nil {
isAlive = processAlive
}
lock, err := acquire()
lock, err := lockutil.TryAcquireFileLock(path)
if err != nil {
if !errors.Is(err, lockutil.ErrLockHeld) {
return nil, err
}
pid, perr := readPID()
pid, perr := readPidFile(path)
if perr == nil && pid > 0 && isAlive(pid) {
return nil, fmt.Errorf("%w (pid %d)", ErrAlreadyRunning, pid)
}
@@ -82,15 +62,6 @@ func (l *fileLock) release() error {
// readPidFile reads and parses the PID recorded in a lock file.
func readPidFile(path string) (int, error) {
data, err := os.ReadFile(path)
return parsePidFile(data, err)
}
func readPidFileRoot(root *os.Root, name string) (int, error) {
data, err := root.ReadFile(name)
return parsePidFile(data, err)
}
func parsePidFile(data []byte, err error) (int, error) {
if err != nil {
return 0, err
}
+7 -162
View File
@@ -7,7 +7,6 @@ import (
"fmt"
"net"
"os"
"path/filepath"
"sync"
"time"
)
@@ -24,23 +23,6 @@ type Server struct {
listener net.Listener
conns map[net.Conn]struct{} // open connections, closed on Shutdown so blocked reads return
lock *fileLock
// runtimeRoot is retained for the full default-daemon lifecycle. Every file
// child is addressed relative to this capability; runtimeDir is used only to
// verify the unavoidable AF_UNIX pathname bind still names the same object.
runtimeRoot *os.Root
runtimeDir string
// socketRoot captures the directory that actually received the pathname
// bind. It can differ from runtimeRoot only when the configured directory is
// exchanged in the final pre-bind window; retaining it makes rollback target
// the socket that was really created rather than re-resolving the pathname.
socketRoot *os.Root
socketName string
// statusRoot binds status publication and shutdown cleanup to the same
// directory object. statusCommitted is set only after this server publishes
// its document, so a failed startup never removes a previous daemon's status.
statusRoot *os.Root
statusName string
statusCommitted bool
ctx context.Context
cancel context.CancelFunc
@@ -59,14 +41,6 @@ type ServerOptions struct {
Now func() time.Time
Log func(string)
isAlive func(int) bool // test hook for the single-instance lock
// beforeStatusReplace, replaceStatusFile, and syncStatusParent are test hooks
// for the status-file commit boundary. nil selects production behavior.
beforeStatusReplace func()
replaceStatusFile func(root *os.Root, src, dst string) error
syncStatusParent func(root *os.Root) error
afterRuntimeRootOpen func() // test hook at the default-root trust boundary
beforeSocketBind func() // test hook before the final root verification
afterSocketPreflight func() // test hook in the unavoidable pathname-bind window
}
// NewServer validates options and builds a Server.
@@ -106,41 +80,11 @@ func (s *Server) Serve() error {
if err := checkSocketPathLength(s.opts.Paths.Socket); err != nil {
return err
}
defaultRoot, isDefault, err := openDefaultRuntimeRoot(s.opts.Paths)
if err != nil {
if err := secureSocketParent(s.opts.Paths.Socket); err != nil {
return err
}
if isDefault {
s.runtimeRoot = defaultRoot
s.runtimeDir = filepath.Dir(s.opts.Paths.Socket)
s.statusRoot = defaultRoot
s.statusName = filepath.Base(s.opts.Paths.Status)
if s.opts.afterRuntimeRootOpen != nil {
s.opts.afterRuntimeRootOpen()
}
if err := runtimeRootStillNamesPath(defaultRoot, s.runtimeDir); err != nil {
s.closeRuntimeRoots()
return err
}
} else {
if err := secureCustomRuntimeParents(s.opts.Paths); err != nil {
return err
}
statusRoot, err := openStatusRoot(s.opts.Paths.Status)
if err != nil {
return fmt.Errorf("daemon: open status directory: %w", err)
}
s.statusRoot = statusRoot
s.statusName = filepath.Base(s.opts.Paths.Status)
}
var lock *fileLock
if s.runtimeRoot != nil {
lock, err = acquireLockRoot(s.runtimeRoot, filepath.Base(s.opts.Paths.Lock), s.opts.Paths.Lock, s.opts.isAlive)
} else {
lock, err = acquireLock(s.opts.Paths.Lock, s.opts.isAlive)
}
lock, err := acquireLock(s.opts.Paths.Lock, s.opts.isAlive)
if err != nil {
s.closeRuntimeRoots()
return err
}
s.lock = lock
@@ -148,22 +92,7 @@ func (s *Server) Serve() error {
// A leftover socket file from an unclean exit would make Listen fail with
// "address already in use"; we hold the lock, so any socket here is stale.
if s.runtimeRoot != nil {
if err := s.runtimeRoot.Remove(filepath.Base(s.opts.Paths.Socket)); err != nil && !errors.Is(err, os.ErrNotExist) {
return fmt.Errorf("daemon: remove stale control socket: %w", err)
}
if s.opts.beforeSocketBind != nil {
s.opts.beforeSocketBind()
}
if err := runtimeRootStillNamesPath(s.runtimeRoot, s.runtimeDir); err != nil {
return err
}
if s.opts.afterSocketPreflight != nil {
s.opts.afterSocketPreflight()
}
} else {
_ = os.Remove(s.opts.Paths.Socket)
}
_ = os.Remove(s.opts.Paths.Socket)
listener, err := net.Listen("unix", s.opts.Paths.Socket)
if err != nil {
@@ -172,22 +101,6 @@ func (s *Server) Serve() error {
s.mu.Lock()
s.listener = listener
s.mu.Unlock()
if s.runtimeRoot != nil {
boundRoot, boundName, err := captureBoundSocketRoot(s.opts.Paths.Socket)
if err != nil {
_ = listener.Close()
return fmt.Errorf("daemon: capture bound control socket: %w", err)
}
s.socketRoot = boundRoot
s.socketName = boundName
if err := runtimeRootStillNamesPath(s.runtimeRoot, s.runtimeDir); err != nil {
return err
}
info, err := s.runtimeRoot.Lstat(filepath.Base(s.opts.Paths.Socket))
if err != nil || info.Mode()&os.ModeSocket == 0 {
return fmt.Errorf("daemon: bound control socket is outside the secured runtime directory")
}
}
// If Shutdown already fired during the bind window, close now and bail so a
// shutdown requested at startup is never lost (the accept loop would otherwise
// block forever waiting for a connection that never comes) (D4).
@@ -198,7 +111,7 @@ func (s *Server) Serve() error {
return nil
default:
}
if err := s.hardenSocket(); err != nil {
if err := hardenSocketFile(s.opts.Paths.Socket); err != nil {
return fmt.Errorf("daemon: harden control socket: %w", err)
}
s.startedAt = s.opts.Now()
@@ -277,56 +190,11 @@ func (s *Server) cleanup() {
if s.listener != nil {
_ = s.listener.Close()
}
if s.socketRoot != nil {
if err := s.socketRoot.Remove(s.socketName); err != nil && !errors.Is(err, os.ErrNotExist) {
s.logf("daemon: remove control socket: %v", err)
}
} else if s.runtimeRoot != nil {
if err := s.runtimeRoot.Remove(filepath.Base(s.opts.Paths.Socket)); err != nil && !errors.Is(err, os.ErrNotExist) {
s.logf("daemon: remove control socket: %v", err)
}
} else {
_ = os.Remove(s.opts.Paths.Socket)
}
if s.statusRoot != nil {
if s.statusCommitted {
if err := s.statusRoot.Remove(s.statusName); err != nil && !errors.Is(err, os.ErrNotExist) {
s.logf("daemon: remove status file: %v", err)
}
}
}
_ = os.Remove(s.opts.Paths.Socket)
_ = os.Remove(s.opts.Paths.Status)
if s.lock != nil {
_ = s.lock.release()
}
s.closeRuntimeRoots()
}
func (s *Server) hardenSocket() error {
if s.runtimeRoot != nil {
return hardenSocketFileRoot(s.runtimeRoot, filepath.Base(s.opts.Paths.Socket))
}
return hardenSocketFile(s.opts.Paths.Socket)
}
func (s *Server) closeRuntimeRoots() {
if s.socketRoot != nil {
if err := s.socketRoot.Close(); err != nil {
s.logf("daemon: close bound socket directory: %v", err)
}
}
s.socketRoot = nil
if s.statusRoot != nil && s.statusRoot != s.runtimeRoot {
if err := s.statusRoot.Close(); err != nil {
s.logf("daemon: close status directory: %v", err)
}
}
s.statusRoot = nil
if s.runtimeRoot != nil {
if err := s.runtimeRoot.Close(); err != nil {
s.logf("daemon: close runtime directory: %v", err)
}
}
s.runtimeRoot = nil
}
func (s *Server) writeStatusFile() error {
@@ -340,30 +208,7 @@ func (s *Server) writeStatusFile() error {
if err != nil {
return err
}
root := s.statusRoot
ownedRoot := false
if root == nil {
var err error
root, err = openStatusRoot(s.opts.Paths.Status)
if err != nil {
return fmt.Errorf("daemon: write status file: %w", err)
}
ownedRoot = true
}
committed, err := writeStatusFileAtomicallyRoot(root, filepath.Base(s.opts.Paths.Status), data, 0o600, s.opts.beforeStatusReplace, s.opts.replaceStatusFile, s.opts.syncStatusParent)
if ownedRoot {
if closeErr := root.Close(); closeErr != nil {
err = errors.Join(err, fmt.Errorf("close status directory: %w", closeErr))
}
}
if committed {
s.statusCommitted = true
}
if err != nil {
if committed {
s.logf("daemon: status file publication committed with warning: %v", err)
return nil
}
if err := os.WriteFile(s.opts.Paths.Status, data, 0o600); err != nil {
return fmt.Errorf("daemon: write status file: %w", err)
}
return nil
+1 -329
View File
@@ -2,201 +2,20 @@ package daemon
import (
"errors"
"net"
"os"
"path/filepath"
"runtime"
"strings"
"testing"
"time"
"github.com/Gitlawb/zero/internal/observability"
)
func TestServeSupportsReadOnlyCustomRuntimeDirectory(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("Unix permission compatibility regression")
}
dir, err := os.MkdirTemp("/tmp", "zero-serve-")
if err != nil {
t.Fatal(err)
}
t.Cleanup(func() { _ = os.RemoveAll(dir) })
if err := os.Chmod(dir, 0o755); err != nil {
t.Fatal(err)
}
paths := Paths{
Socket: filepath.Join(dir, "daemon.sock"),
Lock: filepath.Join(dir, "daemon.lock"),
Status: filepath.Join(dir, "daemon.status"),
}
launcher, _ := seqLauncher(&fakeWorker{pid: 1})
srv := newTestServerWithPaths(t, launcher, paths)
serveErr := make(chan error, 1)
go func() { serveErr <- srv.Serve() }()
waitForFile(t, paths.Status)
srv.Shutdown()
if err := <-serveErr; err != nil {
t.Fatalf("Serve with 0755 custom runtime directory: %v", err)
}
info, err := os.Stat(dir)
if err != nil {
t.Fatal(err)
}
if got := info.Mode().Perm(); got != 0o755 {
t.Fatalf("custom runtime directory permissions = %04o, want unchanged 0755", got)
}
}
func TestServeKeepsDefaultRootBoundAcrossCoordination(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("Windows may deny renaming a directory with an open AF_UNIX lifecycle handle")
}
home, err := os.MkdirTemp("/tmp", "zero-root-")
if err != nil {
t.Fatal(err)
}
t.Cleanup(func() { _ = os.RemoveAll(home) })
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
t.Setenv("XDG_RUNTIME_DIR", "")
paths, err := DefaultPaths()
if err != nil {
t.Fatal(err)
}
dir := filepath.Dir(paths.Socket)
moved := filepath.Join(home, "bound-runtime")
if err := os.Mkdir(dir, 0o700); err != nil {
t.Fatal(err)
}
launcher, _ := seqLauncher(&fakeWorker{pid: 1})
srv := newTestServerWithPaths(t, launcher, paths)
srv.opts.beforeSocketBind = func() {
if err := os.Rename(dir, moved); err != nil {
t.Fatal(err)
}
if err := os.Mkdir(dir, 0o700); err != nil {
t.Fatal(err)
}
}
err = srv.Serve()
if err == nil || !strings.Contains(err.Error(), "changed after it was secured") {
t.Fatalf("Serve error = %v, want replaced-runtime rejection", err)
}
if _, err := os.Stat(filepath.Join(moved, filepath.Base(paths.Lock))); err != nil {
t.Fatalf("rooted lock was not created in the bound directory: %v", err)
}
for _, path := range []string{paths.Lock, paths.Socket, paths.Status} {
if _, err := os.Lstat(path); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("substitute runtime entry %s was touched: %v", path, err)
}
}
}
func TestServeRemovesSocketBoundAfterFinalRuntimeSwap(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("Windows may deny renaming a directory with an open AF_UNIX lifecycle handle")
}
home, err := os.MkdirTemp("/tmp", "zero-bind-swap-")
if err != nil {
t.Fatal(err)
}
t.Cleanup(func() { _ = os.RemoveAll(home) })
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
t.Setenv("XDG_RUNTIME_DIR", "")
paths, err := DefaultPaths()
if err != nil {
t.Fatal(err)
}
dir := filepath.Dir(paths.Socket)
moved := filepath.Join(home, "trusted-runtime")
if err := os.Mkdir(dir, 0o700); err != nil {
t.Fatal(err)
}
launcher, _ := seqLauncher(&fakeWorker{pid: 1})
srv := newTestServerWithPaths(t, launcher, paths)
srv.opts.afterSocketPreflight = func() {
if err := os.Rename(dir, moved); err != nil {
t.Fatal(err)
}
if err := os.Mkdir(dir, 0o700); err != nil {
t.Fatal(err)
}
}
err = srv.Serve()
if err == nil || !strings.Contains(err.Error(), "changed after it was secured") {
t.Fatalf("Serve error = %v, want post-bind runtime replacement rejection", err)
}
if _, err := os.Stat(filepath.Join(moved, filepath.Base(paths.Lock))); err != nil {
t.Fatalf("rooted lock was not retained in the trusted directory: %v", err)
}
if _, err := os.Lstat(paths.Socket); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("substitute runtime retained stale socket: %v", err)
}
listener, err := net.Listen("unix", paths.Socket)
if err != nil {
t.Fatalf("substitute runtime remained unbindable after rollback: %v", err)
}
if err := listener.Close(); err != nil {
t.Fatal(err)
}
}
func TestOpenRuntimeLogHardensDefaultRootBeforeOpen(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("Windows DACL hardening has platform-specific coverage")
}
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
t.Setenv("XDG_RUNTIME_DIR", "")
paths, err := DefaultPaths()
if err != nil {
t.Fatal(err)
}
dir := filepath.Dir(paths.Socket)
if err := os.MkdirAll(dir, 0o777); err != nil {
t.Fatal(err)
}
if err := os.Chmod(dir, 0o777); err != nil {
t.Fatal(err)
}
file, logPath, err := OpenRuntimeLog(paths)
if err != nil {
t.Fatalf("OpenRuntimeLog: %v", err)
}
if err := file.Close(); err != nil {
t.Fatal(err)
}
if logPath != filepath.Join(dir, "daemon.log") {
t.Fatalf("log path = %q", logPath)
}
info, err := os.Stat(dir)
if err != nil {
t.Fatal(err)
}
if got := info.Mode().Perm(); got&0o077 != 0 {
t.Fatalf("default runtime directory permissions = %04o, want owner-only", got)
}
}
func newTestServer(t *testing.T, launcher Launcher) (*Server, Paths) {
t.Helper()
dir := t.TempDir()
secureStatusTestDir(t, dir)
paths := Paths{
Socket: filepath.Join(dir, "d.sock"),
Lock: filepath.Join(dir, "d.lock"),
Status: filepath.Join(dir, "d.status"),
}
return newTestServerWithPaths(t, launcher, paths), paths
}
func newTestServerWithPaths(t *testing.T, launcher Launcher, paths Paths) *Server {
t.Helper()
pool, err := NewPool(PoolOptions{Size: 2, Launcher: launcher, KillTimeout: 200 * time.Millisecond})
if err != nil {
t.Fatalf("NewPool: %v", err)
@@ -209,7 +28,7 @@ func newTestServerWithPaths(t *testing.T, launcher Launcher, paths Paths) *Serve
if err != nil {
t.Fatalf("NewServer: %v", err)
}
return srv
return srv, paths
}
func waitForFile(t *testing.T, path string) {
@@ -317,153 +136,6 @@ func TestServerEndToEnd(t *testing.T) {
}
}
func TestServePreservesPreviousStatusWhenReplacementFails(t *testing.T) {
launcher, _ := seqLauncher(&fakeWorker{pid: 1, exitCode: 0})
dir, err := os.MkdirTemp("", "zero-daemon-status-")
if err != nil {
t.Fatal(err)
}
t.Cleanup(func() { _ = os.RemoveAll(dir) })
secureStatusTestDir(t, dir)
paths := Paths{Socket: filepath.Join(dir, "d.sock"), Lock: filepath.Join(dir, "d.lock"), Status: filepath.Join(dir, "d.status")}
srv := newTestServerWithPaths(t, launcher, paths)
previous := []byte(`{"pid":7,"socket":"previous","version":1}`)
if err := os.WriteFile(paths.Status, previous, 0o600); err != nil {
t.Fatal(err)
}
srv.opts.replaceStatusFile = func(*os.Root, string, string) error {
return errors.New("injected replacement failure")
}
if err := srv.Serve(); err == nil || !strings.Contains(err.Error(), "injected replacement failure") {
t.Fatal("Serve succeeded despite injected status replacement failure")
}
got, err := os.ReadFile(paths.Status)
if err != nil {
t.Fatalf("previous status was removed after failed publication: %v", err)
}
if string(got) != string(previous) {
t.Fatalf("previous status changed after failed publication: %q", got)
}
}
func TestServeCleansStatusThroughBoundDirectoryAfterSwap(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("Windows does not permit renaming the directory containing the live socket")
}
parent, err := os.MkdirTemp("", "zero-daemon-swap-")
if err != nil {
t.Fatal(err)
}
t.Cleanup(func() { _ = os.RemoveAll(parent) })
dir := filepath.Join(parent, "live")
movedDir := filepath.Join(parent, "moved")
if err := os.Mkdir(dir, 0o700); err != nil {
t.Fatal(err)
}
secureStatusTestDir(t, dir)
paths := Paths{
Socket: filepath.Join(dir, "d.sock"),
Lock: filepath.Join(dir, "d.lock"),
Status: filepath.Join(dir, "d.status"),
}
launcher, _ := seqLauncher(&fakeWorker{pid: 1, exitCode: 0})
srv := newTestServerWithPaths(t, launcher, paths)
substitute := []byte(`{"pid":999,"socket":"substitute"}`)
srv.opts.beforeStatusReplace = func() {
if err := os.Rename(dir, movedDir); err != nil {
t.Fatalf("move status directory: %v", err)
}
if err := os.Mkdir(dir, 0o700); err != nil {
t.Fatalf("create substitute directory: %v", err)
}
if err := os.WriteFile(paths.Status, substitute, 0o600); err != nil {
t.Fatalf("write substitute status: %v", err)
}
}
serveErr := make(chan error, 1)
go func() { serveErr <- srv.Serve() }()
waitForFile(t, filepath.Join(movedDir, "d.status"))
srv.Shutdown()
select {
case err := <-serveErr:
if err != nil {
t.Fatalf("Serve: %v", err)
}
case <-time.After(3 * time.Second):
t.Fatal("Serve did not return after shutdown")
}
if _, err := os.Stat(filepath.Join(movedDir, "d.status")); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("bound status survived cleanup: %v", err)
}
got, err := os.ReadFile(paths.Status)
if err != nil {
t.Fatalf("substitute status was removed by pathname cleanup: %v", err)
}
if string(got) != string(substitute) {
t.Fatalf("substitute status changed during cleanup: %q", got)
}
}
func TestServerPublishesDefaultStatusAfterCrashReportCreatesRuntimeDirectory(t *testing.T) {
home, err := os.MkdirTemp("", "zero-home-")
if err != nil {
t.Fatal(err)
}
t.Cleanup(func() { _ = os.RemoveAll(home) })
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
t.Setenv("XDG_RUNTIME_DIR", "")
if _, err := observability.WriteCrashReport(
observability.DefaultCrashDir(),
"cli",
"boom",
[]byte("stack"),
time.Now(),
); err != nil {
t.Fatalf("WriteCrashReport: %v", err)
}
paths, err := DefaultPaths()
if err != nil {
t.Fatalf("DefaultPaths: %v", err)
}
if paths.Status != filepath.Join(home, ".zero", "daemon.status") {
t.Fatalf("default status path = %q, want path beneath temporary home", paths.Status)
}
launcher, _ := seqLauncher(&fakeWorker{pid: 1})
srv := newTestServerWithPaths(t, launcher, paths)
serveErr := make(chan error, 1)
go func() { serveErr <- srv.Serve() }()
deadline := time.NewTimer(3 * time.Second)
defer deadline.Stop()
for {
if _, err := os.Stat(paths.Status); err == nil {
break
}
select {
case err := <-serveErr:
t.Fatalf("Serve returned before publishing status: %v", err)
case <-deadline.C:
t.Fatal("daemon did not publish its default status file")
case <-time.After(2 * time.Millisecond):
}
}
srv.Shutdown()
select {
case err := <-serveErr:
if err != nil {
t.Fatalf("Serve returned error: %v", err)
}
case <-time.After(3 * time.Second):
t.Fatal("Serve did not return after shutdown")
}
}
func TestServerSecondInstanceFails(t *testing.T) {
block := make(chan struct{})
defer close(block)
+4 -179
View File
@@ -1,12 +1,9 @@
package daemon
import (
"errors"
"fmt"
"os"
"path/filepath"
"github.com/Gitlawb/zero/internal/privatedir"
)
// maxUnixSocketPath bounds the socket path to the smallest platform sun_path
@@ -15,182 +12,10 @@ import (
// safe cross-platform ceiling.
const maxUnixSocketPath = 103
func secureCustomRuntimeParents(paths Paths) error {
parents := []struct {
name string
path string
}{
{name: "socket", path: filepath.Dir(paths.Socket)},
{name: "lock", path: filepath.Dir(paths.Lock)},
{name: "status", path: filepath.Dir(paths.Status)},
}
seen := make(map[string]struct{}, len(parents))
for _, parent := range parents {
absolute, err := filepath.Abs(parent.path)
if err != nil {
return fmt.Errorf("daemon: resolve %s directory: %w", parent.name, err)
}
if _, ok := seen[absolute]; ok {
continue
}
seen[absolute] = struct{}{}
if err := os.MkdirAll(absolute, 0o700); err != nil {
return fmt.Errorf("daemon: create %s directory: %w", parent.name, err)
}
root, err := os.OpenRoot(absolute)
if err != nil {
return fmt.Errorf("daemon: open custom %s directory: %w", parent.name, err)
}
validateErr := validateStatusRoot(root)
closeErr := root.Close()
if validateErr != nil {
return fmt.Errorf("daemon: validate custom %s directory: %w", parent.name, validateErr)
}
if closeErr != nil {
return fmt.Errorf("daemon: close custom %s directory: %w", parent.name, closeErr)
}
}
return nil
}
// OpenRuntimeLog secures the default runtime root before acquiring the
// detached daemon's log descriptor. Custom endpoint layouts remain supported,
// but their existing parent permissions are caller-owned and left untouched.
func OpenRuntimeLog(paths Paths) (*os.File, string, error) {
logPath := filepath.Join(filepath.Dir(paths.Socket), "daemon.log")
root, isDefault, err := openDefaultRuntimeRoot(paths)
if err != nil {
return nil, logPath, err
}
if isDefault {
defer root.Close()
file, err := openRootAppendRegular(root, filepath.Base(logPath))
if err != nil {
return nil, logPath, fmt.Errorf("daemon: open runtime log: %w", err)
}
return file, logPath, nil
}
parent := filepath.Dir(logPath)
if err := os.MkdirAll(parent, 0o700); err != nil {
return nil, logPath, fmt.Errorf("daemon: create custom log directory: %w", err)
}
info, err := os.Lstat(logPath)
if err == nil && !info.Mode().IsRegular() {
return nil, logPath, fmt.Errorf("daemon: custom runtime log is not a regular file")
}
if err != nil && !errors.Is(err, os.ErrNotExist) {
return nil, logPath, fmt.Errorf("daemon: inspect custom runtime log: %w", err)
}
file, err := os.OpenFile(logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o600)
if err != nil {
return nil, logPath, fmt.Errorf("daemon: open custom runtime log: %w", err)
}
return file, logPath, nil
}
func openDefaultRuntimeRoot(paths Paths) (*os.Root, bool, error) {
defaults, err := DefaultPaths()
if err != nil {
// A caller-supplied layout remains usable even when the process has no
// resolvable home directory. Production default paths can only reach this
// helper after DefaultPaths has already succeeded.
return nil, false, nil
}
matches, err := samePaths(paths, defaults)
if err != nil {
return nil, false, err
}
if !matches {
return nil, false, nil
}
root, err := privatedir.Open(filepath.Dir(defaults.Socket))
if err != nil {
return nil, false, fmt.Errorf("daemon: secure default runtime directory: %w", err)
}
return root, true, nil
}
func samePaths(left, right Paths) (bool, error) {
pairs := [][2]string{
{left.Socket, right.Socket},
{left.Lock, right.Lock},
{left.Status, right.Status},
}
for _, pair := range pairs {
leftAbs, err := filepath.Abs(pair[0])
if err != nil {
return false, fmt.Errorf("daemon: resolve runtime path: %w", err)
}
rightAbs, err := filepath.Abs(pair[1])
if err != nil {
return false, fmt.Errorf("daemon: resolve default runtime path: %w", err)
}
if filepath.Clean(leftAbs) != filepath.Clean(rightAbs) {
return false, nil
}
}
return true, nil
}
func runtimeRootStillNamesPath(root *os.Root, path string) error {
opened, err := root.Stat(".")
if err != nil {
return fmt.Errorf("inspect bound runtime directory: %w", err)
}
current, err := os.Lstat(path)
if err != nil {
return fmt.Errorf("inspect runtime directory entry: %w", err)
}
if current.Mode()&os.ModeSymlink != 0 || !os.SameFile(opened, current) {
return fmt.Errorf("daemon: default runtime directory changed after it was secured")
}
return nil
}
func captureBoundSocketRoot(path string) (*os.Root, string, error) {
root, err := os.OpenRoot(filepath.Dir(path))
if err != nil {
return nil, "", fmt.Errorf("open bound socket directory: %w", err)
}
name := filepath.Base(path)
info, err := root.Lstat(name)
if err != nil {
_ = root.Close()
return nil, "", fmt.Errorf("inspect bound socket: %w", err)
}
if info.Mode()&os.ModeSocket == 0 {
_ = root.Close()
return nil, "", fmt.Errorf("bound socket path is not a socket")
}
return root, name, nil
}
func openRootAppendRegular(root *os.Root, name string) (*os.File, error) {
info, err := root.Lstat(name)
if errors.Is(err, os.ErrNotExist) {
return root.OpenFile(name, os.O_CREATE|os.O_EXCL|os.O_WRONLY|os.O_APPEND, 0o600)
}
if err != nil {
return nil, err
}
if !info.Mode().IsRegular() {
return nil, fmt.Errorf("runtime log is not a regular file")
}
file, err := root.OpenFile(name, os.O_WRONLY|os.O_APPEND, 0o600)
if err != nil {
return nil, err
}
opened, err := file.Stat()
if err != nil {
_ = file.Close()
return nil, err
}
if !opened.Mode().IsRegular() {
_ = file.Close()
return nil, fmt.Errorf("runtime log changed while opening")
}
return file, nil
// secureSocketParent creates the socket's parent directory owner-only (0700 on
// POSIX; on Windows the per-user profile directory is already ACL-restricted).
func secureSocketParent(socketPath string) error {
return os.MkdirAll(filepath.Dir(socketPath), 0o700)
}
// checkSocketPathLength rejects an over-long unix socket path before bind.
-4
View File
@@ -10,7 +10,3 @@ import "os"
func hardenSocketFile(path string) error {
return os.Chmod(path, 0o600)
}
func hardenSocketFileRoot(root *os.Root, name string) error {
return root.Chmod(name, 0o600)
}
-4
View File
@@ -2,12 +2,8 @@
package daemon
import "os"
// hardenSocketFile is a no-op on Windows: AF_UNIX socket files do not honor POSIX
// mode bits, and the daemon places the socket under the per-user profile
// directory, which is already ACL-restricted to the owner. A dedicated restricted
// ACL (or a named pipe with an explicit DACL) is a documented follow-up.
func hardenSocketFile(string) error { return nil }
func hardenSocketFileRoot(*os.Root, string) error { return nil }
-20
View File
@@ -1,20 +0,0 @@
//go:build !windows
package daemon
import (
"fmt"
"os"
"syscall"
)
func checkStatusDirOwner(_ *os.Root, info os.FileInfo) error {
stat, ok := info.Sys().(*syscall.Stat_t)
if !ok {
return fmt.Errorf("status directory ownership metadata is unavailable")
}
if int(stat.Uid) != os.Geteuid() {
return fmt.Errorf("status directory is owned by uid %d, not the current user", stat.Uid)
}
return nil
}
@@ -1,51 +0,0 @@
//go:build !windows
package daemon
import (
"os"
"strings"
"syscall"
"testing"
"time"
)
func secureStatusTestDirPlatform(t *testing.T, dir string) {
t.Helper()
if err := os.Chmod(dir, 0o700); err != nil {
t.Fatal(err)
}
}
func broadenStatusTestDirPlatform(t *testing.T, dir string) {
t.Helper()
if err := os.Chmod(dir, 0o755); err != nil {
t.Fatal(err)
}
}
func TestCheckStatusDirOwnerRejectsMissingMetadata(t *testing.T) {
err := checkStatusDirOwner(nil, statusDirOwnerTestInfo{})
if err == nil || !strings.Contains(err.Error(), "metadata is unavailable") {
t.Fatalf("checkStatusDirOwner error = %v, want unavailable metadata rejection", err)
}
}
func TestCheckStatusDirOwnerRejectsDifferentUser(t *testing.T) {
info := statusDirOwnerTestInfo{sys: &syscall.Stat_t{Uid: uint32(os.Geteuid() + 1)}}
err := checkStatusDirOwner(nil, info)
if err == nil || !strings.Contains(err.Error(), "not the current user") {
t.Fatalf("checkStatusDirOwner error = %v, want owner mismatch rejection", err)
}
}
type statusDirOwnerTestInfo struct {
sys any
}
func (statusDirOwnerTestInfo) Name() string { return "." }
func (statusDirOwnerTestInfo) Size() int64 { return 0 }
func (statusDirOwnerTestInfo) Mode() os.FileMode { return os.ModeDir | 0o700 }
func (statusDirOwnerTestInfo) ModTime() time.Time { return time.Time{} }
func (statusDirOwnerTestInfo) IsDir() bool { return true }
func (info statusDirOwnerTestInfo) Sys() any { return info.sys }
-137
View File
@@ -1,137 +0,0 @@
//go:build windows
package daemon
import (
"errors"
"fmt"
"os"
"unsafe"
"golang.org/x/sys/windows"
)
const statusDirectoryWriteAccess = windows.ACCESS_MASK(
windows.GENERIC_ALL |
windows.GENERIC_WRITE |
windows.DELETE |
windows.WRITE_DAC |
windows.WRITE_OWNER |
windows.FILE_WRITE_DATA |
windows.FILE_APPEND_DATA |
windows.FILE_WRITE_ATTRIBUTES |
windows.FILE_WRITE_EA |
0x40, // FILE_DELETE_CHILD
)
// checkStatusDirOwner validates ownership and write access through a handle
// opened beneath root. Path-based ACL inspection would recreate the ancestor
// swap race that Root is intended to close.
func checkStatusDirOwner(root *os.Root, _ os.FileInfo) (returnErr error) {
directory, err := root.Open(".")
if err != nil {
return fmt.Errorf("open status directory for access validation: %w", err)
}
defer func() {
if err := directory.Close(); err != nil {
returnErr = errors.Join(returnErr, fmt.Errorf("close status directory access handle: %w", err))
}
}()
raw, err := directory.SyscallConn()
if err != nil {
return fmt.Errorf("access status directory handle: %w", err)
}
var descriptor *windows.SECURITY_DESCRIPTOR
var queryErr error
if err := raw.Control(func(handle uintptr) {
descriptor, queryErr = windows.GetSecurityInfo(
windows.Handle(handle),
windows.SE_FILE_OBJECT,
windows.OWNER_SECURITY_INFORMATION|windows.DACL_SECURITY_INFORMATION,
)
}); err != nil {
return fmt.Errorf("inspect status directory access: %w", err)
}
if queryErr != nil {
return fmt.Errorf("inspect status directory owner and DACL: %w", queryErr)
}
if descriptor == nil {
return fmt.Errorf("status directory security descriptor is unavailable")
}
token := windows.GetCurrentProcessToken()
user, err := token.GetTokenUser()
if err != nil {
return fmt.Errorf("resolve current Windows user: %w", err)
}
tokenOwner, err := currentWindowsTokenOwner(token)
if err != nil {
return fmt.Errorf("resolve current Windows token owner: %w", err)
}
owner, _, err := descriptor.Owner()
if err != nil {
return fmt.Errorf("read status directory owner: %w", err)
}
if owner == nil || (!owner.Equals(user.User.Sid) && !owner.Equals(tokenOwner)) {
return fmt.Errorf("status directory is not owned by the current Windows token")
}
dacl, _, err := descriptor.DACL()
if err != nil {
return fmt.Errorf("read status directory DACL: %w", err)
}
if dacl == nil {
return fmt.Errorf("status directory has an unrestricted Windows DACL")
}
for index := uint16(0); index < dacl.AceCount; index++ {
var ace *windows.ACCESS_ALLOWED_ACE
if err := windows.GetAce(dacl, uint32(index), &ace); err != nil {
return fmt.Errorf("read status directory DACL entry %d: %w", index, err)
}
switch ace.Header.AceType {
case windows.ACCESS_DENIED_ACE_TYPE:
continue
case windows.ACCESS_ALLOWED_ACE_TYPE:
default:
return fmt.Errorf("status directory DACL entry %d has unsupported type %d", index, ace.Header.AceType)
}
if ace.Mask&statusDirectoryWriteAccess == 0 {
continue
}
trustee := (*windows.SID)(unsafe.Pointer(&ace.SidStart))
if !allowedStatusDirectoryTrustee(trustee, user.User.Sid) {
return fmt.Errorf("status directory DACL grants write access to unexpected trustee %s", trustee.String())
}
}
return nil
}
type statusDirectoryTokenOwner struct {
owner *windows.SID
}
func currentWindowsTokenOwner(token windows.Token) (*windows.SID, error) {
var size uint32
err := windows.GetTokenInformation(token, windows.TokenOwner, nil, 0, &size)
if err != windows.ERROR_INSUFFICIENT_BUFFER {
return nil, err
}
buffer := make([]byte, size)
if err := windows.GetTokenInformation(token, windows.TokenOwner, &buffer[0], size, &size); err != nil {
return nil, err
}
owner := (*statusDirectoryTokenOwner)(unsafe.Pointer(&buffer[0])).owner
if owner == nil {
return nil, errors.New("windows access token has no default owner")
}
return owner.Copy()
}
func allowedStatusDirectoryTrustee(trustee, user *windows.SID) bool {
return trustee != nil && (trustee.Equals(user) ||
trustee.IsWellKnown(windows.WinLocalSystemSid) ||
trustee.IsWellKnown(windows.WinBuiltinAdministratorsSid) ||
trustee.IsWellKnown(windows.WinCreatorOwnerSid) ||
trustee.IsWellKnown(windows.WinCreatorOwnerRightsSid))
}
@@ -1,97 +0,0 @@
//go:build windows
package daemon
import (
"fmt"
"os"
"runtime"
"strings"
"testing"
"golang.org/x/sys/windows"
)
func secureStatusTestDirPlatform(t *testing.T, dir string) {
t.Helper()
user, err := windows.GetCurrentProcessToken().GetTokenUser()
if err != nil {
t.Fatal(err)
}
descriptor, err := windows.SecurityDescriptorFromString(
fmt.Sprintf("O:%sD:P(A;OICI;GA;;;%s)(A;OICI;GA;;;SY)", user.User.Sid.String(), user.User.Sid.String()),
)
if err != nil {
t.Fatal(err)
}
dacl, _, err := descriptor.DACL()
if err != nil {
t.Fatal(err)
}
if err := windows.SetNamedSecurityInfo(
dir,
windows.SE_FILE_OBJECT,
windows.DACL_SECURITY_INFORMATION|windows.PROTECTED_DACL_SECURITY_INFORMATION,
nil,
nil,
dacl,
nil,
); err != nil {
t.Fatal(err)
}
}
func broadenStatusTestDirPlatform(t *testing.T, dir string) {
t.Helper()
worldSID, err := windows.CreateWellKnownSid(windows.WinWorldSid)
if err != nil {
t.Fatal(err)
}
var pinner runtime.Pinner
pinner.Pin(worldSID)
defer pinner.Unpin()
broadDACL, err := windows.ACLFromEntries([]windows.EXPLICIT_ACCESS{{
AccessPermissions: windows.GENERIC_ALL,
AccessMode: windows.GRANT_ACCESS,
Inheritance: windows.SUB_CONTAINERS_AND_OBJECTS_INHERIT,
Trustee: windows.TRUSTEE{
TrusteeForm: windows.TRUSTEE_IS_SID,
TrusteeType: windows.TRUSTEE_IS_WELL_KNOWN_GROUP,
TrusteeValue: windows.TrusteeValueFromSID(worldSID),
},
}}, nil)
if err != nil {
t.Fatal(err)
}
if err := windows.SetNamedSecurityInfo(
dir,
windows.SE_FILE_OBJECT,
windows.DACL_SECURITY_INFORMATION|windows.PROTECTED_DACL_SECURITY_INFORMATION,
nil,
nil,
broadDACL,
nil,
); err != nil {
t.Fatal(err)
}
}
func TestCheckStatusDirOwnerRejectsBroadDACL(t *testing.T) {
dir := t.TempDir()
secureStatusTestDirPlatform(t, dir)
broadenStatusTestDirPlatform(t, dir)
root, err := os.OpenRoot(dir)
if err != nil {
t.Fatal(err)
}
defer root.Close()
info, err := root.Stat(".")
if err != nil {
t.Fatal(err)
}
err = checkStatusDirOwner(root, info)
if err == nil || !strings.Contains(err.Error(), "unexpected trustee") {
t.Fatalf("checkStatusDirOwner error = %v, want broad DACL rejection", err)
}
}
-169
View File
@@ -1,169 +0,0 @@
package daemon
import (
"crypto/rand"
"encoding/hex"
"errors"
"fmt"
"os"
"path/filepath"
"runtime"
"github.com/Gitlawb/zero/internal/fsutil"
)
const (
statusTempPrefix = ".daemon-status-"
statusTempPattern = statusTempPrefix + "*"
)
func openStatusRoot(path string) (*os.Root, error) {
root, err := os.OpenRoot(filepath.Dir(path))
if err != nil {
return nil, fmt.Errorf("open status directory: %w", err)
}
if err := validateStatusRoot(root); err != nil {
_ = root.Close()
return nil, err
}
return root, nil
}
// writeStatusFileAtomicallyRoot publishes through a caller-owned, already
// bound directory root. The returned boolean crosses the commit boundary: once
// true, err is a durability/cleanup warning and the complete document is live.
func writeStatusFileAtomicallyRoot(
root *os.Root,
statusName string,
data []byte,
perm os.FileMode,
beforeReplace func(),
replace func(root *os.Root, src, dst string) error,
syncParent func(root *os.Root) error,
) (committed bool, returnErr error) {
if err := validateStatusRoot(root); err != nil {
return false, err
}
temp, tempName, err := createStatusTemp(root, perm)
if err != nil {
return false, err
}
closed := false
defer func() {
if !closed {
if err := temp.Close(); err != nil {
returnErr = errors.Join(returnErr, fmt.Errorf("close temporary status file during cleanup: %w", err))
}
closed = true
}
if err := root.Remove(tempName); err != nil && !errors.Is(err, os.ErrNotExist) {
returnErr = errors.Join(returnErr, fmt.Errorf("remove temporary status file: %w", err))
}
}()
if err := temp.Chmod(perm); err != nil {
return false, fmt.Errorf("set temporary status file permissions: %w", err)
}
if _, err := temp.Write(data); err != nil {
return false, fmt.Errorf("write temporary status file: %w", err)
}
if err := temp.Sync(); err != nil {
return false, fmt.Errorf("sync temporary status file: %w", err)
}
closeErr := temp.Close()
closed = true
if closeErr != nil {
return false, fmt.Errorf("close temporary status file: %w", closeErr)
}
if beforeReplace != nil {
beforeReplace()
}
if err := prepareStatusReplacement(root, tempName, statusName); err != nil {
return false, fmt.Errorf("prepare status replacement: %w", err)
}
rename := root.Rename
if replace != nil {
rename = func(src, dst string) error { return replace(root, src, dst) }
}
// On Windows an existing reader that omitted FILE_SHARE_DELETE can block
// this replacement even though an in-place write would succeed. Keep the
// atomic publication boundary: RenameWithRetry bounds transient sharing
// failures to ten short attempts, and a persistent failure leaves the old
// status document intact instead of exposing a partially written one.
var committedWarning error
if err := fsutil.RenameWithRetry(tempName, statusName, rename); err != nil {
var committedReplacement *fsutil.CommittedReplacementCleanupError
if !errors.As(err, &committedReplacement) {
return false, fmt.Errorf("replace status file: %w", err)
}
committedWarning = fmt.Errorf("clean up replaced status file: %w", err)
}
committed = true
if syncParent == nil {
syncParent = syncStatusRoot
}
if err := syncParent(root); err != nil {
committedWarning = errors.Join(committedWarning, fmt.Errorf("sync status directory: %w", err))
}
if committedWarning != nil {
return committed, committedWarning
}
return committed, nil
}
func validateStatusRoot(root *os.Root) error {
info, err := root.Stat(".")
if err != nil {
return fmt.Errorf("inspect status directory: %w", err)
}
if !info.IsDir() {
return fmt.Errorf("status directory is not a directory")
}
if runtime.GOOS != "windows" && info.Mode().Perm()&0o022 != 0 {
return fmt.Errorf("status directory permissions are %04o, want no group/other write access", info.Mode().Perm())
}
if err := checkStatusDirOwner(root, info); err != nil {
return err
}
return nil
}
func createStatusTemp(root *os.Root, perm os.FileMode) (*os.File, string, error) {
for range 100 {
var suffix [16]byte
if _, err := rand.Read(suffix[:]); err != nil {
return nil, "", fmt.Errorf("generate temporary status file name: %w", err)
}
name := statusTempPrefix + hex.EncodeToString(suffix[:])
file, err := root.OpenFile(name, os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm)
if err == nil {
return file, name, nil
}
if errors.Is(err, os.ErrExist) {
continue
}
return nil, "", fmt.Errorf("create temporary status file: %w", err)
}
return nil, "", fmt.Errorf("create temporary status file: exhausted unique names")
}
// syncStatusRoot makes the replacement directory entry durable through the
// same bound root used for creation and replacement. Windows directory sync is
// best-effort because Go cannot fsync a directory there.
func syncStatusRoot(root *os.Root) error {
if runtime.GOOS == "windows" {
return nil
}
directory, err := root.Open(".")
if err != nil {
return err
}
syncErr := directory.Sync()
closeErr := directory.Close()
if syncErr != nil {
return syncErr
}
return closeErr
}
-476
View File
@@ -1,476 +0,0 @@
package daemon
import (
"encoding/json"
"errors"
"os"
"path/filepath"
"runtime"
"strings"
"testing"
"time"
"github.com/Gitlawb/zero/internal/fsutil"
"github.com/Gitlawb/zero/internal/privatedir"
)
func TestWriteStatusFilePreservesPreviousDocumentWhenReplaceFails(t *testing.T) {
dir := t.TempDir()
secureStatusTestDir(t, dir)
path := filepath.Join(dir, "daemon.status")
previous := []byte(`{"pid":7,"socket":"old.sock","version":1,"startedAt":"2026-08-01T00:00:00Z"}`)
if err := os.WriteFile(path, previous, 0o600); err != nil {
t.Fatal(err)
}
replaceErr := errors.New("injected replacement failure")
server := &Server{
startedAt: time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC),
opts: ServerOptions{
Paths: Paths{Status: path},
Version: 2,
replaceStatusFile: func(root *os.Root, src, dst string) error {
staged, err := root.ReadFile(src)
if err != nil {
t.Fatalf("read staged status: %v", err)
}
var decoded StatusFile
if err := json.Unmarshal(staged, &decoded); err != nil {
t.Fatalf("staged status is incomplete JSON: %v", err)
}
current, err := root.ReadFile(dst)
if err != nil {
t.Fatalf("read previous status at commit boundary: %v", err)
}
if string(current) != string(previous) {
t.Fatalf("previous status changed before commit: %q", current)
}
return replaceErr
},
},
}
err := server.writeStatusFile()
if !errors.Is(err, replaceErr) {
t.Fatalf("writeStatusFile error = %v, want injected replacement failure", err)
}
current, err := os.ReadFile(path)
if err != nil {
t.Fatalf("read preserved status: %v", err)
}
if string(current) != string(previous) {
t.Fatalf("status after failed publication = %q, want previous document", current)
}
assertNoStatusTemps(t, dir)
}
func TestWriteStatusFilePublishesCompleteRestrictedDocument(t *testing.T) {
dir := t.TempDir()
secureStatusTestDir(t, dir)
path := filepath.Join(dir, "daemon.status")
if err := os.WriteFile(path, []byte(`{"pid":7}`), 0o644); err != nil {
t.Fatal(err)
}
startedAt := time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC)
parentSynced := false
server := &Server{
startedAt: startedAt,
opts: ServerOptions{
Paths: Paths{Socket: filepath.Join(dir, "daemon.sock"), Status: path},
Version: 3,
syncStatusParent: func(root *os.Root) error {
if _, err := root.Stat("."); err != nil {
t.Fatalf("stat bound status parent: %v", err)
}
parentSynced = true
return nil
},
},
}
if err := server.writeStatusFile(); err != nil {
t.Fatalf("writeStatusFile: %v", err)
}
data, err := os.ReadFile(path)
if err != nil {
t.Fatal(err)
}
var status StatusFile
if err := json.Unmarshal(data, &status); err != nil {
t.Fatalf("published status is invalid JSON: %v", err)
}
if status.PID != os.Getpid() || status.Socket != server.opts.Paths.Socket || status.Version != 3 || !status.StartedAt.Equal(startedAt) {
t.Fatalf("published status = %+v", status)
}
if !parentSynced {
t.Fatal("status parent directory was not synced")
}
if runtime.GOOS != "windows" {
info, err := os.Stat(path)
if err != nil {
t.Fatal(err)
}
if got := info.Mode().Perm(); got != 0o600 {
t.Fatalf("status mode = %04o, want 0600", got)
}
}
assertNoStatusTemps(t, dir)
}
func TestWriteStatusFileReaderSeesCompleteDocumentsDuringPublication(t *testing.T) {
dir := t.TempDir()
secureStatusTestDir(t, dir)
path := filepath.Join(dir, "daemon.status")
previous := StatusFile{
PID: 7,
Socket: filepath.Join(dir, "old.sock"),
Version: 1,
StartedAt: time.Date(2026, 8, 1, 0, 0, 0, 0, time.UTC),
}
previousData, err := json.Marshal(previous)
if err != nil {
t.Fatal(err)
}
if err := os.WriteFile(path, previousData, 0o600); err != nil {
t.Fatal(err)
}
replacementReady := make(chan struct{})
publish := make(chan struct{})
writeDone := make(chan error, 1)
server := &Server{
startedAt: time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC),
opts: ServerOptions{
Paths: Paths{Socket: filepath.Join(dir, "new.sock"), Status: path},
Version: 2,
replaceStatusFile: func(root *os.Root, src, dst string) error {
close(replacementReady)
<-publish
return fsutil.RenameWithRetry(src, dst, root.Rename)
},
},
}
go func() {
writeDone <- server.writeStatusFile()
}()
<-replacementReady
for range 100 {
status := readStatusDocument(t, path)
if status != previous {
t.Fatalf("status before commit = %+v, want previous document %+v", status, previous)
}
}
close(publish)
if err := <-writeDone; err != nil {
t.Fatalf("writeStatusFile: %v", err)
}
status := readStatusDocument(t, path)
if status.PID != os.Getpid() || status.Socket != server.opts.Paths.Socket || status.Version != 2 || !status.StartedAt.Equal(server.startedAt) {
t.Fatalf("status after commit = %+v", status)
}
assertNoStatusTemps(t, dir)
}
func TestWriteStatusFileContinuesAfterDirectorySyncWarning(t *testing.T) {
dir := t.TempDir()
secureStatusTestDir(t, dir)
path := filepath.Join(dir, "daemon.status")
if err := os.WriteFile(path, []byte(`{"pid":7}`), 0o600); err != nil {
t.Fatal(err)
}
syncErr := errors.New("injected directory sync failure")
var logs []string
server := &Server{
startedAt: time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC),
opts: ServerOptions{
Paths: Paths{Socket: filepath.Join(dir, "daemon.sock"), Status: path},
Version: 4,
Log: func(message string) { logs = append(logs, message) },
syncStatusParent: func(*os.Root) error { return syncErr },
},
}
if err := server.writeStatusFile(); err != nil {
t.Fatalf("writeStatusFile returned a post-commit warning as failure: %v", err)
}
status := readStatusDocument(t, path)
if status.Version != 4 {
t.Fatalf("published status = %+v, want version 4", status)
}
if len(logs) != 1 || !strings.Contains(logs[0], syncErr.Error()) {
t.Fatalf("logs = %q, want directory sync warning", logs)
}
assertNoStatusTemps(t, dir)
}
func TestWriteStatusFileContinuesAfterCommittedReplacementWarning(t *testing.T) {
dir := t.TempDir()
secureStatusTestDir(t, dir)
path := filepath.Join(dir, "daemon.status")
if err := os.WriteFile(path, []byte(`{"pid":7}`), 0o600); err != nil {
t.Fatal(err)
}
cleanupErr := errors.New("injected backup cleanup failure")
parentSynced := false
var logs []string
server := &Server{
startedAt: time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC),
opts: ServerOptions{
Paths: Paths{Socket: filepath.Join(dir, "daemon.sock"), Status: path},
Version: 5,
Log: func(message string) { logs = append(logs, message) },
replaceStatusFile: func(root *os.Root, src, dst string) error {
if err := fsutil.RenameWithRetry(src, dst, root.Rename); err != nil {
return err
}
return &fsutil.CommittedReplacementCleanupError{
BackupPath: filepath.Join(dir, ".zero-replace-backup"),
Cause: cleanupErr,
}
},
syncStatusParent: func(*os.Root) error {
parentSynced = true
return nil
},
},
}
if err := server.writeStatusFile(); err != nil {
t.Fatalf("writeStatusFile returned a post-commit warning as failure: %v", err)
}
if !parentSynced {
t.Fatal("status directory was not synced after committed replacement warning")
}
status := readStatusDocument(t, path)
if status.Version != 5 {
t.Fatalf("published status = %+v, want version 5", status)
}
if len(logs) != 1 || !strings.Contains(logs[0], cleanupErr.Error()) {
t.Fatalf("logs = %q, want committed cleanup warning", logs)
}
assertNoStatusTemps(t, dir)
}
func TestWriteStatusFileContinuesAfterCommittedTempCleanupWarning(t *testing.T) {
dir := t.TempDir()
secureStatusTestDir(t, dir)
path := filepath.Join(dir, "daemon.status")
if err := os.WriteFile(path, []byte(`{"pid":7}`), 0o600); err != nil {
t.Fatal(err)
}
var tempName string
var logs []string
server := &Server{
startedAt: time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC),
opts: ServerOptions{
Paths: Paths{Socket: filepath.Join(dir, "daemon.sock"), Status: path},
Version: 6,
Log: func(message string) { logs = append(logs, message) },
replaceStatusFile: func(root *os.Root, src, dst string) error {
if err := fsutil.RenameWithRetry(src, dst, root.Rename); err != nil {
return err
}
tempName = src
if err := root.Mkdir(src, 0o700); err != nil {
t.Fatalf("recreate staged path as directory: %v", err)
}
keep, err := root.OpenFile(filepath.Join(src, "keep"), os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600)
if err != nil {
t.Fatalf("make staged directory non-empty: %v", err)
}
if err := keep.Close(); err != nil {
t.Fatalf("close staged directory marker: %v", err)
}
return nil
},
},
}
if err := server.writeStatusFile(); err != nil {
t.Fatalf("writeStatusFile returned a post-commit warning as failure: %v", err)
}
status := readStatusDocument(t, path)
if status.Version != 6 {
t.Fatalf("published status = %+v, want version 6", status)
}
if len(logs) != 1 || !strings.Contains(logs[0], "remove temporary status file") {
t.Fatalf("logs = %q, want temporary cleanup warning", logs)
}
if err := os.Remove(filepath.Join(dir, tempName, "keep")); err != nil {
t.Fatal(err)
}
if err := os.Remove(filepath.Join(dir, tempName)); err != nil {
t.Fatal(err)
}
assertNoStatusTemps(t, dir)
}
func TestWriteStatusFileBindsDirectoryDuringAncestorSwap(t *testing.T) {
parent := t.TempDir()
dir := filepath.Join(parent, "live")
movedDir := filepath.Join(parent, "moved")
if err := os.Mkdir(dir, 0o700); err != nil {
t.Fatal(err)
}
secureStatusTestDir(t, dir)
path := filepath.Join(dir, "daemon.status")
if err := os.WriteFile(path, []byte(`{"pid":7}`), 0o600); err != nil {
t.Fatal(err)
}
substitute := []byte(`{"pid":999,"socket":"substitute"}`)
var swapErr error
server := &Server{
startedAt: time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC),
opts: ServerOptions{
Paths: Paths{Socket: filepath.Join(dir, "daemon.sock"), Status: path},
Version: 6,
beforeStatusReplace: func() {
swapErr = os.Rename(dir, movedDir)
if swapErr != nil {
if runtime.GOOS == "windows" {
return
}
t.Fatalf("move bound status directory: %v", swapErr)
}
if err := os.Mkdir(dir, 0o700); err != nil {
t.Fatalf("create substitute status directory: %v", err)
}
if err := os.WriteFile(path, substitute, 0o600); err != nil {
t.Fatalf("write substitute status: %v", err)
}
},
},
}
if err := server.writeStatusFile(); err != nil {
t.Fatalf("writeStatusFile: %v", err)
}
if swapErr != nil {
if runtime.GOOS != "windows" {
t.Fatalf("unexpected directory-swap error: %v", swapErr)
}
status := readStatusDocument(t, path)
if status.Version != 6 {
t.Fatalf("status after blocked swap = %+v, want version 6", status)
}
if _, err := os.Lstat(movedDir); !os.IsNotExist(err) {
t.Fatalf("moved directory exists after blocked Windows swap: %v", err)
}
assertNoStatusTemps(t, dir)
return
}
if got, err := os.ReadFile(path); err != nil {
t.Fatal(err)
} else if string(got) != string(substitute) {
t.Fatalf("substitute destination changed: %q", got)
}
status := readStatusDocument(t, filepath.Join(movedDir, "daemon.status"))
if status.Version != 6 {
t.Fatalf("bound status = %+v, want version 6", status)
}
assertNoStatusTemps(t, dir)
assertNoStatusTemps(t, movedDir)
}
func TestWriteStatusFileAllowsReadOnlyCustomStatusDirectory(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("Windows directory access is governed by DACLs, not Unix mode bits")
}
dir := t.TempDir()
if err := os.Chmod(dir, 0o755); err != nil {
t.Fatal(err)
}
path := filepath.Join(dir, "daemon.status")
server := &Server{
startedAt: time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC),
opts: ServerOptions{
Paths: Paths{Status: path},
Version: 6,
},
}
err := server.writeStatusFile()
if err != nil {
t.Fatalf("writeStatusFile in 0755 directory: %v", err)
}
if _, err := os.Lstat(path); err != nil {
t.Fatalf("status file not created in read-only custom directory: %v", err)
}
assertNoStatusTemps(t, dir)
}
func TestWriteStatusFileRejectsWritableStatusDirectory(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("Windows directory access is governed by DACLs, not Unix mode bits")
}
dir := t.TempDir()
if err := os.Chmod(dir, 0o777); err != nil {
t.Fatal(err)
}
path := filepath.Join(dir, "daemon.status")
server := &Server{
startedAt: time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC),
opts: ServerOptions{Paths: Paths{Status: path}, Version: 6},
}
err := server.writeStatusFile()
if err == nil || !strings.Contains(err.Error(), "no group/other write access") {
t.Fatalf("writeStatusFile error = %v, want writable-directory rejection", err)
}
if _, err := os.Lstat(path); !os.IsNotExist(err) {
t.Fatalf("status file created in writable directory: %v", err)
}
assertNoStatusTemps(t, dir)
}
func TestPrivateDirHardensBroadCurrentUserStatusDirectory(t *testing.T) {
dir := t.TempDir()
broadenStatusTestDirPlatform(t, dir)
if err := privatedir.Ensure(dir); err != nil {
t.Fatalf("privatedir.Ensure: %v", err)
}
root, err := os.OpenRoot(dir)
if err != nil {
t.Fatal(err)
}
defer root.Close()
if err := validateStatusRoot(root); err != nil {
t.Fatalf("validate hardened status root: %v", err)
}
}
func readStatusDocument(t *testing.T, path string) StatusFile {
t.Helper()
data, err := os.ReadFile(path)
if err != nil {
t.Fatalf("read status: %v", err)
}
var status StatusFile
if err := json.Unmarshal(data, &status); err != nil {
t.Fatalf("status is incomplete JSON: %v (content %q)", err, data)
}
return status
}
func assertNoStatusTemps(t *testing.T, dir string) {
t.Helper()
matches, err := filepath.Glob(filepath.Join(dir, statusTempPattern))
if err != nil {
t.Fatal(err)
}
if len(matches) != 0 {
t.Fatalf("temporary status files remain: %v", matches)
}
}
func secureStatusTestDir(t *testing.T, dir string) {
t.Helper()
secureStatusTestDirPlatform(t, dir)
}
@@ -1,63 +0,0 @@
//go:build windows
package daemon
import (
"os"
"path/filepath"
"strings"
"testing"
"time"
"golang.org/x/sys/windows"
)
func TestWriteStatusFilePreservesExistingProtectedDACL(t *testing.T) {
dir := t.TempDir()
secureStatusTestDir(t, dir)
path := filepath.Join(dir, "daemon.status")
if err := os.WriteFile(path, []byte(`{"pid":7}`), 0o600); err != nil {
t.Fatal(err)
}
restricted, err := windows.SecurityDescriptorFromString("D:P(A;;FA;;;OW)")
if err != nil {
t.Skipf("cannot build restrictive DACL: %v", err)
}
dacl, _, err := restricted.DACL()
if err != nil {
t.Skipf("cannot read restrictive DACL: %v", err)
}
if err := windows.SetNamedSecurityInfo(path, windows.SE_FILE_OBJECT,
windows.DACL_SECURITY_INFORMATION|windows.PROTECTED_DACL_SECURITY_INFORMATION,
nil, nil, dacl, nil); err != nil {
t.Skipf("cannot apply restrictive DACL: %v", err)
}
want := statusDACL(t, path)
server := &Server{
startedAt: time.Date(2026, 8, 30, 18, 0, 0, 0, time.UTC),
opts: ServerOptions{
Paths: Paths{Socket: filepath.Join(dir, "daemon.sock"), Status: path},
Version: 2,
},
}
if err := server.writeStatusFile(); err != nil {
t.Fatalf("writeStatusFile: %v", err)
}
if got := statusDACL(t, path); got != want {
t.Fatalf("status DACL after replacement = %q, want %q", got, want)
}
}
func statusDACL(t *testing.T, path string) string {
t.Helper()
descriptor, err := windows.GetNamedSecurityInfo(path, windows.SE_FILE_OBJECT, windows.DACL_SECURITY_INFORMATION)
if err != nil {
t.Skipf("cannot read status DACL: %v", err)
}
text := descriptor.String()
if index := strings.Index(text, "D:"); index >= 0 {
return text[index:]
}
return text
}
-7
View File
@@ -1,7 +0,0 @@
//go:build !windows
package daemon
import "os"
func prepareStatusReplacement(*os.Root, string, string) error { return nil }
-136
View File
@@ -1,136 +0,0 @@
//go:build windows
package daemon
import (
"errors"
"fmt"
"os"
"path/filepath"
"unsafe"
"golang.org/x/sys/windows"
)
// prepareStatusReplacement copies the existing status document's DACL to the
// complete staging file before Root.Rename replaces it. Both files are opened
// relative to the already-bound Root directory handle, so preserving Windows
// security metadata does not reintroduce a pathname/ancestor-swap boundary.
func prepareStatusReplacement(root *os.Root, src, dst string) (returnErr error) {
if filepath.Base(src) != src || filepath.Base(dst) != dst {
return fmt.Errorf("status replacement names must be root-relative base names")
}
directory, err := root.Open(".")
if err != nil {
return fmt.Errorf("open bound status directory for DACL preservation: %w", err)
}
defer func() {
if err := directory.Close(); err != nil {
returnErr = errors.Join(returnErr, fmt.Errorf("close bound status directory for DACL preservation: %w", err))
}
}()
raw, err := directory.SyscallConn()
if err != nil {
return fmt.Errorf("access bound status directory handle: %w", err)
}
var destination, source windows.Handle
var openErr error
if err := raw.Control(func(rootHandle uintptr) {
destination, openErr = openStatusSecurityHandle(windows.Handle(rootHandle), dst, windows.READ_CONTROL|windows.FILE_READ_ATTRIBUTES)
if isMissingStatusObject(openErr) {
openErr = nil // no destination descriptor exists to preserve
return
}
if openErr != nil {
return
}
source, openErr = openStatusSecurityHandle(windows.Handle(rootHandle), src, windows.READ_CONTROL|windows.WRITE_DAC|windows.FILE_READ_ATTRIBUTES)
}); err != nil {
return fmt.Errorf("open status files through bound directory: %w", err)
}
if destination != 0 {
defer windows.CloseHandle(destination)
}
if source != 0 {
defer windows.CloseHandle(source)
}
if openErr != nil {
return fmt.Errorf("open status files for DACL preservation: %w", openErr)
}
if destination == 0 {
return nil
}
descriptor, err := windows.GetSecurityInfo(destination, windows.SE_FILE_OBJECT, windows.DACL_SECURITY_INFORMATION)
if err != nil {
return fmt.Errorf("read existing status DACL: %w", err)
}
dacl, _, err := descriptor.DACL()
if err != nil {
return fmt.Errorf("decode existing status DACL: %w", err)
}
if dacl == nil {
return fmt.Errorf("existing status file has an unrestricted Windows DACL")
}
control, _, err := descriptor.Control()
if err != nil {
return fmt.Errorf("read existing status DACL control: %w", err)
}
securityInfo := windows.SECURITY_INFORMATION(windows.DACL_SECURITY_INFORMATION)
if control&windows.SE_DACL_PROTECTED != 0 {
securityInfo |= windows.SECURITY_INFORMATION(windows.PROTECTED_DACL_SECURITY_INFORMATION)
} else {
securityInfo |= windows.SECURITY_INFORMATION(windows.UNPROTECTED_DACL_SECURITY_INFORMATION)
}
if err := windows.SetSecurityInfo(source, windows.SE_FILE_OBJECT, securityInfo, nil, nil, dacl, nil); err != nil {
return fmt.Errorf("preserve existing status DACL on replacement: %w", err)
}
return nil
}
func openStatusSecurityHandle(root windows.Handle, name string, access uint32) (windows.Handle, error) {
objectName, err := windows.NewNTUnicodeString(name)
if err != nil {
return 0, err
}
attributes := &windows.OBJECT_ATTRIBUTES{
Length: uint32(unsafe.Sizeof(windows.OBJECT_ATTRIBUTES{})),
RootDirectory: root,
ObjectName: objectName,
Attributes: windows.OBJ_CASE_INSENSITIVE | windows.OBJ_DONT_REPARSE,
}
var handle windows.Handle
var status windows.IO_STATUS_BLOCK
err = windows.NtCreateFile(
&handle,
access,
attributes,
&status,
nil,
0,
windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE,
windows.FILE_OPEN,
windows.FILE_NON_DIRECTORY_FILE|windows.FILE_OPEN_REPARSE_POINT,
0,
0,
)
if err != nil {
return 0, err
}
var info windows.ByHandleFileInformation
if err := windows.GetFileInformationByHandle(handle, &info); err != nil {
windows.CloseHandle(handle)
return 0, err
}
if info.FileAttributes&windows.FILE_ATTRIBUTE_REPARSE_POINT != 0 {
windows.CloseHandle(handle)
return 0, fmt.Errorf("refusing status DACL operation on reparse point %q", name)
}
return handle, nil
}
func isMissingStatusObject(err error) bool {
return errors.Is(err, windows.STATUS_OBJECT_NAME_NOT_FOUND) ||
errors.Is(err, windows.STATUS_OBJECT_PATH_NOT_FOUND)
}
-57
View File
@@ -64,28 +64,6 @@ func TryAcquireFileLockAt(root, path string) (*FileLock, error) {
if err != nil {
return nil, fmt.Errorf("lockutil: open lock file: %w", err)
}
return acquireOpenedFileLock(file)
}
// TryAcquireFileLockRoot attempts to lock name relative to an already-bound
// directory root. It is intended for lifecycle owners that must not resolve the
// root pathname again between validation and lock acquisition.
func TryAcquireFileLockRoot(root *os.Root, name, displayPath string) (*FileLock, error) {
if root == nil {
return nil, errors.New("lockutil: lock root is nil")
}
name = filepath.Clean(name)
if name == "." || name == ".." || filepath.IsAbs(name) || strings.HasPrefix(name, ".."+string(filepath.Separator)) {
return nil, fmt.Errorf("lockutil: lock name %q escapes root", name)
}
file, err := openLockFileRoot(root, name, displayPath)
if err != nil {
return nil, fmt.Errorf("lockutil: open rooted lock file: %w", err)
}
return acquireOpenedFileLock(file)
}
func acquireOpenedFileLock(file *os.File) (*FileLock, error) {
state, contended, err := tryLockFile(file)
if err != nil {
_ = file.Close()
@@ -104,41 +82,6 @@ func acquireOpenedFileLock(file *os.File) (*FileLock, error) {
return lock, nil
}
func openLockFileRoot(root *os.Root, name, displayPath string) (*os.File, error) {
info, err := root.Lstat(name)
if errors.Is(err, os.ErrNotExist) {
file, createErr := root.OpenFile(name, os.O_CREATE|os.O_EXCL|os.O_RDWR, 0o600)
if createErr != nil {
return nil, createErr
}
if err := validateRootLockFile(file); err != nil {
return nil, errors.Join(err, file.Close())
}
return file, nil
}
if err != nil {
return nil, err
}
if !info.Mode().IsRegular() {
return nil, errors.New("refusing non-regular rooted lock file")
}
file, err := root.OpenFile(name, os.O_RDWR, 0o600)
if err != nil {
return nil, err
}
opened, err := file.Stat()
if err != nil {
return nil, errors.Join(err, file.Close())
}
if !os.SameFile(info, opened) {
return nil, errors.Join(errors.New("rooted lock file changed while opening"), file.Close())
}
if err := validateRootLockFile(file); err != nil {
return nil, errors.Join(err, file.Close())
}
return file, nil
}
// WriteMetadata replaces the diagnostic contents of the lock file while the
// caller holds it. Metadata does not establish ownership; the kernel lock does.
func (lock *FileLock) WriteMetadata(data []byte) error {
-14
View File
@@ -76,20 +76,6 @@ func openLockFileAt(root, relative, displayPath string) (_ *os.File, resultErr e
return file, nil
}
func validateRootLockFile(file *os.File) error {
var stat unix.Stat_t
if err := unix.Fstat(int(file.Fd()), &stat); err != nil {
return fmt.Errorf("inspect rooted lock file: %w", err)
}
if stat.Mode&unix.S_IFMT != unix.S_IFREG {
return errors.New("refusing non-regular rooted lock file")
}
if stat.Nlink != 1 {
return errors.New("refusing multiply-linked rooted lock file")
}
return nil
}
func tryLockFile(file *os.File) (platformLockState, bool, error) {
err := unix.Flock(int(file.Fd()), unix.LOCK_EX|unix.LOCK_NB)
if err == nil {
-4
View File
@@ -73,10 +73,6 @@ func openLockFileAt(root, relative, displayPath string) (_ *os.File, resultErr e
return file, nil
}
func validateRootLockFile(file *os.File) error {
return validateWindowsLockHandle(windows.Handle(file.Fd()), false)
}
func openLockDirectoryAt(parent windows.Handle, name string) (windows.Handle, error) {
handle, err := openLockPathAt(
parent,
+5 -225
View File
@@ -7,50 +7,17 @@
package observability
import (
"crypto/rand"
"encoding/hex"
"errors"
"fmt"
"io"
"os"
"path/filepath"
"runtime/debug"
"strings"
"time"
"github.com/Gitlawb/zero/internal/privatedir"
)
// crashExitCode is returned when a top-level panic is recovered.
const crashExitCode = 1
const crashTempPrefix = ".crash-report-"
// ErrCrashReportCommitted marks an error that happened after a complete crash
// report was published. Callers may still use the returned path and should
// surface the error as a cleanup warning rather than as a failed publication.
var ErrCrashReportCommitted = errors.New("crash report publication committed")
type crashReportCommittedError struct {
cause error
}
func (err *crashReportCommittedError) Error() string {
return fmt.Sprintf("%v with warning: %v", ErrCrashReportCommitted, err.cause)
}
func (err *crashReportCommittedError) Unwrap() []error {
return []error{ErrCrashReportCommitted, err.cause}
}
type crashReportHooks struct {
beforePublish func()
write func(*os.File, []byte) (int, error)
link func(*os.Root, string, string) error
renameNoReplace func(*os.Root, string, string) error
remove func(*os.Root, string) error
}
// FormatCrashReport renders a human-readable crash report.
func FormatCrashReport(label string, recovered any, stack []byte, ts time.Time) string {
return fmt.Sprintf("zero crash report\ntime: %s\nlabel: %s\npanic: %v\n\nstack:\n%s\n",
@@ -59,189 +26,16 @@ func FormatCrashReport(label string, recovered any, stack []byte, ts time.Time)
// WriteCrashReport writes a crash report file into dir and returns its path.
func WriteCrashReport(dir, label string, recovered any, stack []byte, ts time.Time) (string, error) {
return writeCrashReport(dir, label, recovered, stack, ts, crashReportHooks{})
}
func writeCrashReport(dir, label string, recovered any, stack []byte, ts time.Time, hooks crashReportHooks) (path string, returnErr error) {
absoluteDir, err := filepath.Abs(dir)
if err != nil {
return "", fmt.Errorf("resolve crash report directory: %w", err)
}
root, err := openCrashDirectory(dir)
if err != nil {
if err := os.MkdirAll(dir, 0o755); err != nil {
return "", err
}
committed := false
defer func() {
if err := root.Close(); err != nil {
returnErr = errors.Join(returnErr, fmt.Errorf("close crash report directory: %w", err))
}
if committed && returnErr != nil && !errors.Is(returnErr, ErrCrashReportCommitted) {
returnErr = &crashReportCommittedError{cause: returnErr}
}
if !committed && returnErr != nil {
path = ""
}
}()
name := "crash-" + ts.UTC().Format("20060102-150405") + ".log"
path = filepath.Join(dir, name)
report, tempName, err := createCrashTemp(root)
if err != nil {
path := filepath.Join(dir, "crash-"+ts.UTC().Format("20060102-150405")+".log")
if err := os.WriteFile(path, []byte(FormatCrashReport(label, recovered, stack, ts)), 0o600); err != nil {
return "", err
}
defer func() {
if tempName == "" {
return
}
if err := removeCrashTemp(hooks, root, tempName); err != nil && !errors.Is(err, os.ErrNotExist) {
returnErr = errors.Join(returnErr, fmt.Errorf("remove temporary crash report: %w", err))
}
}()
data := []byte(FormatCrashReport(label, recovered, stack, ts))
writeReport := hooks.write
if writeReport == nil {
writeReport = func(file *os.File, data []byte) (int, error) { return file.Write(data) }
}
written, err := writeReport(report, data)
if err == nil && written != len(data) {
err = io.ErrShortWrite
}
if err != nil {
closeErr := report.Close()
if closeErr != nil {
return "", errors.Join(fmt.Errorf("write crash report: %w", err), fmt.Errorf("close crash report: %w", closeErr))
}
return "", fmt.Errorf("write crash report: %w", err)
}
if err := report.Sync(); err != nil {
closeErr := report.Close()
if closeErr != nil {
return "", errors.Join(fmt.Errorf("sync crash report: %w", err), fmt.Errorf("close crash report: %w", closeErr))
}
return "", fmt.Errorf("sync crash report: %w", err)
}
if err := report.Close(); err != nil {
return "", fmt.Errorf("close crash report: %w", err)
}
if hooks.beforePublish != nil {
hooks.beforePublish()
}
publishName := name
link := root.Link
if hooks.link != nil {
link = func(oldname, newname string) error { return hooks.link(root, oldname, newname) }
}
if linkErr := link(tempName, name); linkErr != nil {
publishName, err = publishCrashFallback(root, name, tempName, hooks.renameNoReplace)
if err != nil {
return "", errors.Join(fmt.Errorf("publish crash report with hard link: %w", linkErr), err)
}
committed = true
tempName = ""
} else {
committed = true
}
path = filepath.Join(dir, publishName)
if !crashPathUsesRoot(root, absoluteDir) {
path = ""
}
if tempName != "" {
if err := removeCrashTemp(hooks, root, tempName); err != nil && !errors.Is(err, os.ErrNotExist) {
tempName = ""
return path, fmt.Errorf("remove temporary crash report: %w", err)
}
tempName = ""
}
return path, nil
}
// publishCrashFallback atomically renames the complete staging file to an
// unpredictable unused name. The no-replace operation is the collision check;
// there is no check-then-rename window in which another report can be replaced.
func publishCrashFallback(root *os.Root, timestampName, tempName string, rename func(*os.Root, string, string) error) (string, error) {
if rename == nil {
rename = renameNoReplace
}
base := strings.TrimSuffix(timestampName, filepath.Ext(timestampName))
if suffix := strings.TrimPrefix(tempName, crashTempPrefix); suffix != "" && suffix != tempName {
candidate := base + "-" + suffix + filepath.Ext(timestampName)
if err := rename(root, tempName, candidate); err == nil {
return candidate, nil
} else if !errors.Is(err, os.ErrExist) {
return "", fmt.Errorf("publish fallback crash report: %w", err)
}
}
for range 100 {
var suffix [16]byte
if _, err := rand.Read(suffix[:]); err != nil {
return "", fmt.Errorf("generate fallback crash report name: %w", err)
}
candidate := base + "-" + hex.EncodeToString(suffix[:]) + filepath.Ext(timestampName)
if err := rename(root, tempName, candidate); err == nil {
return candidate, nil
} else if !errors.Is(err, os.ErrExist) {
return "", fmt.Errorf("publish fallback crash report: %w", err)
}
}
return "", fmt.Errorf("generate fallback crash report name: exhausted unique names")
}
func removeCrashTemp(hooks crashReportHooks, root *os.Root, name string) error {
if hooks.remove != nil {
return hooks.remove(root, name)
}
return root.Remove(name)
}
func createCrashTemp(root *os.Root) (*os.File, string, error) {
for range 100 {
var suffix [16]byte
if _, err := rand.Read(suffix[:]); err != nil {
return nil, "", fmt.Errorf("generate temporary crash report name: %w", err)
}
name := crashTempPrefix + hex.EncodeToString(suffix[:])
file, err := root.OpenFile(name, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600)
if err == nil {
return file, name, nil
}
if errors.Is(err, os.ErrExist) {
continue
}
return nil, "", fmt.Errorf("create temporary crash report: %w", err)
}
return nil, "", fmt.Errorf("create temporary crash report: exhausted unique names")
}
func crashPathUsesRoot(root *os.Root, dir string) bool {
bound, err := root.Stat(".")
if err != nil {
return false
}
current, err := os.Stat(filepath.Clean(dir))
return err == nil && os.SameFile(bound, current)
}
func openCrashDirectory(dir string) (*os.Root, error) {
clean := filepath.Clean(dir)
defaultDir := filepath.Clean(DefaultCrashDir())
parent := filepath.Dir(clean)
// The default layout shares ~/.zero with the daemon runtime fallback. Keep
// both the shared parent and the crash-report child private. Custom crash
// destinations are hardened at the caller-supplied boundary only.
if clean == defaultDir && filepath.Base(clean) == "crashes" && filepath.Base(parent) == ".zero" {
if err := privatedir.Ensure(parent); err != nil {
return nil, fmt.Errorf("secure crash report parent: %w", err)
}
}
root, err := privatedir.Open(clean)
if err != nil {
return nil, fmt.Errorf("secure crash report directory: %w", err)
}
return root, nil
}
// DefaultCrashDir is where crash reports are written by default.
func DefaultCrashDir() string {
if home, err := os.UserHomeDir(); err == nil && home != "" {
@@ -260,22 +54,8 @@ func Recover(dir, label string, stderr io.Writer, code *int) {
return
}
stack := debug.Stack()
reportRecoveredCrash(stderr, code, recovered, stack, func() (string, error) {
return WriteCrashReport(dir, label, recovered, stack, time.Now())
})
}
func reportRecoveredCrash(stderr io.Writer, code *int, recovered any, stack []byte, write func() (string, error)) {
path, err := write()
if err == nil || errors.Is(err, ErrCrashReportCommitted) {
if path != "" {
fmt.Fprintf(stderr, "zero crashed: %v\nA crash report was saved to %s\n", recovered, path)
} else {
fmt.Fprintf(stderr, "zero crashed: %v\nA crash report was saved, but its current path could not be determined\n", recovered)
}
if err != nil {
fmt.Fprintf(stderr, "Warning: %v\n", err)
}
if path, err := WriteCrashReport(dir, label, recovered, stack, time.Now()); err == nil {
fmt.Fprintf(stderr, "zero crashed: %v\nA crash report was saved to %s\n", recovered, path)
} else {
fmt.Fprintf(stderr, "zero crashed: %v\n%s\n", recovered, stack)
}
-419
View File
@@ -2,10 +2,7 @@ package observability
import (
"bytes"
"errors"
"os"
"path/filepath"
"runtime"
"strings"
"testing"
"time"
@@ -30,400 +27,6 @@ func TestWriteAndFormatCrashReport(t *testing.T) {
}
}
func TestWriteCrashReportKeepsTwoReportsFromTheSameSecond(t *testing.T) {
dir := t.TempDir()
ts := time.Date(2026, 8, 30, 18, 0, 0, 0, time.UTC)
first, err := WriteCrashReport(dir, "cli", "first panic", []byte("first stack"), ts)
if err != nil {
t.Fatal(err)
}
second, err := WriteCrashReport(dir, "cli", "second panic", []byte("second stack"), ts)
if err != nil {
t.Fatalf("second report in the same second: %v", err)
}
if first == second {
t.Fatalf("same-second reports shared path %q", first)
}
firstData, err := os.ReadFile(first)
if err != nil {
t.Fatal(err)
}
secondData, err := os.ReadFile(second)
if err != nil {
t.Fatal(err)
}
if !strings.Contains(string(firstData), "first panic") || strings.Contains(string(firstData), "second panic") {
t.Fatalf("first report was replaced: %q", firstData)
}
if !strings.Contains(string(secondData), "second panic") {
t.Fatalf("second report was not persisted: %q", secondData)
}
}
func TestWriteCrashReportCreatesPrivateDefaultDirectories(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
dir := DefaultCrashDir()
if dir != filepath.Join(home, ".zero", "crashes") {
t.Fatalf("DefaultCrashDir = %q, want path beneath temporary home", dir)
}
if _, err := WriteCrashReport(dir, "cli", "boom", []byte("stack"), time.Now()); err != nil {
t.Fatalf("WriteCrashReport: %v", err)
}
if runtime.GOOS == "windows" {
return
}
for _, path := range []string{filepath.Join(home, ".zero"), dir} {
info, err := os.Stat(path)
if err != nil {
t.Fatal(err)
}
if got := info.Mode().Perm(); got&0o077 != 0 {
t.Fatalf("directory %s permissions = %04o, want owner-only", path, got)
}
}
}
func TestWriteCrashReportHardensPreexistingDefaultDirectories(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("Windows DACL migration is covered by the daemon integration test")
}
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
dir := DefaultCrashDir()
if err := os.MkdirAll(dir, 0o755); err != nil {
t.Fatal(err)
}
for _, path := range []string{filepath.Join(home, ".zero"), dir} {
if err := os.Chmod(path, 0o755); err != nil {
t.Fatal(err)
}
}
if _, err := WriteCrashReport(dir, "cli", "boom", []byte("stack"), time.Now()); err != nil {
t.Fatalf("WriteCrashReport: %v", err)
}
for _, path := range []string{filepath.Join(home, ".zero"), dir} {
info, err := os.Stat(path)
if err != nil {
t.Fatal(err)
}
if got := info.Mode().Perm(); got&0o077 != 0 {
t.Fatalf("directory %s permissions = %04o after migration, want owner-only", path, got)
}
}
}
func TestWriteCrashReportBindsDirectoryDuringSwap(t *testing.T) {
parent := t.TempDir()
dir := filepath.Join(parent, "live")
movedDir := filepath.Join(parent, "moved")
if err := os.Mkdir(dir, 0o700); err != nil {
t.Fatal(err)
}
ts := time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC)
var swapErr error
path, err := writeCrashReport(dir, "cli", "secret panic", []byte("secret stack"), ts, crashReportHooks{
beforePublish: func() {
swapErr = os.Rename(dir, movedDir)
if swapErr != nil {
return
}
if err := os.Mkdir(dir, 0o700); err != nil {
t.Fatalf("create substitute crash directory: %v", err)
}
},
})
if err != nil {
t.Fatalf("writeCrashReport: %v", err)
}
if swapErr != nil {
if runtime.GOOS != "windows" {
t.Fatalf("swap crash directory: %v", swapErr)
}
if _, err := os.Stat(path); err != nil {
t.Fatalf("report missing after Windows blocked directory swap: %v", err)
}
return
}
if path != "" {
t.Fatalf("writeCrashReport returned stale path %q after directory swap", path)
}
reportName := "crash-" + ts.UTC().Format("20060102-150405") + ".log"
data, err := os.ReadFile(filepath.Join(movedDir, reportName))
if err != nil {
t.Fatalf("read report through originally bound directory: %v", err)
}
if !strings.Contains(string(data), "secret panic") {
t.Fatalf("report in bound directory missing panic: %q", data)
}
if _, err := os.Stat(filepath.Join(dir, reportName)); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("substitute directory received crash report: %v", err)
}
}
func TestWriteCrashReportPublishesOnlyCompleteContent(t *testing.T) {
dir := t.TempDir()
ts := time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC)
finalPath := filepath.Join(dir, "crash-20260824-120000.log")
staged := make(chan struct{})
release := make(chan struct{})
result := make(chan crashWriteResult, 1)
go func() {
path, err := writeCrashReport(dir, "cli", "boom", []byte("complete stack"), ts, crashReportHooks{
beforePublish: func() {
close(staged)
<-release
},
})
result <- crashWriteResult{path: path, err: err}
}()
select {
case <-staged:
case written := <-result:
t.Fatalf("writeCrashReport returned before publication hook: %v", written.err)
case <-time.After(time.Second):
t.Fatal("writeCrashReport did not reach publication hook")
}
defer func() {
select {
case <-release:
default:
close(release)
}
}()
if _, err := os.Stat(finalPath); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("final report visible before complete publication: %v", err)
}
close(release)
written := <-result
if written.err != nil {
t.Fatalf("writeCrashReport: %v", written.err)
}
data, err := os.ReadFile(written.path)
if err != nil {
t.Fatal(err)
}
if !strings.Contains(string(data), "complete stack") {
t.Fatalf("published report is incomplete: %q", data)
}
}
func TestWriteCrashReportRemovesTempAfterWriteFailure(t *testing.T) {
dir := t.TempDir()
injected := errors.New("injected write failure")
_, err := writeCrashReport(dir, "cli", "boom", []byte("stack"), time.Now(), crashReportHooks{
write: func(*os.File, []byte) (int, error) { return 0, injected },
})
if !errors.Is(err, injected) {
t.Fatalf("writeCrashReport error = %v, want injected failure", err)
}
entries, err := os.ReadDir(dir)
if err != nil {
t.Fatal(err)
}
if len(entries) != 0 {
t.Fatalf("crash directory contains partial files after failure: %v", entries)
}
}
func TestWriteCrashReportDoesNotOverwriteExistingReport(t *testing.T) {
dir := t.TempDir()
ts := time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC)
path := filepath.Join(dir, "crash-20260824-120000.log")
if err := os.WriteFile(path, []byte("existing"), 0o600); err != nil {
t.Fatal(err)
}
published, err := WriteCrashReport(dir, "cli", "boom", []byte("stack"), ts)
if err != nil {
t.Fatalf("WriteCrashReport with existing timestamp: %v", err)
}
if published == path {
t.Fatal("new report reused the occupied timestamp path")
}
data, err := os.ReadFile(path)
if err != nil {
t.Fatal(err)
}
if string(data) != "existing" {
t.Fatalf("existing report overwritten: %q", data)
}
entries, err := os.ReadDir(dir)
if err != nil {
t.Fatal(err)
}
if len(entries) != 2 {
t.Fatalf("crash directory should contain existing and uniquely published reports: %v", entries)
}
}
func TestWriteCrashReportPreservesPathAfterCommittedCleanupWarning(t *testing.T) {
dir := t.TempDir()
ts := time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC)
injected := errors.New("injected cleanup failure")
path, err := writeCrashReport(dir, "cli", "boom", []byte("complete stack"), ts, crashReportHooks{
remove: func(*os.Root, string) error { return injected },
})
if !errors.Is(err, ErrCrashReportCommitted) || !errors.Is(err, injected) {
t.Fatalf("writeCrashReport error = %v, want committed cleanup warning", err)
}
wantPath := filepath.Join(dir, "crash-20260824-120000.log")
if path != wantPath {
t.Fatalf("writeCrashReport path = %q, want %q", path, wantPath)
}
data, readErr := os.ReadFile(path)
if readErr != nil {
t.Fatalf("read committed crash report: %v", readErr)
}
if !strings.Contains(string(data), "complete stack") {
t.Fatalf("committed crash report is incomplete: %q", data)
}
}
func TestWriteCrashReportDoesNotReturnStalePathWithCleanupWarning(t *testing.T) {
parent := t.TempDir()
dir := filepath.Join(parent, "live")
movedDir := filepath.Join(parent, "moved")
if err := os.Mkdir(dir, 0o700); err != nil {
t.Fatal(err)
}
injected := errors.New("injected cleanup failure")
ts := time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC)
var swapErr error
path, err := writeCrashReport(dir, "cli", "boom", []byte("complete stack"), ts, crashReportHooks{
beforePublish: func() {
swapErr = os.Rename(dir, movedDir)
if swapErr == nil {
if err := os.Mkdir(dir, 0o700); err != nil {
t.Fatalf("create substitute crash directory: %v", err)
}
}
},
remove: func(*os.Root, string) error { return injected },
})
if swapErr != nil {
if runtime.GOOS == "windows" {
t.Skipf("Windows kept the open crash directory from being renamed: %v", swapErr)
}
t.Fatalf("swap crash directory: %v", swapErr)
}
if !errors.Is(err, ErrCrashReportCommitted) || !errors.Is(err, injected) {
t.Fatalf("writeCrashReport error = %v, want committed cleanup warning", err)
}
if path != "" {
t.Fatalf("writeCrashReport returned stale path %q after directory swap", path)
}
report := filepath.Join(movedDir, "crash-20260824-120000.log")
data, readErr := os.ReadFile(report)
if readErr != nil {
t.Fatalf("read committed report through moved directory: %v", readErr)
}
if !strings.Contains(string(data), "complete stack") {
t.Fatalf("committed report is incomplete: %q", data)
}
}
func TestWriteCrashReportFallsBackWhenHardLinksAreUnavailable(t *testing.T) {
dir := t.TempDir()
ts := time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC)
unsupported := errors.New("hard links unavailable")
var collisionName string
path, err := writeCrashReport(dir, "cli", "boom", []byte("complete stack"), ts, crashReportHooks{
link: func(root *os.Root, oldname, _ string) error {
suffix := strings.TrimPrefix(oldname, crashTempPrefix)
collisionName = "crash-20260824-120000-" + suffix + ".log"
if err := root.WriteFile(collisionName, []byte("existing"), 0o600); err != nil {
t.Fatalf("create fallback-name collision: %v", err)
}
return unsupported
},
})
if err != nil {
t.Fatalf("writeCrashReport fallback: %v", err)
}
if path == filepath.Join(dir, collisionName) {
t.Fatalf("fallback replaced the existing report %q", collisionName)
}
if !strings.HasPrefix(filepath.Base(path), "crash-20260824-120000-") {
t.Fatalf("fallback path = %q, want timestamp and random suffix", path)
}
data, readErr := os.ReadFile(path)
if readErr != nil {
t.Fatalf("read fallback crash report: %v", readErr)
}
if !strings.Contains(string(data), "complete stack") {
t.Fatalf("fallback crash report is incomplete: %q", data)
}
existing, readErr := os.ReadFile(filepath.Join(dir, collisionName))
if readErr != nil {
t.Fatalf("read existing collision: %v", readErr)
}
if string(existing) != "existing" {
t.Fatalf("existing fallback collision was overwritten: %q", existing)
}
entries, readErr := os.ReadDir(dir)
if readErr != nil {
t.Fatal(readErr)
}
if len(entries) != 2 {
t.Fatalf("crash directory contains partial files after fallback: %v", entries)
}
}
func TestWriteCrashReportFallbackDoesNotReplaceCommitTimeCollision(t *testing.T) {
dir := t.TempDir()
ts := time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC)
unsupported := errors.New("hard links unavailable")
var collisionName string
attempts := 0
path, err := writeCrashReport(dir, "cli", "boom", []byte("complete stack"), ts, crashReportHooks{
link: func(*os.Root, string, string) error { return unsupported },
renameNoReplace: func(root *os.Root, oldname, newname string) error {
attempts++
if attempts == 1 {
collisionName = newname
if err := root.WriteFile(newname, []byte("concurrent report"), 0o600); err != nil {
t.Fatalf("create commit-time collision: %v", err)
}
}
return renameNoReplace(root, oldname, newname)
},
})
if err != nil {
t.Fatalf("writeCrashReport fallback: %v", err)
}
if attempts < 2 {
t.Fatalf("fallback rename attempts = %d, want collision retry", attempts)
}
if filepath.Base(path) == collisionName {
t.Fatalf("fallback replaced the concurrent report %q", collisionName)
}
existing, err := os.ReadFile(filepath.Join(dir, collisionName))
if err != nil {
t.Fatal(err)
}
if string(existing) != "concurrent report" {
t.Fatalf("concurrent report overwritten: %q", existing)
}
published, err := os.ReadFile(path)
if err != nil {
t.Fatal(err)
}
if !strings.Contains(string(published), "complete stack") {
t.Fatalf("published fallback is incomplete: %q", published)
}
}
type crashWriteResult struct {
path string
err error
}
func TestRecoverCapturesPanic(t *testing.T) {
dir := t.TempDir()
var stderr bytes.Buffer
@@ -460,25 +63,3 @@ func TestRecoverNoPanicIsNoop(t *testing.T) {
t.Fatalf("unexpected output without a panic: %q", stderr.String())
}
}
func TestReportRecoveredCrashReportsCommittedCleanupWarning(t *testing.T) {
var stderr bytes.Buffer
code := 0
injected := errors.New("injected cleanup failure")
reportRecoveredCrash(&stderr, &code, "kaboom", []byte("secret stack"), func() (string, error) {
return "/tmp/crash.log", &crashReportCommittedError{cause: injected}
})
if code != crashExitCode {
t.Fatalf("exit code = %d, want %d", code, crashExitCode)
}
output := stderr.String()
for _, want := range []string{"kaboom", "saved to /tmp/crash.log", "Warning:", injected.Error()} {
if !strings.Contains(output, want) {
t.Fatalf("crash notice missing %q: %q", want, output)
}
}
if strings.Contains(output, "secret stack") {
t.Fatalf("committed report warning fell back to inline stack: %q", output)
}
}
@@ -1,19 +0,0 @@
//go:build darwin
package observability
import (
"fmt"
"os"
"golang.org/x/sys/unix"
)
func renameNoReplace(root *os.Root, oldname, newname string) error {
directory, err := root.Open(".")
if err != nil {
return fmt.Errorf("open crash directory for publication: %w", err)
}
defer directory.Close()
return unix.RenameatxNp(int(directory.Fd()), oldname, int(directory.Fd()), newname, unix.RENAME_EXCL)
}
@@ -1,19 +0,0 @@
//go:build linux
package observability
import (
"fmt"
"os"
"golang.org/x/sys/unix"
)
func renameNoReplace(root *os.Root, oldname, newname string) error {
directory, err := root.Open(".")
if err != nil {
return fmt.Errorf("open crash directory for publication: %w", err)
}
defer directory.Close()
return unix.Renameat2(int(directory.Fd()), oldname, int(directory.Fd()), newname, unix.RENAME_NOREPLACE)
}
@@ -1,12 +0,0 @@
//go:build !darwin && !linux && !windows
package observability
import (
"errors"
"os"
)
func renameNoReplace(*os.Root, string, string) error {
return errors.ErrUnsupported
}
@@ -1,84 +0,0 @@
//go:build windows
package observability
import (
"errors"
"os"
"unsafe"
"golang.org/x/sys/windows"
)
type fileRenameInformation struct {
replaceIfExists uint32
rootDirectory windows.Handle
fileNameLength uint32
fileName [1]uint16
}
func renameNoReplace(root *os.Root, oldname, newname string) (returnErr error) {
directory, err := root.Open(".")
if err != nil {
return err
}
defer directory.Close()
raw, err := directory.SyscallConn()
if err != nil {
return err
}
if err := raw.Control(func(rawHandle uintptr) {
rootHandle := windows.Handle(rawHandle)
oldObjectName, nameErr := windows.NewNTUnicodeString(oldname)
if nameErr != nil {
returnErr = nameErr
return
}
attributes := &windows.OBJECT_ATTRIBUTES{
Length: uint32(unsafe.Sizeof(windows.OBJECT_ATTRIBUTES{})),
RootDirectory: rootHandle,
ObjectName: oldObjectName,
Attributes: windows.OBJ_CASE_INSENSITIVE | windows.OBJ_DONT_REPARSE,
}
var source windows.Handle
var status windows.IO_STATUS_BLOCK
returnErr = windows.NtCreateFile(
&source,
windows.DELETE|windows.SYNCHRONIZE,
attributes,
&status,
nil,
0,
windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE,
windows.FILE_OPEN,
windows.FILE_NON_DIRECTORY_FILE|windows.FILE_SYNCHRONOUS_IO_NONALERT|windows.FILE_OPEN_REPARSE_POINT,
0,
0,
)
if returnErr != nil {
return
}
defer windows.CloseHandle(source)
newName, nameErr := windows.UTF16FromString(newname)
if nameErr != nil {
returnErr = nameErr
return
}
nameBytes := (len(newName) - 1) * 2
var layout fileRenameInformation
bufferSize := int(unsafe.Offsetof(layout.fileName)) + nameBytes
buffer := make([]byte, bufferSize)
info := (*fileRenameInformation)(unsafe.Pointer(&buffer[0]))
info.rootDirectory = rootHandle
info.fileNameLength = uint32(nameBytes)
copy((*[windows.MAX_LONG_PATH]uint16)(unsafe.Pointer(&info.fileName[0]))[:nameBytes/2:nameBytes/2], newName)
returnErr = windows.NtSetInformationFile(source, &status, &buffer[0], uint32(bufferSize), windows.FileRenameInformation)
}); err != nil {
return err
}
if errors.Is(returnErr, windows.STATUS_OBJECT_NAME_COLLISION) || errors.Is(returnErr, windows.STATUS_OBJECT_NAME_EXISTS) {
return os.ErrExist
}
return returnErr
}
-85
View File
@@ -1,85 +0,0 @@
// Package privatedir creates and safely hardens current-user-owned state
// directories without applying permission changes through a path-only check.
package privatedir
import (
"errors"
"fmt"
"os"
"path/filepath"
)
// Ensure creates path when needed and enforces owner-only access. Existing
// directories are hardened only after ownership is verified through a bound
// directory handle; foreign-owned paths fail closed.
func Ensure(path string) error {
root, err := Open(path)
if err != nil {
return err
}
if err := root.Close(); err != nil {
return fmt.Errorf("close private directory: %w", err)
}
return nil
}
// Open creates path when needed, enforces owner-only access, and returns a
// traversal-resistant handle that callers can retain across subsequent I/O.
func Open(path string) (*os.Root, error) {
absolute, err := filepath.Abs(path)
if err != nil {
return nil, fmt.Errorf("resolve private directory: %w", err)
}
if err := os.MkdirAll(absolute, 0o700); err != nil {
return nil, fmt.Errorf("create private directory: %w", err)
}
before, err := os.Lstat(absolute)
if err != nil {
return nil, fmt.Errorf("inspect private directory entry: %w", err)
}
if before.Mode()&os.ModeSymlink != 0 {
return nil, fmt.Errorf("private directory entry is a symbolic link")
}
if !before.IsDir() {
return nil, fmt.Errorf("private directory path is not a directory")
}
root, err := os.OpenRoot(absolute)
if err != nil {
return nil, fmt.Errorf("open private directory: %w", err)
}
opened, err := root.Stat(".")
if err != nil {
_ = root.Close()
return nil, fmt.Errorf("inspect opened private directory: %w", err)
}
after, err := os.Lstat(absolute)
if err != nil {
_ = root.Close()
return nil, fmt.Errorf("reinspect private directory entry: %w", err)
}
if after.Mode()&os.ModeSymlink != 0 || !os.SameFile(opened, after) {
_ = root.Close()
return nil, fmt.Errorf("private directory entry changed while opening")
}
if err := harden(root); err != nil {
closeErr := root.Close()
if closeErr != nil {
return nil, errors.Join(err, fmt.Errorf("close private directory: %w", closeErr))
}
return nil, err
}
current, err := os.Lstat(absolute)
if err != nil || current.Mode()&os.ModeSymlink != 0 || !os.SameFile(opened, current) {
closeErr := root.Close()
if err == nil {
err = fmt.Errorf("private directory entry changed while hardening")
} else {
err = fmt.Errorf("verify private directory entry after hardening: %w", err)
}
if closeErr != nil {
err = errors.Join(err, fmt.Errorf("close private directory: %w", closeErr))
}
return nil, err
}
return root, nil
}
-35
View File
@@ -1,35 +0,0 @@
package privatedir
import (
"os"
"path/filepath"
"runtime"
"strings"
"testing"
)
func TestOpenRejectsFinalDirectorySymlinkWithoutHardeningTarget(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("creating directory symlinks requires privileges on some Windows runners")
}
parent := t.TempDir()
target := filepath.Join(parent, "target")
if err := os.Mkdir(target, 0o755); err != nil {
t.Fatal(err)
}
link := filepath.Join(parent, "private")
if err := os.Symlink(target, link); err != nil {
t.Fatal(err)
}
if _, err := Open(link); err == nil || !strings.Contains(err.Error(), "symbolic link") {
t.Fatalf("Open symlink error = %v, want symbolic-link rejection", err)
}
info, err := os.Stat(target)
if err != nil {
t.Fatal(err)
}
if got := info.Mode().Perm(); got != 0o755 {
t.Fatalf("symlink target permissions = %04o, want unchanged 0755", got)
}
}
-47
View File
@@ -1,47 +0,0 @@
//go:build !windows
package privatedir
import (
"errors"
"fmt"
"os"
"syscall"
)
func harden(root *os.Root) (returnErr error) {
directory, err := root.Open(".")
if err != nil {
return fmt.Errorf("open private directory hardening handle: %w", err)
}
defer func() {
if err := directory.Close(); err != nil {
returnErr = errors.Join(returnErr, fmt.Errorf("close private directory hardening handle: %w", err))
}
}()
info, err := directory.Stat()
if err != nil {
return fmt.Errorf("inspect private directory before hardening: %w", err)
}
if !info.IsDir() {
return fmt.Errorf("private directory path is not a directory")
}
stat, ok := info.Sys().(*syscall.Stat_t)
if !ok {
return fmt.Errorf("private directory ownership metadata is unavailable")
}
if int(stat.Uid) != os.Geteuid() {
return fmt.Errorf("private directory is owned by uid %d, not the current user", stat.Uid)
}
if err := directory.Chmod(0o700); err != nil {
return fmt.Errorf("harden private directory permissions: %w", err)
}
info, err = directory.Stat()
if err != nil {
return fmt.Errorf("verify private directory permissions: %w", err)
}
if info.Mode().Perm()&0o077 != 0 {
return fmt.Errorf("private directory permissions are %04o, want owner-only", info.Mode().Perm())
}
return nil
}
-135
View File
@@ -1,135 +0,0 @@
//go:build windows
package privatedir
import (
"errors"
"fmt"
"os"
"unsafe"
"golang.org/x/sys/windows"
)
func harden(root *os.Root) (returnErr error) {
directory, err := root.Open(".")
if err != nil {
return fmt.Errorf("open private directory hardening handle: %w", err)
}
defer func() {
if err := directory.Close(); err != nil {
returnErr = errors.Join(returnErr, fmt.Errorf("close private directory hardening handle: %w", err))
}
}()
raw, err := directory.SyscallConn()
if err != nil {
return fmt.Errorf("access private directory hardening handle: %w", err)
}
// Root.Open uses NtCreateFile, so ReOpenFile cannot accept its handle. An
// empty relative NT object name reopens the bound directory itself with the
// security rights needed for the DACL update and no pathname re-resolution.
var securityHandle windows.Handle
var openErr error
if err := raw.Control(func(rawHandle uintptr) {
objectName, nameErr := windows.NewNTUnicodeString("")
if nameErr != nil {
openErr = nameErr
return
}
openErr = windows.NtCreateFile(
&securityHandle,
windows.READ_CONTROL|windows.WRITE_DAC|windows.FILE_READ_ATTRIBUTES|windows.SYNCHRONIZE,
&windows.OBJECT_ATTRIBUTES{
Length: uint32(unsafe.Sizeof(windows.OBJECT_ATTRIBUTES{})),
RootDirectory: windows.Handle(rawHandle),
ObjectName: objectName,
Attributes: windows.OBJ_CASE_INSENSITIVE | windows.OBJ_DONT_REPARSE,
},
&windows.IO_STATUS_BLOCK{},
nil,
windows.FILE_ATTRIBUTE_DIRECTORY,
windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE,
windows.FILE_OPEN,
windows.FILE_DIRECTORY_FILE|windows.FILE_SYNCHRONOUS_IO_NONALERT|windows.FILE_OPEN_REPARSE_POINT|windows.FILE_OPEN_FOR_BACKUP_INTENT,
0,
0,
)
}); err != nil {
return fmt.Errorf("open private directory security handle: %w", err)
}
if openErr != nil {
return fmt.Errorf("open private directory with security access: %w", openErr)
}
if securityHandle == 0 || securityHandle == windows.InvalidHandle {
return fmt.Errorf("open private directory with security access: invalid handle")
}
defer func() {
if err := windows.CloseHandle(securityHandle); err != nil {
returnErr = errors.Join(returnErr, fmt.Errorf("close private directory security handle: %w", err))
}
}()
token := windows.GetCurrentProcessToken()
user, err := token.GetTokenUser()
if err != nil {
return fmt.Errorf("resolve current Windows user: %w", err)
}
tokenOwner, err := windowsTokenOwner(token)
if err != nil {
return fmt.Errorf("resolve current Windows token owner: %w", err)
}
current, err := windows.GetSecurityInfo(securityHandle, windows.SE_FILE_OBJECT, windows.OWNER_SECURITY_INFORMATION)
if err != nil {
return fmt.Errorf("read private directory owner before hardening: %w", err)
}
owner, _, err := current.Owner()
if err != nil {
return fmt.Errorf("read private directory owner before hardening: %w", err)
}
if owner == nil || (!owner.Equals(user.User.Sid) && !owner.Equals(tokenOwner)) {
return fmt.Errorf("private directory is not owned by the current Windows token")
}
desired, err := windows.SecurityDescriptorFromString(
fmt.Sprintf("O:%sD:P(A;OICI;GA;;;%s)(A;OICI;GA;;;SY)", user.User.Sid.String(), user.User.Sid.String()),
)
if err != nil {
return fmt.Errorf("build private Windows directory DACL: %w", err)
}
dacl, _, err := desired.DACL()
if err != nil {
return fmt.Errorf("read private Windows directory DACL: %w", err)
}
if err := windows.SetSecurityInfo(
securityHandle,
windows.SE_FILE_OBJECT,
windows.DACL_SECURITY_INFORMATION|windows.PROTECTED_DACL_SECURITY_INFORMATION,
nil,
nil,
dacl,
nil,
); err != nil {
return fmt.Errorf("harden private Windows directory DACL: %w", err)
}
return nil
}
type tokenOwnerInfo struct {
owner *windows.SID
}
func windowsTokenOwner(token windows.Token) (*windows.SID, error) {
var size uint32
err := windows.GetTokenInformation(token, windows.TokenOwner, nil, 0, &size)
if err != windows.ERROR_INSUFFICIENT_BUFFER {
return nil, err
}
buffer := make([]byte, size)
if err := windows.GetTokenInformation(token, windows.TokenOwner, &buffer[0], size, &size); err != nil {
return nil, err
}
owner := (*tokenOwnerInfo)(unsafe.Pointer(&buffer[0])).owner
if owner == nil {
return nil, errors.New("windows access token has no default owner")
}
return owner.Copy()
}
+3 -12
View File
@@ -226,20 +226,11 @@ func CompactionPayloadFromPlan(summary string, plan CompactionPlan) (CompactionP
}
func (store *Store) ReadRehydratedEvents(sessionID string) ([]Event, error) {
events, _, err := store.ReadRehydratedEventsWithPresence(sessionID)
return events, err
}
// ReadRehydratedEventsWithPresence carries the underlying event-log presence
// through compaction projection without changing ReadRehydratedEvents' existing
// empty-on-missing contract.
func (store *Store) ReadRehydratedEventsWithPresence(sessionID string) ([]Event, bool, error) {
events, present, err := store.ReadEventsWithPresence(sessionID)
events, err := store.ReadEvents(sessionID)
if err != nil {
return nil, present, err
return nil, err
}
rehydrated, err := RehydrateEvents(events)
return rehydrated, present, err
return RehydrateEvents(events)
}
func (store *Store) ReadReplayEvents(sessionID string) ([]Event, error) {
+5 -14
View File
@@ -813,24 +813,15 @@ func (store *Store) UpdateModel(sessionID string, modelID string) (Metadata, err
}
func (store *Store) ReadEvents(sessionID string) ([]Event, error) {
events, _, err := store.ReadEventsWithPresence(sessionID)
return events, err
}
// ReadEventsWithPresence preserves the distinction between an intentionally
// empty event log and a missing one. Most readers accept both as empty history,
// but activation protocols that promise to restore previously populated
// context need the stronger signal.
func (store *Store) ReadEventsWithPresence(sessionID string) ([]Event, bool, error) {
if !ValidSessionID(sessionID) {
return nil, false, fmt.Errorf("invalid zero session id %q", sessionID)
return nil, fmt.Errorf("invalid zero session id %q", sessionID)
}
data, err := os.ReadFile(store.eventsPath(sessionID))
if err != nil {
if errors.Is(err, os.ErrNotExist) {
return []Event{}, false, nil
return []Event{}, nil
}
return nil, false, fmt.Errorf("read zero session events: %w", err)
return nil, fmt.Errorf("read zero session events: %w", err)
}
// A genuine torn tail is an INCOMPLETE final write — a crash mid-append leaves
// the last line without its terminating newline. If the file ends with a
@@ -860,11 +851,11 @@ func (store *Store) ReadEventsWithPresence(sessionID string) ([]Event, bool, err
if index == lastNonEmpty && tornTailPossible {
break
}
return nil, true, fmt.Errorf("invalid json in zero session %s %s at line %d: %w", sessionID, EventsFile, index+1, err)
return nil, fmt.Errorf("invalid json in zero session %s %s at line %d: %w", sessionID, EventsFile, index+1, err)
}
events = append(events, event)
}
return events, true, nil
return events, nil
}
func (store *Store) timestamp() string {
+160 -4
View File
@@ -2,8 +2,10 @@ package tools
import (
"context"
"errors"
"os"
"path/filepath"
"slices"
"strconv"
"strings"
"testing"
@@ -363,7 +365,7 @@ func TestUnifiedPatchCopyOperation(t *testing.T) {
if content, _ := os.ReadFile(filepath.Join(root, "dst.txt")); string(content) != "hello\nnew\n" {
t.Fatalf("copy destination = %q", string(content))
}
if len(result.ChangedFiles) != 2 || result.ChangedFiles[0] != "src.txt" || result.ChangedFiles[1] != "dst.txt" {
if len(result.ChangedFiles) != 1 || result.ChangedFiles[0] != "dst.txt" {
t.Fatalf("changed files = %v", result.ChangedFiles)
}
destination, err := filepath.EvalSymlinks(filepath.Join(root, "dst.txt"))
@@ -571,9 +573,9 @@ func TestApplyStructuredPatchChangeRefusesSourceChangedAfterPlanning(t *testing.
"copy": {kind: structuredPatchCopy, from: target, to: destination, before: "planned\n", after: "planned\n", mode: 0o644},
} {
writeTestFile(t, path, "changed after planning\n")
committed, err := applyStructuredPatchChange(workspace, change)
if err == nil || committed || !strings.Contains(err.Error(), "changed on disk between planning and commit") {
t.Fatalf("%s: expected a refusal, got committed=%v err=%v", name, committed, err)
outcome, err := applyStructuredPatchChange(workspace, change)
if err == nil || len(outcome.completed) != 0 || len(outcome.incompletePaths) != 0 || !strings.Contains(err.Error(), "changed on disk between planning and commit") {
t.Fatalf("%s: expected a refusal, got outcome=%#v err=%v", name, outcome, err)
}
if content, _ := os.ReadFile(path); string(content) != "changed after planning\n" {
t.Fatalf("%s: file must be untouched, got %q", name, string(content))
@@ -709,4 +711,158 @@ func TestApplyPatchOperationsReportsCommittedPrefixOnFailure(t *testing.T) {
if content, _ := os.ReadFile(filepath.Join(root, "third.txt")); string(content) != "three\n" {
t.Fatalf("third.txt must be untouched, got %q", string(content))
}
if got := result.ChangedFiles; len(got) != 1 || got[0] != "first.txt" {
t.Fatalf("partial patch ChangedFiles = %#v, want committed prefix only", got)
}
resolvedFirst, err := filepath.EvalSymlinks(filepath.Join(root, "first.txt"))
if err != nil {
t.Fatal(err)
}
if got := result.FileDiffs; len(got) != 1 || got[0] != (FileDiff{Path: resolvedFirst, OldExists: true, NewExists: true, OldText: "one\n", NewText: "ONE\n"}) {
t.Fatalf("partial patch FileDiffs = %#v", got)
}
}
func TestApplyPatchOperationsReportsWorkspaceRelativeCommittedPrefixUnderCwd(t *testing.T) {
root := t.TempDir()
applyRoot := filepath.Join(root, "sub", "dir")
if err := os.MkdirAll(applyRoot, 0o755); err != nil {
t.Fatal(err)
}
writeTestFile(t, filepath.Join(applyRoot, "first.txt"), "one\n")
writeTestFile(t, filepath.Join(applyRoot, "second.txt"), "two\n")
structuredPatchBeforeCommit = func(change structuredPatchChange) {
if change.to.relative == "second.txt" {
writeTestFile(t, filepath.Join(applyRoot, "second.txt"), "changed\n")
}
}
t.Cleanup(func() { structuredPatchBeforeCommit = nil })
patch := strings.Join([]string{
"*** Begin Patch",
"*** Update File: first.txt", "@@", "-one", "+ONE",
"*** Update File: second.txt", "@@", "-two", "+TWO",
"*** End Patch", "",
}, "\n")
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{
"cwd": "sub/dir", "patch": patch,
})
if result.Status != StatusError || !strings.Contains(result.Output, "already committed: sub/dir/first.txt") {
t.Fatalf("nested partial failure = status=%s output=%q", result.Status, result.Output)
}
if got := result.ChangedFiles; len(got) != 1 || got[0] != "sub/dir/first.txt" {
t.Fatalf("nested partial ChangedFiles = %#v", got)
}
}
func TestApplyPatchSeparatesVerifiedAndUnverifiedPartialPublications(t *testing.T) {
root := t.TempDir()
priorPublish := structuredPatchPublishNoReplace
structuredPatchPublishNoReplace = func(workspace *os.Root, source, target string, mode os.FileMode) (bool, error) {
if target != "uncertain.txt" {
return priorPublish(workspace, source, target, mode)
}
file, err := workspace.OpenFile(target, os.O_WRONLY|os.O_CREATE|os.O_EXCL, mode.Perm())
if err != nil {
return false, err
}
if _, err := file.WriteString("partial"); err != nil {
_ = file.Close()
return true, err
}
if err := file.Close(); err != nil {
return true, err
}
return true, errors.New("injected failure after partial publication")
}
t.Cleanup(func() { structuredPatchPublishNoReplace = priorPublish })
patch := strings.Join([]string{
"*** Begin Patch",
"*** Add File: verified.txt", "+verified",
"*** Add File: uncertain.txt", "+intended complete content",
"*** Add File: untouched.txt", "+untouched",
"*** End Patch", "",
}, "\n")
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch})
if result.Status != StatusError {
t.Fatalf("partial publication status = %s, want error", result.Status)
}
for _, want := range []string{"already committed: verified.txt", "published but unverified: uncertain.txt"} {
if !strings.Contains(result.Output, want) {
t.Fatalf("partial publication error must contain %q: %s", want, result.Output)
}
}
committedFragment := strings.SplitN(result.Output, "; published but unverified:", 2)[0]
if strings.Contains(committedFragment, "uncertain.txt") {
t.Fatalf("unverified path appears in committed fragment: %s", result.Output)
}
if got, want := result.ChangedFiles, []string{"verified.txt"}; !slices.Equal(got, want) {
t.Fatalf("ChangedFiles = %#v, want verified publications only %#v", got, want)
}
resolvedVerified, err := filepath.EvalSymlinks(filepath.Join(root, "verified.txt"))
if err != nil {
t.Fatal(err)
}
if got := result.FileDiffs; len(got) != 1 || got[0].Path != resolvedVerified {
t.Fatalf("FileDiffs = %#v, want verified.txt only", got)
}
if got := mustReadTestFile(t, filepath.Join(root, "verified.txt")); got != "verified\n" {
t.Fatalf("verified publication = %q", got)
}
if got := mustReadTestFile(t, filepath.Join(root, "uncertain.txt")); got != "partial" {
t.Fatalf("unverified publication = %q", got)
}
if _, err := os.Stat(filepath.Join(root, "untouched.txt")); !os.IsNotExist(err) {
t.Fatalf("later publication must remain untouched: %v", err)
}
}
func TestApplyPatchMoveReportsPublishedDestinationWhenSourceRemovalFails(t *testing.T) {
root := t.TempDir()
writeTestFile(t, filepath.Join(root, "source.txt"), "before\n")
priorRemove := structuredPatchRemove
structuredPatchRemove = func(workspace *os.Root, name string) error {
if name == "source.txt" {
return os.ErrPermission
}
return priorRemove(workspace, name)
}
t.Cleanup(func() { structuredPatchRemove = priorRemove })
patch := strings.Join([]string{
"*** Begin Patch",
"*** Update File: source.txt",
"*** Move to: destination.txt",
"@@",
"-before",
"+after",
"*** End Patch",
"",
}, "\n")
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch})
if result.Status != StatusError {
t.Fatalf("move status = %s, want error", result.Status)
}
if got := mustReadTestFile(t, filepath.Join(root, "source.txt")); got != "before\n" {
t.Fatalf("source content = %q", got)
}
if got := mustReadTestFile(t, filepath.Join(root, "destination.txt")); got != "after\n" {
t.Fatalf("destination content = %q", got)
}
if got, want := result.ChangedFiles, []string{"destination.txt"}; !slices.Equal(got, want) {
t.Fatalf("ChangedFiles = %#v, want %#v", got, want)
}
resolvedDestination, err := filepath.EvalSymlinks(filepath.Join(root, "destination.txt"))
if err != nil {
t.Fatal(err)
}
wantDiff := FileDiff{Path: resolvedDestination, OldExists: false, NewExists: true, NewText: "after\n"}
if got := result.FileDiffs; len(got) != 1 || got[0] != wantDiff {
t.Fatalf("FileDiffs = %#v, want %#v", got, []FileDiff{wantDiff})
}
if !strings.Contains(result.Display.Preview, "destination.txt") || strings.Contains(result.Display.Preview, "source.txt") {
t.Fatalf("preview must show only destination creation: %q", result.Display.Preview)
}
}
+106 -3
View File
@@ -1,19 +1,122 @@
package tools
import udiff "github.com/aymanbagabas/go-udiff"
import (
"path/filepath"
"strings"
"unicode"
"unicode/utf8"
"github.com/Gitlawb/zero/internal/redaction"
udiff "github.com/aymanbagabas/go-udiff"
)
// maxToolPreviewBytes caps the inline diff a write tool appends to its result, so
// a large generated file can't flood the transcript or balloon the persisted
// session events. Past this the tool falls back to its summary line alone.
const maxToolPreviewBytes = 48 * 1024
// A structured replacement contains two complete file sides, so its transport
// budget is intentionally separate from the single rendered-preview budget.
// Each side may be as large as a normal preview; aggregate producers apply the
// two-sided result cap in file order.
const maxToolResultFileDiffBytes = 2 * maxToolPreviewBytes
// FileDiff is a human-facing before/after file change. Registry-boundary
// redaction applies to both sides before any caller receives it.
type FileDiff struct {
// Path is the canonical absolute path required by ACP diff content. The
// separate ChangedFiles result remains workspace-relative for local UI use.
Path string
// OldExists and NewExists distinguish an empty file from a missing side of a
// create/delete/move. Empty strings alone cannot encode that difference.
OldExists bool
NewExists bool
OldText string
NewText string
}
// boundedFileDiff declines rather than truncating: a truncated side would look
// like an exact file replacement. Callers keep ChangedFiles as the safe
// fallback for large, unsafe, or unchanged content. Each complete side gets the
// same bound as a rendered preview; aggregate result producers apply their own
// cap. Control bytes are rejected. Ordinary Unicode format/space characters are
// retained unless removing them reveals a credential shape that the normal
// redactor could not see in the original text.
func boundedFileDiff(path, oldText, newText string, oldExists, newExists bool) (FileDiff, bool) {
if !filepath.IsAbs(path) || (!oldExists && !newExists) ||
(oldExists == newExists && oldText == newText) ||
!utf8.ValidString(oldText) || !utf8.ValidString(newText) ||
unsafeDiffText(oldText) || unsafeDiffText(newText) ||
len(oldText) > maxToolPreviewBytes || len(newText) > maxToolPreviewBytes {
return FileDiff{}, false
}
return FileDiff{Path: path, OldExists: oldExists, NewExists: newExists, OldText: oldText, NewText: newText}, true
}
func unsafeDiffText(text string) bool {
if !utf8.ValidString(text) {
return true
}
hasCanonicalizableSeparator := false
for _, r := range text {
switch r {
case '\n', '\r', '\t', ' ':
continue
}
if unicode.IsControl(r) {
return true
}
if isDiffCredentialIgnorable(r) {
hasCanonicalizableSeparator = true
}
}
if !hasCanonicalizableSeparator {
return false
}
return diffTextRevealsObfuscatedSecret(text)
}
func diffTextRevealsObfuscatedSecret(text string) bool {
if !utf8.ValidString(text) {
return false
}
canonical := strings.Map(func(r rune) rune {
switch r {
case '\n', '\r', '\t', ' ':
return r
}
if isDiffCredentialIgnorable(r) {
return -1
}
return r
}, text)
return canonical != text && redaction.RedactString(canonical, redaction.Options{}) != canonical
}
// isDiffCredentialIgnorable mirrors Unicode's Default_Ignorable_Code_Point
// derived property using the tables exposed by the Go standard library. Cf
// catches format controls, while the other two tables cover non-Cf default
// ignorables such as U+034F COMBINING GRAPHEME JOINER and variation selectors.
// Unicode whitespace is also removable because it can split a credential that
// the boundary redactor would otherwise recognize. The caller preserves normal
// ASCII layout whitespace before consulting this helper.
func isDiffCredentialIgnorable(r rune) bool {
return unicode.Is(unicode.Cf, r) ||
unicode.Is(unicode.Other_Default_Ignorable_Code_Point, r) ||
unicode.Is(unicode.Variation_Selector, r) ||
unicode.IsSpace(r)
}
// boundedUnifiedDiff returns a unified diff of oldContent -> newContent labelled
// with path, suitable for the TUI's diff card renderer. A create (oldContent "")
// yields an all-additions (green) preview; an overwrite/edit yields red/green.
// Returns "" when there is no change or the diff exceeds maxToolPreviewBytes.
// Returns "" when there is no change, the rendered diff is unsafe text, or the
// diff exceeds maxToolPreviewBytes. This applies the same unsafe-text gate as
// FileDiff to what reaches Display.Preview, without discarding a safe hunk only
// because an unrelated part of the source file contains an unsafe byte.
func boundedUnifiedDiff(path, oldContent, newContent string) string {
diff := udiff.Unified(path, path, oldContent, newContent)
if diff == "" || len(diff) > maxToolPreviewBytes {
if diff == "" || !utf8.ValidString(diff) || unsafeDiffText(diff) || len(diff) > maxToolPreviewBytes {
return ""
}
return diff
+186
View File
@@ -0,0 +1,186 @@
package tools
import (
"path/filepath"
"strings"
"testing"
)
func TestBoundedFileDiffRefusesPartialOrBinaryContent(t *testing.T) {
path := filepath.Join(t.TempDir(), "a.txt")
if diff, ok := boundedFileDiff(path, "old", "new", true, true); !ok || diff.Path != path || !diff.OldExists || !diff.NewExists || diff.OldText != "old" || diff.NewText != "new" {
t.Fatalf("small text diff = %#v, %t", diff, ok)
}
for _, tc := range []struct {
name string
old string
new string
}{
{"unchanged", "same", "same"},
{"binary old", string([]byte{0xff}), "text"},
{"binary new", "text", string([]byte{0xff})},
{"nul old", "token=sk-proj-abc\x00def", "text"},
{"escape new", "text", "token=sk-proj-abc\x1bdef"},
{"c1 old", "token=sk-proj-abc\u0085def", "text"},
{"zero width space", "token=sk-proj-abc\u200bdef", "text"},
{"zero width joiner", "token=sk-proj-abc\u200ddef", "text"},
{"byte order mark", "token=sk-proj-abc\ufeffdef", "text"},
{"soft hyphen", "token=sk-proj-abc\u00addef", "text"},
{"non breaking space", "token=sk-proj-abc\u00a0def", "text"},
{"too large", strings.Repeat("a", maxToolPreviewBytes+1), "b"},
} {
t.Run(tc.name, func(t *testing.T) {
if diff, ok := boundedFileDiff(path, tc.old, tc.new, true, true); ok || diff != (FileDiff{}) {
t.Fatalf("unexpected diff = %#v, %t", diff, ok)
}
})
}
}
func TestBoundedFileDiffPreservesEmptyFileOperations(t *testing.T) {
path := filepath.Join(t.TempDir(), "empty.txt")
for _, tc := range []struct {
name string
oldExists, newExists bool
}{
{name: "create", newExists: true},
{name: "delete", oldExists: true},
} {
t.Run(tc.name, func(t *testing.T) {
diff, ok := boundedFileDiff(path, "", "", tc.oldExists, tc.newExists)
if !ok || diff.OldExists != tc.oldExists || diff.NewExists != tc.newExists {
t.Fatalf("empty %s = %#v, %t", tc.name, diff, ok)
}
})
}
}
func TestBoundedUnifiedDiffRejectsUnsafeRichText(t *testing.T) {
if got := boundedUnifiedDiff("safe.txt", "before\n", "after\n"); got == "" {
t.Fatal("safe unified diff was unexpectedly omitted")
}
for _, content := range []string{
"token=sk-proj-abc\x00def",
"token=sk-proj-abc\x1bdef",
"token=sk-proj-abc\u200bdef",
string([]byte{0xff}),
} {
if got := boundedUnifiedDiff("secret.txt", content, "safe\n"); got != "" {
t.Fatalf("unsafe rich diff = %q", got)
}
}
old := strings.Repeat("unchanged\n", 12) + "form\ffeed\n" + strings.Repeat("unchanged\n", 12)
updated := strings.Replace(old, "unchanged\n", "changed\n", 1)
if got := boundedUnifiedDiff("safe-hunk.txt", old, updated); got == "" || strings.Contains(got, "\f") {
t.Fatalf("safe hunk near unrelated unsafe text = %q", got)
}
}
func TestStructuredPatchFileDiffsPreserveEmptyOperationsAndResultBudget(t *testing.T) {
root, err := filepath.EvalSymlinks(t.TempDir())
if err != nil {
t.Fatal(err)
}
changes := []structuredPatchChange{
{kind: structuredPatchAdd, to: structuredPatchTarget{absolute: filepath.Join(root, "created"), relative: "created"}},
{kind: structuredPatchDelete, from: structuredPatchTarget{absolute: filepath.Join(root, "deleted"), relative: "deleted"}},
{kind: structuredPatchUpdate, from: structuredPatchTarget{absolute: filepath.Join(root, "from"), relative: "from"}, to: structuredPatchTarget{absolute: filepath.Join(root, "to"), relative: "to"}},
}
diffs := fileDiffsFromStructuredPatch(".", changes)
if len(diffs) != 4 {
t.Fatalf("empty create/delete/move diffs = %#v", diffs)
}
for _, diff := range diffs {
if diff.Path == "" || (!diff.OldExists && !diff.NewExists) {
t.Fatalf("invalid diff = %#v", diff)
}
}
large := strings.Repeat("x", maxToolResultFileDiffBytes/3+1)
budgeted := fileDiffsFromStructuredPatch(".", []structuredPatchChange{
{kind: structuredPatchAdd, to: structuredPatchTarget{absolute: filepath.Join(root, "one")}, after: large},
{kind: structuredPatchAdd, to: structuredPatchTarget{absolute: filepath.Join(root, "two")}, after: "tiny"},
{kind: structuredPatchAdd, to: structuredPatchTarget{absolute: filepath.Join(root, "three")}, after: large},
{kind: structuredPatchAdd, to: structuredPatchTarget{absolute: filepath.Join(root, "four")}, after: large},
})
if len(budgeted) != 3 || filepath.Base(budgeted[0].Path) != "one" || filepath.Base(budgeted[1].Path) != "two" || filepath.Base(budgeted[2].Path) != "three" {
t.Fatalf("ordered aggregate file-diff budget = %#v", budgeted)
}
}
func TestStructuredPatchFileDiffsKeepEligibleSameBasenameSibling(t *testing.T) {
root, err := filepath.EvalSymlinks(t.TempDir())
if err != nil {
t.Fatal(err)
}
diffs := fileDiffsFromStructuredPatch(".", []structuredPatchChange{
{
kind: structuredPatchAdd,
to: structuredPatchTarget{absolute: filepath.Join(root, "a.go"), relative: "a.go"},
after: strings.Repeat("x", maxToolPreviewBytes+1),
},
{
kind: structuredPatchAdd,
to: structuredPatchTarget{absolute: filepath.Join(root, "sub", "a.go"), relative: filepath.Join("sub", "a.go")},
after: "package sub\n",
},
})
if len(diffs) != 1 || diffs[0].Path != filepath.Join(root, "sub", "a.go") {
t.Fatalf("same-basename rich diffs = %#v", diffs)
}
}
func TestBoundedDiffPreservesOrdinaryUnicodeButRejectsObfuscatedSecrets(t *testing.T) {
path := filepath.Join(t.TempDir(), "unicode.txt")
for name, content := range map[string]string{
"family emoji": "family: 👨‍👩‍👧‍👦\n",
"emoji variation selector": "heart: ❤️\n",
"combining grapheme joiner": "ordinary\u034fprose\n",
"nonbreaking space": "ordinary\u00a0prose\n",
"byte order mark": "\ufeffdocument\n",
"soft hyphen": "co\u00adoperate\n",
} {
t.Run(name, func(t *testing.T) {
if _, ok := boundedFileDiff(path, "before\n", content, true, true); !ok {
t.Fatalf("ordinary Unicode content was rejected: %q", content)
}
if preview := boundedUnifiedDiff("unicode.txt", "before\n", content); preview == "" {
t.Fatal("ordinary Unicode preview was omitted")
}
})
}
secret := "sk-ant-api03-AAAABBBBCCCCDDDDEEEEFFFFGGGG"
for name, separator := range map[string]string{
"zero width space": "\u200b",
"zero width joiner": "\u200d",
"byte order mark": "\ufeff",
"soft hyphen": "\u00ad",
"nonbreaking space": "\u00a0",
"combining grapheme joiner": "\u034f",
"variation selector": "\ufe0f",
} {
t.Run("split "+name, func(t *testing.T) {
obfuscated := secret[:20] + separator + secret[20:]
if _, ok := boundedFileDiff(path, "before\n", obfuscated, true, true); ok {
t.Fatalf("obfuscated credential produced a rich diff: %q", obfuscated)
}
if preview := boundedUnifiedDiff("secret.txt", "before\n", obfuscated); preview != "" {
t.Fatalf("obfuscated credential produced preview: %q", preview)
}
})
}
}
func TestWriteFileMarksSuppressedObfuscatedSecretAsRedacted(t *testing.T) {
root := t.TempDir()
secret := "sk-ant-api03-AAAABBBBCCCCDDDDEEEEFFFFGGGG"
obfuscated := secret[:20] + "\u200b" + secret[20:]
result := NewScopedWriteFileTool(root, nil).Run(t.Context(), map[string]any{
"path": "secret.txt", "content": obfuscated,
})
if result.Status != StatusOK || len(result.ChangedFiles) != 1 || len(result.FileDiffs) != 0 || !result.Redacted {
t.Fatalf("obfuscated-secret result = status=%s changed=%#v diffs=%#v redacted=%t", result.Status, result.ChangedFiles, result.FileDiffs, result.Redacted)
}
}
+29 -6
View File
@@ -80,6 +80,10 @@ func (tool editFileTool) RunWithOptions(ctx context.Context, args map[string]any
}
}
content := string(contentBytes)
priorInfo, err := os.Stat(absolutePath)
if err != nil {
return errorResult("Error reading " + relativePath + ": " + err.Error())
}
occurrences := strings.Count(content, oldString)
// CRLF fallback: read_file normalizes \r\n → \n before presenting content to
@@ -153,19 +157,25 @@ func (tool editFileTool) RunWithOptions(ctx context.Context, args map[string]any
if err := recheckScopedWriteTarget(tool.workspaceRoot, tool.scope, requestedPath); err != nil {
return errorResult("Error writing " + relativePath + ": " + err.Error())
}
if err := os.WriteFile(absolutePath, []byte(updated), 0o644); err != nil {
if err := commitFileContents(absolutePath, priorInfo, &content, updated); err != nil {
return errorResult("Error writing " + relativePath + ": " + err.Error())
}
modelKnownContent := updated
// Optional format-on-write (ZERO_FORMAT_ON_WRITE). Must run BEFORE the
// FileTracker re-baseline: recording pre-format content would make the very
// next edit look like an external modification and trip the conflict guard.
formatting := maybeFormatWrittenFile(ctx, absolutePath, updated)
formatting := maybeFormatWrittenFileScoped(ctx, tool.workspaceRoot, tool.scope, absolutePath, updated, priorInfo.Mode().Perm())
updated = formatting.Content
finalContentKnown := formatting.ContentKnown
// Re-baseline to the content we just wrote so subsequent edits in this session
// compare against the current on-disk state, not the pre-edit version.
newInfo, _ := os.Stat(absolutePath)
if updated == modelKnownContent {
newInfo := formatting.Info
if newInfo == nil {
newInfo, _ = os.Stat(absolutePath)
}
if !finalContentKnown {
options.FileTracker.Forget(absolutePath)
} else if updated == modelKnownContent {
// OUR edit, so we know precisely which lines moved: RecordEdit carries
// across the reads this edit did not disturb instead of dropping them.
//
@@ -195,12 +205,25 @@ func (tool editFileTool) RunWithOptions(ctx context.Context, args map[string]any
}
summary := fmt.Sprintf("Successfully edited %s (replaced %d occurrence%s).", relativePath, replacedCount, suffix)
summary += formatting.notice(relativePath)
summary += inlineDiagnostics(ctx, options, absolutePath, relativePath)
if finalContentKnown {
summary += inlineDiagnostics(ctx, options, absolutePath, relativePath)
}
result := okResult(summary)
result.ChangedFiles = []string{relativePath}
if finalContentKnown {
if diff, ok := boundedFileDiff(absolutePath, content, updated, true, true); ok {
result.FileDiffs = []FileDiff{diff}
} else if diffTextRevealsObfuscatedSecret(content) || diffTextRevealsObfuscatedSecret(updated) {
result.Redacted = true
}
}
// Card-only preview (Display.Preview): the model's Output stays the one-line
// summary, so the red/green diff costs zero model tokens.
result.Display = Display{Summary: fmt.Sprintf("Edited %s", relativePath), Kind: "diff", Preview: boundedUnifiedDiff(relativePath, content, updated)}
preview := ""
if finalContentKnown {
preview = boundedUnifiedDiff(relativePath, content, updated)
}
result.Display = Display{Summary: fmt.Sprintf("Edited %s", relativePath), Kind: "diff", Preview: preview}
return result
}
+108
View File
@@ -0,0 +1,108 @@
package tools
import (
"errors"
"fmt"
"io"
"os"
)
var errFileChangedDuringWrite = errors.New("file changed on disk before the write committed")
// fileWriteBeforeCommit is a deterministic test hook. Production leaves it
// nil; tests use it to replace a path after observation but before opening the
// object that will actually be mutated.
var fileWriteBeforeCommit func(path string)
// fileWriteStat is a deterministic test seam for proving that the opened-file
// identity is captured before the final preimage comparison. Production uses
// the file descriptor directly.
var fileWriteStat = func(file *os.File) (os.FileInfo, error) { return file.Stat() }
// commitFileContents binds an overwrite to the file identity and bytes that
// the caller observed. A create uses exclusive creation. An overwrite opens the
// observed object without truncation, verifies identity/content through that
// handle, then truncates and writes the same handle. A path replacement before
// or during commit therefore fails instead of publishing stale rich evidence.
//
// expectedInfo nil means the caller observed a missing path. expectedContent
// may be nil for an existing but unreadable file; that path may still be
// overwritten, but callers must omit rich before/after evidence.
func commitFileContents(path string, expectedInfo os.FileInfo, expectedContent *string, content string) error {
if fileWriteBeforeCommit != nil {
fileWriteBeforeCommit(path)
}
if expectedInfo == nil {
file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644)
if err != nil {
return err
}
openedInfo, err := fileWriteStat(file)
if err != nil {
_ = file.Close()
return err
}
return writeAndVerifyFileIdentity(path, file, openedInfo, content, false)
}
flags := os.O_WRONLY
if expectedContent != nil {
flags = os.O_RDWR
}
file, err := os.OpenFile(path, flags, 0)
if err != nil {
return err
}
openedInfo, err := fileWriteStat(file)
if err != nil {
_ = file.Close()
return err
}
if !os.SameFile(expectedInfo, openedInfo) {
_ = file.Close()
return errFileChangedDuringWrite
}
pathInfo, err := os.Stat(path)
if err != nil || !os.SameFile(openedInfo, pathInfo) {
_ = file.Close()
return errFileChangedDuringWrite
}
if expectedContent != nil {
current, readErr := io.ReadAll(file)
if readErr != nil {
_ = file.Close()
return readErr
}
if string(current) != *expectedContent {
_ = file.Close()
return errFileChangedDuringWrite
}
}
return writeAndVerifyFileIdentity(path, file, openedInfo, content, true)
}
func writeAndVerifyFileIdentity(path string, file *os.File, openedInfo os.FileInfo, content string, truncate bool) error {
if truncate {
if err := file.Truncate(0); err != nil {
_ = file.Close()
return err
}
if _, err := file.Seek(0, io.SeekStart); err != nil {
_ = file.Close()
return err
}
}
if _, err := io.WriteString(file, content); err != nil {
_ = file.Close()
return err
}
if err := file.Close(); err != nil {
return err
}
pathInfo, err := os.Stat(path)
if err != nil || !os.SameFile(openedInfo, pathInfo) {
return fmt.Errorf("%w: path identity changed", errFileChangedDuringWrite)
}
return nil
}
+131
View File
@@ -0,0 +1,131 @@
package tools
import (
"context"
"os"
"path/filepath"
"strings"
"testing"
)
func installFileWriteRace(t *testing.T, mutate func(string)) {
t.Helper()
prior := fileWriteBeforeCommit
fileWriteBeforeCommit = mutate
t.Cleanup(func() { fileWriteBeforeCommit = prior })
}
func installFileWriteStat(t *testing.T, stat func(*os.File) (os.FileInfo, error)) {
t.Helper()
prior := fileWriteStat
fileWriteStat = stat
t.Cleanup(func() { fileWriteStat = prior })
}
func TestWriteFileRefusesCreateAndOverwriteRaces(t *testing.T) {
t.Run("create", func(t *testing.T) {
root := t.TempDir()
target := filepath.Join(root, "created.txt")
installFileWriteRace(t, func(path string) {
if err := os.WriteFile(path, []byte("other writer\n"), 0o644); err != nil {
t.Fatal(err)
}
})
result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{
"path": "created.txt", "content": "zero\n",
})
if result.Status != StatusError {
t.Fatalf("raced create status = %s, want error", result.Status)
}
if got, err := os.ReadFile(target); err != nil || string(got) != "other writer\n" {
t.Fatalf("raced create content = %q, err=%v", got, err)
}
})
t.Run("overwrite", func(t *testing.T) {
root := t.TempDir()
target := filepath.Join(root, "existing.txt")
if err := os.WriteFile(target, []byte("observed\n"), 0o644); err != nil {
t.Fatal(err)
}
installFileWriteRace(t, func(path string) {
if err := os.WriteFile(path, []byte("other writer\n"), 0o644); err != nil {
t.Fatal(err)
}
})
result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{
"path": "existing.txt", "content": "zero\n", "overwrite": true,
})
if result.Status != StatusError || !strings.Contains(result.Output, errFileChangedDuringWrite.Error()) {
t.Fatalf("raced overwrite = %s: %s", result.Status, result.Output)
}
if got, err := os.ReadFile(target); err != nil || string(got) != "other writer\n" {
t.Fatalf("raced overwrite content = %q, err=%v", got, err)
}
})
}
func TestEditFileRefusesPreimageRace(t *testing.T) {
root := t.TempDir()
target := filepath.Join(root, "existing.txt")
if err := os.WriteFile(target, []byte("observed\n"), 0o644); err != nil {
t.Fatal(err)
}
installFileWriteRace(t, func(path string) {
if err := os.WriteFile(path, []byte("other writer\n"), 0o644); err != nil {
t.Fatal(err)
}
})
result := NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{
"path": "existing.txt", "old_string": "observed", "new_string": "zero",
})
if result.Status != StatusError || !strings.Contains(result.Output, errFileChangedDuringWrite.Error()) {
t.Fatalf("raced edit = %s: %s", result.Status, result.Output)
}
if got, err := os.ReadFile(target); err != nil || string(got) != "other writer\n" {
t.Fatalf("raced edit content = %q, err=%v", got, err)
}
}
func TestOverwriteDoesNotStatOpenedFileAfterFinalPreimageComparison(t *testing.T) {
for name, run := range map[string]func(string) Result{
"write overwrite": func(root string) Result {
return NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{
"path": "existing.txt", "content": "zero\n", "overwrite": true,
})
},
"edit": func(root string) Result {
return NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{
"path": "existing.txt", "old_string": "observed", "new_string": "zero",
})
},
} {
t.Run(name, func(t *testing.T) {
root := t.TempDir()
target := filepath.Join(root, "existing.txt")
if err := os.WriteFile(target, []byte("observed\n"), 0o644); err != nil {
t.Fatal(err)
}
statCalls := 0
installFileWriteStat(t, func(file *os.File) (os.FileInfo, error) {
statCalls++
if statCalls == 2 {
// This preserves the inode, so identity-only checks cannot detect it.
if err := os.WriteFile(target, []byte("other writer\n"), 0o644); err != nil {
t.Fatal(err)
}
}
return file.Stat()
})
result := run(root)
if result.Status != StatusOK {
t.Fatalf("overwrite status = %s: %s", result.Status, result.Output)
}
if statCalls != 1 {
t.Fatalf("opened file was statted %d times; the final byte comparison must be followed directly by mutation", statCalls)
}
})
}
}
+104 -32
View File
@@ -3,11 +3,15 @@ package tools
import (
"context"
"errors"
"fmt"
"io"
"os"
"os/exec"
"path/filepath"
"strings"
"time"
"github.com/Gitlawb/zero/internal/sandbox"
)
// Format-on-write for the mutating file tools. When enabled, a successful
@@ -44,7 +48,9 @@ var formatOnWriteTimeout = 10 * time.Second
// and finds out from a CI format check it cannot see, which is the thing this
// feature exists to prevent.
type formatOnWriteResult struct {
Content string
Content string
ContentKnown bool
Info os.FileInfo
// Formatter is the binary that was run, named in the notice so the user can
// tell a slow gofmt from a slow prettier.
Formatter string
@@ -117,14 +123,32 @@ func formatOnWriteEnabled() bool {
return value != "" && value != "0" && !strings.EqualFold(value, "false")
}
// maybeFormatWrittenFile runs the configured formatter for absolutePath (when
// enabled and on PATH) and returns the file's content afterwards. Best-effort
// throughout: any failure — no formatter, formatter error, timeout, unreadable
// result — returns writtenContent so the caller's state matches the last write
// it performed itself. Only the timeout is reported back, for the reason on
// formatOnWriteResult.
var runFormatOnWriteCommand = func(ctx context.Context, binaryPath string, arguments []string, directory string) error {
formatter := exec.CommandContext(ctx, binaryPath, arguments...)
formatter.Dir = directory
formatter.Stdin = strings.NewReader("")
return formatter.Run()
}
var readFormattedFile = readRootedFile
// maybeFormatWrittenFile is the unscoped test-facing wrapper. Production
// callers use maybeFormatWrittenFileScoped so a formatter cannot redirect the
// final read or recovery write outside the configured write roots.
func maybeFormatWrittenFile(ctx context.Context, absolutePath string, writtenContent string) formatOnWriteResult {
unformatted := formatOnWriteResult{Content: writtenContent}
restoreMode := os.FileMode(0o644)
if info, err := os.Stat(absolutePath); err == nil {
restoreMode = info.Mode().Perm()
}
return maybeFormatWrittenFileScoped(ctx, filepath.Dir(absolutePath), nil, absolutePath, writtenContent, restoreMode)
}
// maybeFormatWrittenFileScoped runs the configured formatter and returns only
// content verified through a descriptor-bound root. Formatter failures restore
// writtenContent through that same root; if restoration or the final read
// fails, ContentKnown is false and callers omit exact diff evidence.
func maybeFormatWrittenFileScoped(ctx context.Context, workspaceRoot string, scope PathScope, absolutePath string, writtenContent string, restoreMode os.FileMode) formatOnWriteResult {
unformatted := formatOnWriteResult{Content: writtenContent, ContentKnown: true}
if !formatOnWriteEnabled() {
return unformatted
}
@@ -136,42 +160,90 @@ func maybeFormatWrittenFile(ctx context.Context, absolutePath string, writtenCon
if err != nil {
return unformatted
}
root, relativePath, err := openFormattedFileRoot(workspaceRoot, scope, absolutePath)
if err != nil {
unformatted.ContentKnown = false
return unformatted
}
defer root.Close()
formatCtx, cancel := context.WithTimeout(ctx, formatOnWriteTimeout)
defer cancel()
arguments := append(append([]string(nil), command[1:]...), absolutePath)
formatter := exec.CommandContext(formatCtx, binaryPath, arguments...)
formatter.Dir = filepath.Dir(absolutePath)
formatter.Stdin = strings.NewReader("")
if err := formatter.Run(); err != nil {
if err := runFormatOnWriteCommand(formatCtx, binaryPath, arguments, filepath.Dir(absolutePath)); err != nil {
unformatted.Formatter = command[0]
// THE FORMATTER EDITS IN PLACE, SO A FAILED RUN CAN LEAVE THE FILE
// NEITHER FORMATTED NOR AS WRITTEN. Killed by the deadline or by the
// caller, or exiting partway through its own rewrite, the target can hold
// a truncation. Returning the written bytes on top of that would leave the
// tracker baseline and the diff preview describing a file that is not on
// disk, which is a worse failure than the missing formatting: the next
// edit compares against content the file does not have.
//
// Written back unconditionally on this path rather than only when the
// bytes differ. Comparing first means reading the file to find out, and a
// read that fails leaves the same ambiguity this exists to remove.
if restoreErr := os.WriteFile(absolutePath, []byte(writtenContent), 0o644); restoreErr != nil {
if restoreErr := restoreFormattedFile(root, relativePath, writtenContent, restoreMode); restoreErr != nil {
unformatted.RestoreFailed = true
unformatted.ContentKnown = false
} else if restored, info, readErr := readFormattedFile(root, relativePath); readErr != nil {
unformatted.ContentKnown = false
} else {
unformatted.Content = string(restored)
unformatted.Info = info
}
// OUR deadline, not the caller's cancellation and not the formatter's own
// exit status. A cancelled tool call is already being reported as
// cancelled, and a formatter that ran and refused the file usually means
// content it could not parse, which the write itself does not promise to
// fix. Neither is this notice's business; the restore above is, for all
// three.
if errors.Is(formatCtx.Err(), context.DeadlineExceeded) && ctx.Err() == nil {
unformatted.TimedOut = true
}
return unformatted
}
formatted, err := os.ReadFile(absolutePath)
formatted, info, err := readFormattedFile(root, relativePath)
if err != nil {
unformatted.ContentKnown = false
return unformatted
}
return formatOnWriteResult{Content: string(formatted), Formatter: command[0]}
return formatOnWriteResult{Content: string(formatted), ContentKnown: true, Info: info, Formatter: command[0]}
}
func restoreFormattedFile(root *os.Root, relativePath string, content string, mode os.FileMode) error {
file, err := root.OpenFile(relativePath, os.O_WRONLY|os.O_TRUNC|os.O_CREATE, mode)
if err != nil {
return err
}
if _, err := io.WriteString(file, content); err != nil {
_ = file.Close()
return err
}
return file.Close()
}
// openFormattedFileRoot opens the write root before the formatter runs and
// computes the target relative to that descriptor-bound root. Atomic in-root
// replacement remains valid; a formatter that swaps the target to an escaping
// symlink is rejected when readFormattedFile opens it through the root.
func openFormattedFileRoot(workspaceRoot string, scope PathScope, absolutePath string) (*os.Root, string, error) {
roots, err := scopedRoots(workspaceRoot, scope)
if err != nil {
return nil, "", err
}
var firstErr error
for _, configuredRoot := range roots {
resolvedRoot, err := filepath.Abs(configuredRoot)
if err == nil {
resolvedRoot, err = filepath.EvalSymlinks(resolvedRoot)
}
if err != nil {
if firstErr == nil {
firstErr = err
}
continue
}
candidate := sandbox.NormalizePrefixForRoot(absolutePath, resolvedRoot)
relativePath, err := filepath.Rel(resolvedRoot, candidate)
if err != nil || relativePath == ".." || strings.HasPrefix(relativePath, ".."+string(filepath.Separator)) || filepath.IsAbs(relativePath) {
continue
}
root, err := os.OpenRoot(resolvedRoot)
if err != nil {
if firstErr == nil {
firstErr = err
}
continue
}
return root, relativePath, nil
}
if firstErr != nil {
return nil, "", firstErr
}
return nil, "", fmt.Errorf("%s must stay inside the configured write roots", absolutePath)
}
+411 -2
View File
@@ -5,6 +5,7 @@ import (
"os"
"os/exec"
"path/filepath"
"runtime"
"strings"
"testing"
)
@@ -98,7 +99,7 @@ func TestFormatOnWriteFormatsAndKeepsTrackerConsistent(t *testing.T) {
func TestFormatOnWriteSkipsUnknownExtensions(t *testing.T) {
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
formatting := maybeFormatWrittenFile(context.Background(), filepath.Join(t.TempDir(), "notes.xyz"), "raw text")
if formatting.Content != "raw text" {
if formatting.Content != "raw text" || !formatting.ContentKnown {
t.Fatalf("unknown extension must pass through: %q", formatting.Content)
}
if notice := formatting.notice("notes.xyz"); notice != "" {
@@ -115,10 +116,418 @@ func TestFormatOnWriteFormatterLookupFailure(t *testing.T) {
t.Fatal(err)
}
formatting := maybeFormatWrittenFile(context.Background(), targetPath, uglyContent)
if formatting.Content != uglyContent {
if formatting.Content != uglyContent || !formatting.ContentKnown {
t.Fatalf("missing formatter must return written content, got %q", formatting.Content)
}
if notice := formatting.notice("a.go"); notice != "" {
t.Fatalf("an uninstalled formatter is a standing fact, not a miss worth reporting, got %q", notice)
}
}
func TestFormatOnWriteReadsMutatedFileAfterFormatterFailure(t *testing.T) {
requireGofmt(t)
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
targetPath := filepath.Join(t.TempDir(), "a.go")
if err := os.WriteFile(targetPath, []byte("requested"), 0o644); err != nil {
t.Fatal(err)
}
priorRunner := runFormatOnWriteCommand
runFormatOnWriteCommand = func(_ context.Context, _ string, _ []string, _ string) error {
if err := os.WriteFile(targetPath, []byte("formatter-mutated"), 0o644); err != nil {
t.Fatal(err)
}
return exec.ErrNotFound
}
t.Cleanup(func() { runFormatOnWriteCommand = priorRunner })
formatting := maybeFormatWrittenFile(context.Background(), targetPath, "requested")
if !formatting.ContentKnown || formatting.Content != "requested" {
t.Fatalf("formatter failure content = %q, known=%t", formatting.Content, formatting.ContentKnown)
}
}
func TestFormatOnWriteReportsWhenFormatterFailureCannotBeRestored(t *testing.T) {
requireGofmt(t)
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
targetPath := filepath.Join(t.TempDir(), "a.go")
if err := os.WriteFile(targetPath, []byte("requested"), 0o644); err != nil {
t.Fatal(err)
}
priorRunner := runFormatOnWriteCommand
runFormatOnWriteCommand = func(_ context.Context, _ string, _ []string, _ string) error {
if err := os.Remove(targetPath); err != nil {
t.Fatal(err)
}
if err := os.Mkdir(targetPath, 0o755); err != nil {
t.Fatal(err)
}
return exec.ErrNotFound
}
t.Cleanup(func() { runFormatOnWriteCommand = priorRunner })
formatting := maybeFormatWrittenFile(context.Background(), targetPath, "requested")
if !formatting.RestoreFailed {
t.Fatal("failed recovery was not reported")
}
if formatting.ContentKnown {
t.Fatalf("failed recovery reported known content %q", formatting.Content)
}
if notice := formatting.notice("a.go"); !strings.Contains(notice, "WARNING: a.go may not hold what was written") {
t.Fatalf("failed recovery notice = %q", notice)
}
}
func TestWriteFileRecreatesFormatterDeletedTargetWithOriginalMode(t *testing.T) {
requireGofmt(t)
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
for _, tc := range []struct {
name string
existing bool
wantMode os.FileMode
overwrite bool
}{
{name: "create", wantMode: 0o644},
{name: "overwrite", existing: true, wantMode: 0o751, overwrite: true},
} {
t.Run(tc.name, func(t *testing.T) {
root := t.TempDir()
targetPath := filepath.Join(root, "a.go")
if tc.existing {
if err := os.WriteFile(targetPath, []byte("before\n"), tc.wantMode); err != nil {
t.Fatal(err)
}
}
priorRunner := runFormatOnWriteCommand
runFormatOnWriteCommand = func(_ context.Context, _ string, _ []string, _ string) error {
if err := os.Remove(targetPath); err != nil {
t.Fatal(err)
}
return exec.ErrNotFound
}
t.Cleanup(func() { runFormatOnWriteCommand = priorRunner })
result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{
"path": "a.go", "content": "requested\n", "overwrite": tc.overwrite,
})
if result.Status != StatusOK {
t.Fatalf("write status = %s: %s", result.Status, result.Output)
}
content, err := os.ReadFile(targetPath)
if err != nil || string(content) != "requested\n" {
t.Fatalf("recreated content = %q, err=%v", content, err)
}
if runtime.GOOS != "windows" {
info, err := os.Stat(targetPath)
if err != nil {
t.Fatal(err)
}
if got := info.Mode().Perm(); got != tc.wantMode {
t.Fatalf("recreated mode = %o, want %o", got, tc.wantMode)
}
}
})
}
}
func TestFormatOnWriteMarksUnreadableFinalStateUnknown(t *testing.T) {
requireGofmt(t)
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
targetPath := filepath.Join(t.TempDir(), "a.go")
if err := os.WriteFile(targetPath, []byte("requested"), 0o644); err != nil {
t.Fatal(err)
}
priorRunner := runFormatOnWriteCommand
priorReader := readFormattedFile
runFormatOnWriteCommand = func(context.Context, string, []string, string) error { return nil }
readFormattedFile = func(*os.Root, string) ([]byte, os.FileInfo, error) { return nil, nil, os.ErrPermission }
t.Cleanup(func() {
runFormatOnWriteCommand = priorRunner
readFormattedFile = priorReader
})
formatting := maybeFormatWrittenFile(context.Background(), targetPath, "requested")
if formatting.ContentKnown || formatting.Content != "requested" {
t.Fatalf("unreadable formatter result = %q, known=%t", formatting.Content, formatting.ContentKnown)
}
}
func TestWriteFileUsesVerifiedBytesAfterFormatterFailure(t *testing.T) {
requireGofmt(t)
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
root := t.TempDir()
targetPath := filepath.Join(root, "a.go")
priorRunner := runFormatOnWriteCommand
runFormatOnWriteCommand = func(_ context.Context, _ string, _ []string, _ string) error {
if err := os.WriteFile(targetPath, []byte("formatter-mutated\n"), 0o644); err != nil {
t.Fatal(err)
}
return exec.ErrNotFound
}
t.Cleanup(func() { runFormatOnWriteCommand = priorRunner })
result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{
"path": "a.go", "content": "requested\n",
})
if result.Status != StatusOK {
t.Fatalf("write status = %s: %s", result.Status, result.Output)
}
if got := result.FileDiffs; len(got) != 1 || got[0].NewText != "requested\n" {
t.Fatalf("formatter-failure FileDiff = %#v", got)
}
}
func TestEditFileUsesVerifiedBytesAfterFormatterFailure(t *testing.T) {
requireGofmt(t)
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
root := t.TempDir()
targetPath := filepath.Join(root, "a.go")
if err := os.WriteFile(targetPath, []byte("before\n"), 0o644); err != nil {
t.Fatal(err)
}
priorRunner := runFormatOnWriteCommand
runFormatOnWriteCommand = func(_ context.Context, _ string, _ []string, _ string) error {
if err := os.WriteFile(targetPath, []byte("formatter-mutated\n"), 0o644); err != nil {
t.Fatal(err)
}
return exec.ErrNotFound
}
t.Cleanup(func() { runFormatOnWriteCommand = priorRunner })
result := NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{
"path": "a.go", "old_string": "before", "new_string": "requested",
})
if result.Status != StatusOK {
t.Fatalf("edit status = %s: %s", result.Status, result.Output)
}
if got := result.FileDiffs; len(got) != 1 || got[0].OldText != "before\n" || got[0].NewText != "requested\n" {
t.Fatalf("formatter-failure edit FileDiff = %#v", got)
}
}
func TestWriteFileOmitsRichDiffWhenFormatterFinalReadFails(t *testing.T) {
requireGofmt(t)
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
root := t.TempDir()
priorRunner := runFormatOnWriteCommand
priorReader := readFormattedFile
runFormatOnWriteCommand = func(context.Context, string, []string, string) error { return nil }
readFormattedFile = func(*os.Root, string) ([]byte, os.FileInfo, error) { return nil, nil, os.ErrPermission }
t.Cleanup(func() {
runFormatOnWriteCommand = priorRunner
readFormattedFile = priorReader
})
result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{
"path": "a.go", "content": "requested\n",
})
if result.Status != StatusOK || len(result.ChangedFiles) != 1 || len(result.FileDiffs) != 0 {
t.Fatalf("unverified formatter result = status=%s changed=%#v diffs=%#v", result.Status, result.ChangedFiles, result.FileDiffs)
}
if result.Display.Preview != "" {
t.Fatalf("unverified formatter result exposed stale preview: %q", result.Display.Preview)
}
}
func TestEditFileOmitsPreviewWhenFormatterFinalReadFails(t *testing.T) {
requireGofmt(t)
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
root := t.TempDir()
targetPath := filepath.Join(root, "a.go")
if err := os.WriteFile(targetPath, []byte("before\n"), 0o644); err != nil {
t.Fatal(err)
}
priorRunner := runFormatOnWriteCommand
priorReader := readFormattedFile
runFormatOnWriteCommand = func(context.Context, string, []string, string) error { return nil }
readFormattedFile = func(*os.Root, string) ([]byte, os.FileInfo, error) { return nil, nil, os.ErrPermission }
t.Cleanup(func() {
runFormatOnWriteCommand = priorRunner
readFormattedFile = priorReader
})
result := NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{
"path": "a.go", "old_string": "before", "new_string": "requested",
})
if result.Status != StatusOK || len(result.ChangedFiles) != 1 || len(result.FileDiffs) != 0 {
t.Fatalf("unverified formatter result = status=%s changed=%#v diffs=%#v", result.Status, result.ChangedFiles, result.FileDiffs)
}
if result.Display.Preview != "" {
t.Fatalf("unverified formatter result exposed stale preview: %q", result.Display.Preview)
}
}
func TestWriteFileOmitsRichEvidenceWhenFormatterReplacesTargetWithOutOfRootSymlink(t *testing.T) {
requireGofmt(t)
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
root := t.TempDir()
targetPath := filepath.Join(root, "a.go")
resolvedRoot, err := filepath.EvalSymlinks(root)
if err != nil {
t.Fatal(err)
}
trackedPath := filepath.Join(resolvedRoot, "a.go")
outsidePath := filepath.Join(t.TempDir(), "outside.go")
outsideContent := "package external\n\nconst Secret = \"outside\"\n"
if err := os.WriteFile(outsidePath, []byte(outsideContent), 0o644); err != nil {
t.Fatal(err)
}
priorRunner := runFormatOnWriteCommand
runFormatOnWriteCommand = func(context.Context, string, []string, string) error {
if err := os.Remove(targetPath); err != nil {
t.Fatal(err)
}
if err := os.Symlink(outsidePath, targetPath); err != nil {
t.Skipf("symlinks unavailable: %v", err)
}
return nil
}
t.Cleanup(func() { runFormatOnWriteCommand = priorRunner })
tracker := NewFileTracker()
diagnosticsCalled := false
result := NewScopedWriteFileTool(root, nil).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{
"path": "a.go", "content": "package requested\n",
}, RunOptions{FileTracker: tracker, Diagnostics: func(context.Context, string) string {
diagnosticsCalled = true
return "must not run"
}})
if result.Status != StatusOK || len(result.ChangedFiles) != 1 || len(result.FileDiffs) != 0 || result.Display.Preview != "" {
t.Fatalf("out-of-root formatter result = status=%s changed=%#v diffs=%#v preview=%q output=%q", result.Status, result.ChangedFiles, result.FileDiffs, result.Display.Preview, result.Output)
}
if diagnosticsCalled {
t.Fatal("diagnostics must not inspect an unverified formatter target")
}
if _, tracked := tracker.Version(trackedPath); tracked {
t.Fatal("out-of-root formatter target must not be recorded in the tracker")
}
}
func TestEditFileOmitsRichEvidenceWhenFormatterReplacesTargetWithOutOfRootSymlink(t *testing.T) {
requireGofmt(t)
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
root := t.TempDir()
targetPath := filepath.Join(root, "a.go")
resolvedRoot, err := filepath.EvalSymlinks(root)
if err != nil {
t.Fatal(err)
}
trackedPath := filepath.Join(resolvedRoot, "a.go")
if err := os.WriteFile(targetPath, []byte("package before\n"), 0o644); err != nil {
t.Fatal(err)
}
outsidePath := filepath.Join(t.TempDir(), "outside.go")
outsideContent := "package external\n\nconst Secret = \"outside\"\n"
if err := os.WriteFile(outsidePath, []byte(outsideContent), 0o644); err != nil {
t.Fatal(err)
}
priorRunner := runFormatOnWriteCommand
runFormatOnWriteCommand = func(context.Context, string, []string, string) error {
if err := os.Remove(targetPath); err != nil {
t.Fatal(err)
}
if err := os.Symlink(outsidePath, targetPath); err != nil {
t.Skipf("symlinks unavailable: %v", err)
}
return nil
}
t.Cleanup(func() { runFormatOnWriteCommand = priorRunner })
tracker := NewFileTracker()
read := NewScopedReadFileTool(root, nil).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{
"path": "a.go",
}, RunOptions{FileTracker: tracker})
if read.Status != StatusOK {
t.Fatalf("read before edit failed: %s", read.Output)
}
diagnosticsCalled := false
result := NewScopedEditFileTool(root, nil).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{
"path": "a.go", "old_string": "before", "new_string": "requested",
}, RunOptions{FileTracker: tracker, Diagnostics: func(context.Context, string) string {
diagnosticsCalled = true
return "must not run"
}})
if result.Status != StatusOK || len(result.ChangedFiles) != 1 || len(result.FileDiffs) != 0 || result.Display.Preview != "" {
t.Fatalf("out-of-root formatter result = status=%s changed=%#v diffs=%#v preview=%q output=%q", result.Status, result.ChangedFiles, result.FileDiffs, result.Display.Preview, result.Output)
}
if diagnosticsCalled {
t.Fatal("diagnostics must not inspect an unverified formatter target")
}
if _, tracked := tracker.Version(trackedPath); tracked {
t.Fatal("out-of-root formatter target must not be recorded in the tracker")
}
}
func TestWriteFileAcceptsInRootAtomicFormatterReplacement(t *testing.T) {
requireGofmt(t)
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
root := t.TempDir()
targetPath := filepath.Join(root, "a.go")
formatted := "package formatted\n"
priorRunner := runFormatOnWriteCommand
runFormatOnWriteCommand = func(context.Context, string, []string, string) error {
tempPath := filepath.Join(root, "formatter.tmp")
if err := os.WriteFile(tempPath, []byte(formatted), 0o644); err != nil {
t.Fatal(err)
}
return os.Rename(tempPath, targetPath)
}
t.Cleanup(func() { runFormatOnWriteCommand = priorRunner })
tracker := NewFileTracker()
result := NewScopedWriteFileTool(root, nil).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{
"path": "a.go", "content": "package requested\n",
}, RunOptions{FileTracker: tracker})
if result.Status != StatusOK || len(result.FileDiffs) != 1 || result.FileDiffs[0].NewText != formatted {
t.Fatalf("atomic formatter write = status=%s diffs=%#v output=%q", result.Status, result.FileDiffs, result.Output)
}
resolvedRoot, err := filepath.EvalSymlinks(root)
if err != nil {
t.Fatal(err)
}
version, tracked := tracker.Version(filepath.Join(resolvedRoot, "a.go"))
if !tracked || version.Hash != HashContent([]byte(formatted)) {
t.Fatalf("atomic formatter tracker = %#v, tracked=%t", version, tracked)
}
}
func TestEditFileAcceptsInRootAtomicFormatterReplacement(t *testing.T) {
requireGofmt(t)
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
root := t.TempDir()
targetPath := filepath.Join(root, "a.go")
if err := os.WriteFile(targetPath, []byte("package before\n"), 0o644); err != nil {
t.Fatal(err)
}
formatted := "package formatted\n"
priorRunner := runFormatOnWriteCommand
runFormatOnWriteCommand = func(context.Context, string, []string, string) error {
tempPath := filepath.Join(root, "formatter.tmp")
if err := os.WriteFile(tempPath, []byte(formatted), 0o644); err != nil {
t.Fatal(err)
}
return os.Rename(tempPath, targetPath)
}
t.Cleanup(func() { runFormatOnWriteCommand = priorRunner })
tracker := NewFileTracker()
read := NewScopedReadFileTool(root, nil).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{
"path": "a.go",
}, RunOptions{FileTracker: tracker})
if read.Status != StatusOK {
t.Fatalf("read before edit failed: %s", read.Output)
}
result := NewScopedEditFileTool(root, nil).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{
"path": "a.go", "old_string": "before", "new_string": "requested",
}, RunOptions{FileTracker: tracker})
if result.Status != StatusOK || len(result.FileDiffs) != 1 || result.FileDiffs[0].NewText != formatted {
t.Fatalf("atomic formatter edit = status=%s diffs=%#v output=%q", result.Status, result.FileDiffs, result.Output)
}
resolvedRoot, err := filepath.EvalSymlinks(root)
if err != nil {
t.Fatal(err)
}
version, tracked := tracker.Version(filepath.Join(resolvedRoot, "a.go"))
if !tracked || version.Hash != HashContent([]byte(formatted)) {
t.Fatalf("atomic formatter tracker = %#v, tracked=%t", version, tracked)
}
}
+34
View File
@@ -343,6 +343,33 @@ func scrubResultSecrets(res Result) Result {
res.Display.Preview = scrubbed
res.Redacted = true
}
fileDiffs := make([]FileDiff, 0, len(res.FileDiffs))
for _, diff := range res.FileDiffs {
// Never normalize control bytes in a diff: normalizing after redaction can
// reassemble a split credential. Decline unsafe rich content entirely and
// leave ChangedFiles as the safe fallback.
if unsafeDiffText(diff.OldText) || unsafeDiffText(diff.NewText) {
res.Redacted = true
continue
}
if scrubbed := redaction.RedactString(diff.OldText, redaction.Options{}); scrubbed != diff.OldText {
diff.OldText = scrubbed
res.Redacted = true
}
if scrubbed := redaction.RedactString(diff.NewText, redaction.Options{}); scrubbed != diff.NewText {
diff.NewText = scrubbed
res.Redacted = true
}
// Redaction can collapse two distinct credentials to the same token. At
// this final outbound boundary, retain rich evidence only when the
// transformed sides still describe a real transition; ChangedFiles
// remains the safe path-only fallback.
if diff.OldExists == diff.NewExists && diff.OldText == diff.NewText {
continue
}
fileDiffs = append(fileDiffs, diff)
}
res.FileDiffs = fileDiffs
// Meta values carry model-controlled strings (e.g. glob pattern, bash cwd) and
// are forwarded into the transcript, so they are part of the boundary too.
for key, value := range res.Meta {
@@ -354,6 +381,13 @@ func scrubResultSecrets(res Result) Result {
return res
}
// ScrubResultSecrets applies the registry's transcript boundary to a result
// that was produced before Registry.RunWithOptions could own it, such as a
// local pre-permission rejection in the agent loop.
func ScrubResultSecrets(res Result) Result {
return scrubResultSecrets(res)
}
func CoreReadOnlyToolsScoped(workspaceRoot string, scope PathScope) []Tool {
return []Tool{
NewScopedReadMinifiedFileTool(workspaceRoot, scope),
+108 -1
View File
@@ -445,13 +445,120 @@ func (t denyTool) Run(context.Context, map[string]any) Result { return Result{St
// must be scrubbed too, not just the tool-execution paths.
func TestScrubResultSecretsRedactsPreview(t *testing.T) {
secret := "ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
res := scrubResultSecrets(Result{Display: Display{Preview: "+++ b/x\n+token := \"" + secret + "\""}})
res := scrubResultSecrets(Result{
Display: Display{Preview: "+++ b/x\n+token := \"" + secret + "\""},
FileDiffs: []FileDiff{{Path: filepath.Join(t.TempDir(), "x"), OldExists: true, NewExists: true, OldText: "before " + secret, NewText: "after " + secret}},
})
if strings.Contains(res.Display.Preview, secret) {
t.Errorf("Display.Preview (the card-only code preview) must be redacted, leaked: %q", res.Display.Preview)
}
if !res.Redacted {
t.Error("scrubbing a secret from the preview should set Redacted")
}
if len(res.FileDiffs) != 1 || strings.Contains(res.FileDiffs[0].OldText, secret) || strings.Contains(res.FileDiffs[0].NewText, secret) {
t.Errorf("FileDiff must be redacted: %#v", res.FileDiffs)
}
}
func TestScrubResultSecretsDropsControlSplitFileDiff(t *testing.T) {
secret := "sk-proj-abcdefghijklmnopqrstuvwxyz"
res := scrubResultSecrets(Result{FileDiffs: []FileDiff{{
Path: filepath.Join(t.TempDir(), "x"),
OldExists: true,
NewExists: true,
OldText: "token=" + secret[:12] + "\x00" + secret[12:],
NewText: "safe",
}}})
if len(res.FileDiffs) != 0 || !res.Redacted {
t.Fatalf("unsafe FileDiff = %#v, redacted = %t", res.FileDiffs, res.Redacted)
}
}
func TestScrubResultSecretsDropsDefaultIgnorableSplitFileDiffOnEitherSide(t *testing.T) {
secret := "sk-ant-api03-AAAABBBBCCCCDDDDEEEEFFFFGGGG"
for name, separator := range map[string]string{
"combining grapheme joiner": "\u034f",
"variation selector": "\ufe0f",
} {
for _, side := range []string{"old", "new"} {
t.Run(name+" "+side, func(t *testing.T) {
obfuscated := secret[:20] + separator + secret[20:]
diff := FileDiff{
Path: filepath.Join(t.TempDir(), "x"), OldExists: true, NewExists: true,
OldText: "safe old", NewText: "safe new",
}
if side == "old" {
diff.OldText = obfuscated
} else {
diff.NewText = obfuscated
}
res := scrubResultSecrets(Result{FileDiffs: []FileDiff{diff}})
if len(res.FileDiffs) != 0 || !res.Redacted {
t.Fatalf("unsafe FileDiff = %#v, redacted = %t", res.FileDiffs, res.Redacted)
}
})
}
}
}
func TestScrubResultSecretsDoesNotMutateCallerFileDiffSlice(t *testing.T) {
path := filepath.Join(t.TempDir(), "x")
secret := "ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
retainedOld := "before token=" + secret
retainedNew := "after token=" + secret
original := []FileDiff{
{Path: path, OldExists: true, NewExists: true, OldText: "token=sk-proj-abc\x00def", NewText: "unsafe"},
{Path: path, OldExists: true, NewExists: true, OldText: retainedOld, NewText: retainedNew},
}
result := scrubResultSecrets(Result{FileDiffs: original})
if len(result.FileDiffs) != 1 || strings.Contains(result.FileDiffs[0].OldText, secret) || strings.Contains(result.FileDiffs[0].NewText, secret) {
t.Fatalf("filtered FileDiffs = %#v", result.FileDiffs)
}
if original[0].NewText != "unsafe" || original[1].OldText != retainedOld || original[1].NewText != retainedNew {
t.Fatalf("caller slice was mutated: %#v", original)
}
}
func TestScrubResultSecretsDropsSemanticallyUnchangedRedactedDiff(t *testing.T) {
path := filepath.Join(t.TempDir(), "credentials.txt")
oldSecret := "ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
newSecret := "ghp_9876543210ZYXWVUTSRQPONMLKJIHGFEDCBA"
result := scrubResultSecrets(Result{
ChangedFiles: []string{"credentials.txt"},
FileDiffs: []FileDiff{{
Path: path, OldExists: true, NewExists: true,
OldText: "token=" + oldSecret, NewText: "token=" + newSecret,
}},
})
if len(result.FileDiffs) != 0 {
t.Fatalf("semantically unchanged redacted diff = %#v", result.FileDiffs)
}
if got, want := result.ChangedFiles, []string{"credentials.txt"}; !slices.Equal(got, want) {
t.Fatalf("ChangedFiles = %#v, want %#v", got, want)
}
if !result.Redacted {
t.Fatal("credential rotation must set Redacted")
}
}
func TestScrubResultSecretsKeepsRealTransitionAroundRedactedSecrets(t *testing.T) {
path := filepath.Join(t.TempDir(), "credentials.txt")
oldSecret := "ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
newSecret := "ghp_9876543210ZYXWVUTSRQPONMLKJIHGFEDCBA"
result := scrubResultSecrets(Result{FileDiffs: []FileDiff{{
Path: path, OldExists: true, NewExists: true,
OldText: "environment=staging token=" + oldSecret,
NewText: "environment=production token=" + newSecret,
}}})
if len(result.FileDiffs) != 1 {
t.Fatalf("redacted real transition = %#v", result.FileDiffs)
}
if result.FileDiffs[0].OldText == result.FileDiffs[0].NewText {
t.Fatalf("redacted transition became unchanged: %#v", result.FileDiffs[0])
}
if strings.Contains(result.FileDiffs[0].OldText, oldSecret) || strings.Contains(result.FileDiffs[0].NewText, newSecret) {
t.Fatalf("redacted transition leaked a secret: %#v", result.FileDiffs[0])
}
}
func TestRunWithOptionsScrubsSecretsOnDenialPaths(t *testing.T) {
+43
View File
@@ -0,0 +1,43 @@
package tools
import (
"fmt"
"io"
"os"
)
// readRootedFile returns bytes and identity from the same opened object, and
// verifies that the rooted path still names that object after the read. Root.Open
// binds symlink containment to use rather than relying on a pathname pre-check.
func readRootedFile(root *os.Root, relativePath string) ([]byte, os.FileInfo, error) {
file, err := root.Open(relativePath)
if err != nil {
return nil, nil, err
}
closed := false
defer func() {
if !closed {
_ = file.Close()
}
}()
openedInfo, err := file.Stat()
if err != nil {
return nil, nil, err
}
if !openedInfo.Mode().IsRegular() {
return nil, nil, fmt.Errorf("%s is not a regular file", relativePath)
}
content, err := io.ReadAll(file)
if err != nil {
return nil, nil, err
}
pathInfo, err := root.Stat(relativePath)
if err != nil || !os.SameFile(openedInfo, pathInfo) {
return nil, nil, fmt.Errorf("%w: path identity changed", errFileChangedDuringWrite)
}
if err := file.Close(); err != nil {
return nil, nil, err
}
closed = true
return content, openedInfo, nil
}
+255 -55
View File
@@ -86,12 +86,13 @@ type structuredPatchTarget struct {
}
type structuredPatchChange struct {
kind structuredPatchKind
from structuredPatchTarget
to structuredPatchTarget
before string
after string
mode os.FileMode
kind structuredPatchKind
from structuredPatchTarget
to structuredPatchTarget
before string
after string
mode os.FileMode
beforeInfo os.FileInfo
}
// unifiedHunkRangePattern recognises a unified-diff range header ("-12,4 +12,6",
@@ -156,8 +157,14 @@ func applyPatchOperations(applyRoot, relativeRoot string, operations []structure
}
}
}
if err := applyStructuredPatchChanges(workspace, changes, options.FileTracker); err != nil {
return errorResult("Error applying patch: " + err.Error())
applyOutcome, err := applyStructuredPatchChanges(workspace, relativeRoot, changes, options.FileTracker)
if err != nil {
result := errorResult("Error applying patch: " + err.Error())
result.ChangedFiles = changedFilesFromStructuredPatch(relativeRoot, applyOutcome.committed)
result.FileDiffs = fileDiffsFromStructuredPatch(relativeRoot, applyOutcome.committed)
result.Redacted = structuredPatchContainsObfuscatedSecret(applyOutcome.committed)
result.Display = Display{Summary: result.Output, Kind: "diff", Preview: structuredPatchPreview(applyOutcome.committed)}
return result
}
for _, change := range changes {
@@ -183,10 +190,78 @@ func applyPatchOperations(applyRoot, relativeRoot string, operations []structure
}
result := okResult(summary)
result.ChangedFiles = changedFilesFromStructuredPatch(relativeRoot, changes)
result.FileDiffs = fileDiffsFromStructuredPatch(relativeRoot, changes)
result.Redacted = structuredPatchContainsObfuscatedSecret(changes)
result.Display = Display{Summary: summary, Kind: "diff", Preview: structuredPatchPreview(changes)}
return result
}
func structuredPatchContainsObfuscatedSecret(changes []structuredPatchChange) bool {
for _, change := range changes {
if diffTextRevealsObfuscatedSecret(change.before) || diffTextRevealsObfuscatedSecret(change.after) {
return true
}
}
return false
}
func fileDiffsFromStructuredPatch(_ string, changes []structuredPatchChange) []FileDiff {
const maxToolResultFileDiffs = 64
diffs := make([]FileDiff, 0, len(changes)*2)
usedBytes := 0
appendGroup := func(group ...FileDiff) bool {
if len(group) == 0 || len(diffs)+len(group) > maxToolResultFileDiffs {
return false
}
groupBytes := 0
for _, diff := range group {
groupBytes += len(diff.OldText) + len(diff.NewText)
}
if usedBytes+groupBytes > maxToolResultFileDiffBytes {
return false
}
diffs = append(diffs, group...)
usedBytes += groupBytes
return true
}
makeDiff := func(path, before, after string, oldExists, newExists bool) (FileDiff, bool) {
return boundedFileDiff(path, before, after, oldExists, newExists)
}
for _, change := range changes {
var group []FileDiff
switch {
case change.kind == structuredPatchDelete:
if diff, ok := makeDiff(change.from.absolute, change.before, "", true, false); ok {
group = append(group, diff)
}
case change.kind == structuredPatchAdd:
if diff, ok := makeDiff(change.to.absolute, "", change.after, false, true); ok {
group = append(group, diff)
}
case change.kind == structuredPatchCopy && change.from.absolute != change.to.absolute:
// A copy leaves its source unchanged; the destination is a create.
if diff, ok := makeDiff(change.to.absolute, "", change.after, false, true); ok {
group = append(group, diff)
}
case change.kind == structuredPatchUpdate && change.from.absolute != change.to.absolute:
// A move is two filesystem changes, not a destination overwrite.
from, fromOK := makeDiff(change.from.absolute, change.before, "", true, false)
to, toOK := makeDiff(change.to.absolute, "", change.after, false, true)
if fromOK && toOK {
group = append(group, from, to)
}
default:
if diff, ok := makeDiff(change.to.absolute, change.before, change.after, true, true); ok {
group = append(group, diff)
}
}
if len(group) > 0 && !appendGroup(group...) {
break
}
}
return diffs
}
func parseStructuredPatch(patch string) ([]structuredPatchOperation, error) {
normalized := strings.TrimSpace(strings.TrimPrefix(strings.ReplaceAll(patch, "\r\n", "\n"), "\ufeff"))
if normalized == "" {
@@ -394,15 +469,12 @@ func planStructuredPatch(root *os.Root, operations []structuredPatchOperation, t
}
change.after = operation.contents
case structuredPatchDelete, structuredPatchUpdate, structuredPatchCopy:
info, err := root.Stat(from.relative)
if err != nil {
return nil, fmt.Errorf("stating %s: %w", from.relative, err)
}
change.mode = info.Mode()
content, err := root.ReadFile(from.relative)
content, info, err := readRootedFile(root, from.relative)
if err != nil {
return nil, fmt.Errorf("reading %s: %w", from.relative, err)
}
change.mode = info.Mode()
change.beforeInfo = info
if err := tracker.CheckConflict(from.absolute, content); err != nil {
return nil, fmt.Errorf("%s", fileConflictMessage(from.relative))
}
@@ -672,38 +744,112 @@ func findStructuredPatchSequence(lines, wanted []string, start int, endOfFile bo
return -1, false
}
func applyStructuredPatchChanges(root *os.Root, changes []structuredPatchChange, tracker *FileTracker) error {
type structuredPatchApplyOutcome struct {
committed []structuredPatchChange
incompletePaths []string
}
type structuredPatchChangeOutcome struct {
completed []structuredPatchChange
incompletePaths []string
}
func applyStructuredPatchChanges(root *os.Root, relativeRoot string, changes []structuredPatchChange, tracker *FileTracker) (structuredPatchApplyOutcome, error) {
// committed lists, in order, the paths whose change reached disk before a
// later change failed, so the caller (and the model) knows exactly which
// files now hold the patched content and which were never touched.
var committed []string
var outcome structuredPatchApplyOutcome
for _, change := range changes {
done, err := applyStructuredPatchChange(root, change)
if done {
committed = append(committed, structuredPatchChangePaths(change)...)
}
changeOutcome, err := applyStructuredPatchChange(root, change)
outcome.committed = append(outcome.committed, changeOutcome.completed...)
outcome.incompletePaths = append(outcome.incompletePaths, changeOutcome.incompletePaths...)
if err != nil {
forgetStructuredPatchFiles(tracker, changes)
if len(committed) > 0 {
return fmt.Errorf("%w; patch was partially applied — already committed: %s; the remaining files are unchanged; re-read the committed files before retrying", err, strings.Join(committed, ", "))
committedPaths := changedFilesFromStructuredPatch(relativeRoot, outcome.committed)
unverifiedPaths := appendUniqueStructuredPatchPaths(nil, relativeRoot, outcome.incompletePaths)
if len(committedPaths) > 0 || len(unverifiedPaths) > 0 {
parts := []string{"patch was partially applied"}
if len(committedPaths) > 0 {
parts = append(parts, "already committed: "+strings.Join(committedPaths, ", "))
}
if len(unverifiedPaths) > 0 {
parts = append(parts, "published but unverified: "+strings.Join(unverifiedPaths, ", "))
}
parts = append(parts, "the remaining files are unchanged")
if len(unverifiedPaths) > 0 {
parts = append(parts, "re-read the listed files before retrying")
} else {
parts = append(parts, "re-read the committed files before retrying")
}
return outcome, fmt.Errorf("%w; %s", err, strings.Join(parts, "; "))
}
return err
return outcome, err
}
}
return nil
return outcome, nil
}
// structuredPatchChangePaths names the workspace-relative paths a committed
// change touched: the destination, plus the source of a move or copy.
func structuredPatchChangePaths(change structuredPatchChange) []string {
if change.kind == structuredPatchDelete {
return []string{change.from.relative}
// completedStructuredPatchEffect turns a partially completed compound change
// into the exact filesystem sub-effect that is still verifiable after the
// error. In particular, a failed move may have published its destination while
// leaving the source in place; that is a destination creation, not a move.
func completedStructuredPatchEffect(root *os.Root, change structuredPatchChange) (structuredPatchChange, bool) {
if change.to.relative == "" {
return structuredPatchChange{}, false
}
paths := []string{change.to.relative}
if change.from.absolute != change.to.absolute && change.from.relative != "" {
paths = append([]string{change.from.relative}, paths...)
content, _, err := readRootedFile(root, change.to.relative)
if err != nil || string(content) != change.after {
return structuredPatchChange{}, false
}
return paths
switch change.kind {
case structuredPatchAdd, structuredPatchCopy:
return structuredPatchChange{
kind: structuredPatchAdd,
to: change.to,
after: change.after,
mode: change.mode,
}, true
case structuredPatchUpdate:
if change.from.absolute != change.to.absolute {
return structuredPatchChange{
kind: structuredPatchAdd,
to: change.to,
after: change.after,
mode: change.mode,
}, true
}
}
return structuredPatchChange{}, false
}
func incompleteStructuredPatchWrite(root *os.Root, change structuredPatchChange, published bool) structuredPatchChangeOutcome {
if !published {
return structuredPatchChangeOutcome{}
}
if completed, ok := completedStructuredPatchEffect(root, change); ok {
return structuredPatchChangeOutcome{completed: []structuredPatchChange{completed}}
}
if change.to.relative != "" {
return structuredPatchChangeOutcome{incompletePaths: []string{change.to.relative}}
}
return structuredPatchChangeOutcome{}
}
func appendUniqueStructuredPatchPaths(existing []string, relativeRoot string, paths []string) []string {
seen := make(map[string]bool, len(existing)+len(paths))
for _, path := range existing {
seen[path] = true
}
for _, path := range paths {
if relativeRoot != "" && relativeRoot != "." {
path = filepath.ToSlash(filepath.Join(relativeRoot, path))
}
if path != "" && !seen[path] {
seen[path] = true
existing = append(existing, path)
}
}
return existing
}
func forgetStructuredPatchFiles(tracker *FileTracker, changes []structuredPatchChange) {
@@ -723,7 +869,17 @@ func forgetStructuredPatchFiles(tracker *FileTracker, changes []structuredPatchC
// deterministically; it is nil in production.
var structuredPatchBeforeCommit func(change structuredPatchChange)
func applyStructuredPatchChange(root *os.Root, change structuredPatchChange) (bool, error) {
// structuredPatchBeforeRename runs after a same-path update has staged and
// closed its replacement but before the final preimage recheck and rename.
// Tests use it to reproduce a competing writer deterministically.
var structuredPatchBeforeRename func(change structuredPatchChange)
// structuredPatchRemove is a deterministic test seam for failures after a
// move destination has been published. Production removes through the opened
// workspace root.
var structuredPatchRemove = func(root *os.Root, name string) error { return root.Remove(name) }
func applyStructuredPatchChange(root *os.Root, change structuredPatchChange) (structuredPatchChangeOutcome, error) {
if structuredPatchBeforeCommit != nil {
structuredPatchBeforeCommit(change)
}
@@ -732,39 +888,63 @@ func applyStructuredPatchChange(root *os.Root, change structuredPatchChange) (bo
// by another process in between is refused rather than overwritten or
// removed.
if change.kind != structuredPatchAdd {
current, err := root.ReadFile(change.from.relative)
if err != nil {
return false, fmt.Errorf("re-reading %s before commit: %w", change.from.relative, err)
}
if string(current) != change.before {
return false, fmt.Errorf("%s changed on disk between planning and commit; re-read it and retry", change.from.relative)
if err := recheckStructuredPatchPreimage(root, change); err != nil {
return structuredPatchChangeOutcome{}, err
}
}
switch change.kind {
case structuredPatchDelete:
if err := root.Remove(change.from.relative); err != nil {
return false, fmt.Errorf("deleting %s: %w", change.from.relative, err)
if err := structuredPatchRemove(root, change.from.relative); err != nil {
return structuredPatchChangeOutcome{}, fmt.Errorf("deleting %s: %w", change.from.relative, err)
}
return true, nil
return structuredPatchChangeOutcome{completed: []structuredPatchChange{change}}, nil
case structuredPatchAdd, structuredPatchCopy:
return writeStructuredPatchFile(root, change.to, change.after, change.mode, true)
var beforePublish func() error
if change.kind == structuredPatchCopy {
beforePublish = structuredPatchPrePublishGuard(root, change)
}
published, err := writeStructuredPatchFile(root, change.to, change.after, change.mode, true, beforePublish)
if err != nil {
return incompleteStructuredPatchWrite(root, change, published), err
}
return structuredPatchChangeOutcome{completed: []structuredPatchChange{change}}, nil
case structuredPatchUpdate:
moving := change.from.absolute != change.to.absolute
committed, err := writeStructuredPatchFile(root, change.to, change.after, change.mode, moving)
published, err := writeStructuredPatchFile(root, change.to, change.after, change.mode, moving, structuredPatchPrePublishGuard(root, change))
if err != nil {
return committed, err
return incompleteStructuredPatchWrite(root, change, published), err
}
if moving {
if err := root.Remove(change.from.relative); err != nil {
return true, fmt.Errorf("removing moved source %s: %w", change.from.relative, err)
if err := structuredPatchRemove(root, change.from.relative); err != nil {
return incompleteStructuredPatchWrite(root, change, true), fmt.Errorf("removing moved source %s: %w", change.from.relative, err)
}
}
return true, nil
return structuredPatchChangeOutcome{completed: []structuredPatchChange{change}}, nil
}
return false, fmt.Errorf("unsupported structured patch operation")
return structuredPatchChangeOutcome{}, fmt.Errorf("unsupported structured patch operation")
}
func writeStructuredPatchFile(root *os.Root, target structuredPatchTarget, content string, mode os.FileMode, createOnly bool) (bool, error) {
func recheckStructuredPatchPreimage(root *os.Root, change structuredPatchChange) error {
current, info, err := readRootedFile(root, change.from.relative)
if err != nil {
return fmt.Errorf("re-reading %s before commit: %w", change.from.relative, err)
}
if (change.beforeInfo != nil && !os.SameFile(change.beforeInfo, info)) || string(current) != change.before {
return fmt.Errorf("%s changed on disk between planning and commit; re-read it and retry", change.from.relative)
}
return nil
}
func structuredPatchPrePublishGuard(root *os.Root, change structuredPatchChange) func() error {
return func() error {
if change.kind == structuredPatchUpdate && change.from.absolute == change.to.absolute && structuredPatchBeforeRename != nil {
structuredPatchBeforeRename(change)
}
return recheckStructuredPatchPreimage(root, change)
}
}
func writeStructuredPatchFile(root *os.Root, target structuredPatchTarget, content string, mode os.FileMode, createOnly bool, beforePublish func() error) (bool, error) {
parent := filepath.Dir(target.relative)
if err := root.MkdirAll(parent, 0o755); err != nil {
return false, fmt.Errorf("creating parent directory for %s: %w", target.relative, err)
@@ -785,8 +965,13 @@ func writeStructuredPatchFile(root *os.Root, target structuredPatchTarget, conte
if err := temp.Close(); err != nil {
return false, fmt.Errorf("writing %s: %w", target.relative, err)
}
if beforePublish != nil {
if err := beforePublish(); err != nil {
return false, err
}
}
if createOnly {
return publishStructuredPatchNoReplace(root, tempName, target.relative, mode)
return structuredPatchPublishNoReplace(root, tempName, target.relative, mode)
}
if err := root.Rename(tempName, target.relative); err != nil {
return false, fmt.Errorf("writing %s: %w", target.relative, err)
@@ -798,6 +983,11 @@ func publishStructuredPatchNoReplace(root *os.Root, source, target string, mode
return publishStructuredPatchNoReplaceWith(root, source, target, mode, root.Link)
}
// structuredPatchPublishNoReplace is a deterministic test seam for failures
// after a create-only target has become visible. Production uses the atomic
// no-replace publisher above.
var structuredPatchPublishNoReplace = publishStructuredPatchNoReplace
func publishStructuredPatchNoReplaceWith(root *os.Root, source, target string, mode os.FileMode, link func(string, string) error) (bool, error) {
if linkErr := link(source, target); linkErr == nil {
cleanupErr := removeStructuredPatchTemp(root, source)
@@ -914,9 +1104,19 @@ func changedFilesFromStructuredPatch(relativeRoot string, changes []structuredPa
seen := make(map[string]bool)
var paths []string
for _, change := range changes {
targets := []structuredPatchTarget{change.to}
if change.from.absolute != change.to.absolute {
targets = append([]structuredPatchTarget{change.from}, targets...)
var targets []structuredPatchTarget
switch change.kind {
case structuredPatchDelete:
targets = []structuredPatchTarget{change.from}
case structuredPatchUpdate:
targets = []structuredPatchTarget{change.to}
if change.from.absolute != change.to.absolute {
targets = append([]structuredPatchTarget{change.from}, targets...)
}
default:
// Adds and copies mutate only their destination; the copy source is
// evidence for the operation, not a changed file.
targets = []structuredPatchTarget{change.to}
}
for _, target := range targets {
path := target.relative
+4
View File
@@ -120,6 +120,10 @@ type Result struct {
// entries under a granted extra write root are absolute, since
// workspace-relative would be ambiguous there.
ChangedFiles []string
// FileDiffs carries exact, bounded before/after text for built-in file
// mutations. A missing entry means the client must fall back to ChangedFiles
// rather than inventing a partial diff.
FileDiffs []FileDiff
// ChangeSummaries contains bounded generated-tree changes. These are shown
// in session evidence and the Files panel but are never treated as files to
// open or diagnose individually.
+47 -10
View File
@@ -12,6 +12,7 @@ type writeFileTool struct {
baseTool
workspaceRoot string
scope PathScope
readFile func(string) ([]byte, error)
}
func NewScopedWriteFileTool(workspaceRoot string, scope PathScope) Tool {
@@ -34,6 +35,7 @@ func NewScopedWriteFileTool(workspaceRoot string, scope PathScope) Tool {
},
workspaceRoot: normalizeWorkspaceRoot(workspaceRoot),
scope: scope,
readFile: os.ReadFile,
}
}
@@ -61,8 +63,10 @@ func (tool writeFileTool) RunWithOptions(ctx context.Context, args map[string]an
}
existed := false
if _, err := os.Stat(absolutePath); err == nil {
var priorInfo os.FileInfo
if info, err := os.Stat(absolutePath); err == nil {
existed = true
priorInfo = info
if !overwrite {
return errorResult("Error: " + relativePath + " already exists. Pass overwrite: true to replace it.")
}
@@ -82,7 +86,7 @@ func (tool writeFileTool) RunWithOptions(ctx context.Context, args map[string]an
// Fail CLOSED: if the tracked file can't be re-read to verify it, refuse
// the overwrite rather than clobbering a file whose current state is
// unknown (it may have been replaced or removed out from under us).
current, rerr := os.ReadFile(absolutePath)
current, rerr := tool.readFile(absolutePath)
if rerr != nil {
return errorResult(fileConflictMessage(relativePath))
}
@@ -95,9 +99,11 @@ func (tool writeFileTool) RunWithOptions(ctx context.Context, args map[string]an
// Capture the prior content (before we replace it) so an overwrite can show a
// real diff; a fresh create stays "" and previews as all-additions.
priorContent := ""
priorContentKnown := !existed
if existed {
if prev, rerr := os.ReadFile(absolutePath); rerr == nil {
if prev, rerr := tool.readFile(absolutePath); rerr == nil {
priorContent = string(prev)
priorContentKnown = true
}
}
@@ -107,20 +113,36 @@ func (tool writeFileTool) RunWithOptions(ctx context.Context, args map[string]an
if err := recheckScopedWriteTarget(tool.workspaceRoot, tool.scope, requestedPath); err != nil {
return errorResult("Error writing file " + relativePath + ": " + err.Error())
}
if err := os.WriteFile(absolutePath, []byte(content), 0o644); err != nil {
var expectedContent *string
if priorContentKnown {
expectedContent = &priorContent
}
if err := commitFileContents(absolutePath, priorInfo, expectedContent, content); err != nil {
return errorResult("Error writing file " + relativePath + ": " + err.Error())
}
modelKnownContent := content
// Optional format-on-write (ZERO_FORMAT_ON_WRITE). Must run BEFORE the
// FileTracker baseline: recording pre-format content would make the very
// next edit look like an external modification and trip the conflict guard.
formatting := maybeFormatWrittenFile(ctx, absolutePath, content)
restoreMode := os.FileMode(0o644)
if priorInfo != nil {
restoreMode = priorInfo.Mode().Perm()
}
formatting := maybeFormatWrittenFileScoped(ctx, tool.workspaceRoot, tool.scope, absolutePath, content, restoreMode)
content = formatting.Content
finalContentKnown := formatting.ContentKnown
// Baseline the freshly written content so a later edit/overwrite in this
// session compares against what is now on disk.
newInfo, _ := os.Stat(absolutePath)
options.FileTracker.Record(absolutePath, []byte(content), newInfo)
if content == modelKnownContent {
newInfo := formatting.Info
if newInfo == nil {
newInfo, _ = os.Stat(absolutePath)
}
if finalContentKnown {
options.FileTracker.Record(absolutePath, []byte(content), newInfo)
} else {
options.FileTracker.Forget(absolutePath)
}
if finalContentKnown && content == modelKnownContent {
options.FileTracker.RecordSeenRange(absolutePath, 1, trackedLineTotal(content), trackedLineTotal(content))
}
if !existed {
@@ -139,13 +161,28 @@ func (tool writeFileTool) RunWithOptions(ctx context.Context, args map[string]an
}
summary := fmt.Sprintf("%s %s (%d lines).", verb, relativePath, lines)
summary += formatting.notice(relativePath)
summary += inlineDiagnostics(ctx, options, absolutePath, relativePath)
if finalContentKnown {
summary += inlineDiagnostics(ctx, options, absolutePath, relativePath)
}
result := okResult(summary)
result.ChangedFiles = []string{relativePath}
// Do not pretend an unreadable overwrite was a creation. The write may be
// valid, but ACP only receives an exact before/after pair we actually saw.
if priorContentKnown && finalContentKnown {
if diff, ok := boundedFileDiff(absolutePath, priorContent, content, existed, true); ok {
result.FileDiffs = []FileDiff{diff}
} else if diffTextRevealsObfuscatedSecret(priorContent) || diffTextRevealsObfuscatedSecret(content) {
result.Redacted = true
}
}
// Card-only preview: a real unified diff (all-green for a create, red/green for
// an overwrite) on Display.Preview. Output stays the summary, so the model never
// re-reads the file — the rich preview costs zero model tokens.
result.Display = Display{Summary: summary, Kind: "file", Preview: boundedUnifiedDiff(relativePath, priorContent, content)}
preview := ""
if priorContentKnown && finalContentKnown {
preview = boundedUnifiedDiff(relativePath, priorContent, content)
}
result.Display = Display{Summary: summary, Kind: "file", Preview: preview}
return result
}
+151 -5
View File
@@ -6,6 +6,8 @@ import (
"io"
"os"
"path/filepath"
"reflect"
"slices"
"strings"
"testing"
)
@@ -323,6 +325,10 @@ func TestEditFileToolReplacesExactStrings(t *testing.T) {
root := t.TempDir()
path := filepath.Join(root, "code.go")
writeTestFile(t, path, "const a = 1\nconst b = 2\n")
path, err := filepath.EvalSymlinks(path)
if err != nil {
t.Fatal(err)
}
result := NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{
"path": "code.go",
@@ -367,7 +373,12 @@ func TestEditFileToolReplacesCRLF(t *testing.T) {
func TestEditFileToolEmitsUnifiedDiff(t *testing.T) {
root := t.TempDir()
writeTestFile(t, filepath.Join(root, "code.go"), "const a = 1\nconst b = 2\n")
path := filepath.Join(root, "code.go")
writeTestFile(t, path, "const a = 1\nconst b = 2\n")
path, err := filepath.EvalSymlinks(path)
if err != nil {
t.Fatal(err)
}
res := NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{
"path": "code.go", "old_string": "const a = 1", "new_string": "const a = 42",
})
@@ -387,6 +398,9 @@ func TestEditFileToolEmitsUnifiedDiff(t *testing.T) {
t.Fatalf("edit preview missing diff marker %q: %q", want, res.Display.Preview)
}
}
if got := res.FileDiffs; len(got) != 1 || got[0].Path != path || !got[0].OldExists || !got[0].NewExists || got[0].OldText != "const a = 1\nconst b = 2\n" || got[0].NewText != "const a = 42\nconst b = 2\n" {
t.Fatalf("file diffs = %#v", got)
}
}
func TestWriteFileToolEmitsAdditionsDiff(t *testing.T) {
@@ -408,6 +422,13 @@ func TestWriteFileToolEmitsAdditionsDiff(t *testing.T) {
if strings.Contains(res.Display.Preview, "\n-line") {
t.Fatalf("a fresh-create diff must have no removed lines: %q", res.Display.Preview)
}
path, err := filepath.EvalSymlinks(filepath.Join(root, "new.txt"))
if err != nil {
t.Fatal(err)
}
if got := res.FileDiffs; len(got) != 1 || got[0].Path != path || got[0].OldExists || !got[0].NewExists || got[0].OldText != "" || got[0].NewText != "line one\nline two\n" {
t.Fatalf("file diffs = %#v", got)
}
}
func TestWriteFileToolOverwriteEmitsRedGreenDiff(t *testing.T) {
@@ -429,6 +450,28 @@ func TestWriteFileToolOverwriteEmitsRedGreenDiff(t *testing.T) {
}
}
func TestWriteFileToolOmitsDiffWhenOverwritePreimageCannotBeRead(t *testing.T) {
root := t.TempDir()
path := filepath.Join(root, "private.txt")
writeTestFile(t, path, "before\n")
tool := NewScopedWriteFileTool(root, nil).(writeFileTool)
tool.readFile = func(string) ([]byte, error) { return nil, os.ErrPermission }
registry := NewRegistry()
registry.Register(tool)
result := registry.RunWithOptions(context.Background(), tool.Name(), map[string]any{
"path": "private.txt", "content": "after\n", "overwrite": true,
}, RunOptions{PermissionGranted: true})
if result.Status != StatusOK {
t.Fatalf("write = %s", result.Output)
}
if len(result.FileDiffs) != 0 {
t.Fatalf("unreadable preimage must not produce a create-like diff: %#v", result.FileDiffs)
}
if got, err := os.ReadFile(path); err != nil || string(got) != "after\n" {
t.Fatalf("written content = %q, err = %v", got, err)
}
}
func TestEditFileToolAllowsDeletingRegions(t *testing.T) {
root := t.TempDir()
path := filepath.Join(root, "notes.txt")
@@ -584,6 +627,17 @@ func TestApplyPatchToolAppliesStructuredAddAndMove(t *testing.T) {
if got := result.ChangedFiles; strings.Join(got, ",") != "nested/new.txt,old.txt,moved.txt" {
t.Fatalf("ChangedFiles = %v", got)
}
resolvedRoot, err := filepath.EvalSymlinks(root)
if err != nil {
t.Fatal(err)
}
if got, want := result.FileDiffs, []FileDiff{
{Path: filepath.Join(resolvedRoot, "nested", "new.txt"), OldExists: false, NewExists: true, OldText: "", NewText: "created\n"},
{Path: filepath.Join(resolvedRoot, "old.txt"), OldExists: true, NewExists: false, OldText: "old\n", NewText: ""},
{Path: filepath.Join(resolvedRoot, "moved.txt"), OldExists: false, NewExists: true, OldText: "", NewText: "moved\n"},
}; !reflect.DeepEqual(got, want) {
t.Fatalf("FileDiffs = %#v, want %#v", got, want)
}
}
func TestApplyPatchToolStructuredPatchMatchesWhitespaceTolerantly(t *testing.T) {
@@ -831,7 +885,7 @@ func TestStructuredPatchAddDoesNotOverwriteRacedDestination(t *testing.T) {
after: "patch content\n", mode: 0o644,
}
err = applyStructuredPatchChanges(workspace, []structuredPatchChange{change}, nil)
_, err = applyStructuredPatchChanges(workspace, ".", []structuredPatchChange{change}, nil)
if err == nil || !errors.Is(err, os.ErrExist) {
t.Fatalf("raced add destination = %v, want os.ErrExist", err)
}
@@ -857,7 +911,7 @@ func TestStructuredPatchFailedDeleteDoesNotRecreateMissingFile(t *testing.T) {
before: "removed by another writer\n", mode: 0o644,
}
err = applyStructuredPatchChanges(workspace, []structuredPatchChange{change}, nil)
_, err = applyStructuredPatchChanges(workspace, ".", []structuredPatchChange{change}, nil)
if err == nil {
t.Fatal("delete of an already removed file should fail")
}
@@ -894,7 +948,7 @@ func TestStructuredPatchMoveWithMissingSourceIsRefusedBeforePublishing(t *testin
}
defer func() { structuredPatchBeforeCommit = nil }()
err = applyStructuredPatchChanges(workspace, []structuredPatchChange{change}, nil)
_, err = applyStructuredPatchChanges(workspace, ".", []structuredPatchChange{change}, nil)
if !removed {
t.Fatal("pre-commit hook did not run")
}
@@ -909,6 +963,59 @@ func TestStructuredPatchMoveWithMissingSourceIsRefusedBeforePublishing(t *testin
}
}
func TestStructuredPatchUpdatePreservesCompetingWriteBeforeRename(t *testing.T) {
for _, tc := range []struct {
name string
competing string
replaceIdentity bool
}{
{name: "content changed", competing: "competing writer\n"},
{name: "identity changed with same content", competing: "planned\n", replaceIdentity: true},
} {
t.Run(tc.name, func(t *testing.T) {
root := t.TempDir()
targetPath := filepath.Join(root, "file.txt")
writeTestFile(t, targetPath, "planned\n")
patch := "*** Begin Patch\n*** Update File: file.txt\n@@\n-planned\n+patched\n*** End Patch\n"
hookRan := false
var competingInfo os.FileInfo
structuredPatchBeforeRename = func(change structuredPatchChange) {
if tc.replaceIdentity {
tempPath := filepath.Join(root, "competing.tmp")
writeTestFile(t, tempPath, tc.competing)
if err := os.Rename(tempPath, targetPath); err != nil {
t.Fatal(err)
}
} else if err := os.WriteFile(targetPath, []byte(tc.competing), 0o644); err != nil {
t.Fatal(err)
}
var err error
competingInfo, err = os.Stat(targetPath)
if err != nil {
t.Fatal(err)
}
hookRan = true
}
t.Cleanup(func() { structuredPatchBeforeRename = nil })
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch})
if !hookRan {
t.Fatal("pre-rename hook did not run")
}
if result.Status != StatusError || len(result.ChangedFiles) != 0 || len(result.FileDiffs) != 0 {
t.Fatalf("raced update = status=%s changed=%#v diffs=%#v output=%q", result.Status, result.ChangedFiles, result.FileDiffs, result.Output)
}
if got := mustReadTestFile(t, targetPath); got != tc.competing {
t.Fatalf("raced update overwrote competing content: %q", got)
}
finalInfo, err := os.Stat(targetPath)
if err != nil || !os.SameFile(competingInfo, finalInfo) {
t.Fatalf("raced update replaced the competing file identity: %v", err)
}
})
}
}
func TestStructuredPatchCreateOnlyFallsBackWhenHardLinksUnsupported(t *testing.T) {
root := t.TempDir()
workspace, err := os.OpenRoot(root)
@@ -978,6 +1085,45 @@ func TestStructuredPatchCopyFailureReportsSurvivingPartialTarget(t *testing.T) {
}
}
func TestIncompleteStructuredPatchNoReplaceWriteReportsOnlyVerifiedTarget(t *testing.T) {
for name, content := range map[string]string{
"complete publication": "complete content\n",
"partial publication": "partial",
} {
t.Run(name, func(t *testing.T) {
root := t.TempDir()
workspace, err := os.OpenRoot(root)
if err != nil {
t.Fatal(err)
}
defer workspace.Close()
targetPath := filepath.Join(root, "target.txt")
writeTestFile(t, targetPath, content)
change := structuredPatchChange{
kind: structuredPatchAdd,
to: structuredPatchTarget{
requested: "target.txt",
relative: "target.txt",
absolute: targetPath,
},
after: "complete content\n",
mode: 0o644,
}
outcome := incompleteStructuredPatchWrite(workspace, change, true)
if content == change.after {
if len(outcome.completed) != 1 || outcome.completed[0].kind != structuredPatchAdd || len(outcome.incompletePaths) != 0 {
t.Fatalf("verified publication outcome = %#v", outcome)
}
return
}
if len(outcome.completed) != 0 || !slices.Equal(outcome.incompletePaths, []string{"target.txt"}) {
t.Fatalf("partial publication outcome = %#v", outcome)
}
})
}
}
func TestStructuredPatchPartialFailureLeavesCompletedChangeAndClearsTrackedState(t *testing.T) {
root := t.TempDir()
trackedPath := filepath.Join(root, "tracked.txt")
@@ -1017,7 +1163,7 @@ func TestStructuredPatchPartialFailureLeavesCompletedChangeAndClearsTrackedState
},
}
err = applyStructuredPatchChanges(workspace, changes, tracker)
_, err = applyStructuredPatchChanges(workspace, ".", changes, tracker)
if err == nil || !strings.Contains(err.Error(), "partially applied") {
t.Fatalf("second change = %v, want partial-application error", err)
}