diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7eb358b8..104ede8b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,7 +45,7 @@ jobs: - name: gofumpt run: | go install mvdan.cc/gofumpt@v0.10.0 - out=$(gofumpt -l .) + out=$(git ls-files -- '*.go' ':!external/**' | xargs gofumpt -l) if [ -n "$out" ]; then echo "::error::gofumpt would reformat:" echo "$out" | head -20 @@ -54,7 +54,7 @@ jobs: - name: goimports run: | go install golang.org/x/tools/cmd/goimports@v0.30.0 - out=$(goimports -l .) + out=$(git ls-files -- '*.go' ':!external/**' | xargs goimports -l) if [ -n "$out" ]; then echo "::error::goimports would reformat:" echo "$out" | head -20 diff --git a/.github/workflows/daemon-image.yml b/.github/workflows/daemon-image.yml new file mode 100644 index 00000000..6ac85820 --- /dev/null +++ b/.github/workflows/daemon-image.yml @@ -0,0 +1,116 @@ +name: Daemon image + +on: + push: + branches: [main] + tags: ["v*"] + pull_request: + branches: [main] + paths: + - "Dockerfile.daemon" + - "packaging/systemd/hawk-daemon.service" + - "internal/**" + - "cmd/**" + - "external/**" + - "go.mod" + - "go.sum" + +permissions: + contents: read + packages: write + security-events: write + +env: + REGISTRY: ghcr.io + IMAGE_NAME: graycodeai/hawk-daemon + +jobs: + build: + name: build + scan + publish (daemon) + runs-on: ubuntu-latest + steps: + - name: Check out source + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + submodules: recursive + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@d7f5e7f509e45cec5c76c4d5afdd7de93d0b3df5 # v4.1.0 + + - name: Log in to GHCR + if: github.event_name != 'pull_request' + uses: docker/login-action@650006c6eb7dba73a995cc03b0b2d7f5ca915bee # v4.2.0 + with: + registry: ${{ env.REGISTRY }} + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + - name: Build daemon image for scan + uses: docker/build-push-action@f9f3042f7e2789586610d6e8b85c8f03e5195baf # v7.2.0 + with: + context: . + file: Dockerfile.daemon + platforms: linux/amd64 + push: false + load: true + tags: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:scan + cache-from: type=gha,scope=hawk-daemon + cache-to: type=gha,mode=max,scope=hawk-daemon + build-args: | + VERSION=${{ github.ref_name }} + COMMIT=${{ github.sha }} + BUILD_DATE=${{ github.event.head_commit.timestamp }} + + - name: Scan daemon image with Trivy + uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0 + with: + image-ref: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:scan + format: sarif + output: trivy-daemon-image.sarif + severity: CRITICAL,HIGH + # Go reachability is enforced separately by govulncheck in CI. The + # binary also carries the full workspace module graph, including + # non-reachable packages that Trivy reports as binary findings. + vuln-type: os + ignore-unfixed: true + exit-code: '1' + + - name: Generate image metadata + id: meta + uses: docker/metadata-action@80c7e94dd9b9319bd5eb7a0e0fe9291e23a2a2e9 # v6.1.0 + with: + images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} + tags: | + type=raw,value=latest,enable={{is_default_branch}} + type=ref,event=branch + type=semver,pattern={{version}} + type=semver,pattern={{major}}.{{minor}} + type=sha,prefix=sha-,format=long + + # The PR already exercised the daemon Dockerfile in the scan build above. + # Skip the redundant multi-arch publish build on pull requests so CI can + # finish as soon as the security gate passes. + - name: Build and publish daemon image + if: github.event_name != 'pull_request' + uses: docker/build-push-action@f9f3042f7e2789586610d6e8b85c8f03e5195baf # v7.2.0 + with: + context: . + file: Dockerfile.daemon + platforms: linux/amd64,linux/arm64 + push: ${{ github.event_name != 'pull_request' }} + tags: ${{ steps.meta.outputs.tags }} + labels: ${{ steps.meta.outputs.labels }} + cache-from: type=gha,scope=hawk-daemon + cache-to: type=gha,mode=max,scope=hawk-daemon + build-args: | + VERSION=${{ github.ref_name }} + COMMIT=${{ github.sha }} + BUILD_DATE=${{ github.event.head_commit.timestamp }} + + # Publish the daemon scan to GitHub code scanning. The PR still runs the + # scan, but it skips the redundant publish build and release artifacts. + - name: Upload daemon image scan results + if: always() + uses: github/codeql-action/upload-sarif@8aad20d150bbac5944a9f9d289da16a4b0d87c1e # v4 + with: + sarif_file: trivy-daemon-image.sarif diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index bb784149..b9478173 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -76,9 +76,13 @@ jobs: image-ref: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:scan format: sarif output: trivy-image.sarif - severity: CRITICAL + severity: CRITICAL,HIGH + # Go reachability is enforced separately by govulncheck in CI. The + # binary also carries the full workspace module graph, including + # non-reachable packages that Trivy reports as binary findings. + vuln-type: os ignore-unfixed: true - exit-code: '0' # Don't fail the build; results are uploaded for review + exit-code: '1' # Block publishing images with actionable vulnerabilities # Second build is a cache hit (layers exported by the scan build), so it # only re-links and pushes the platform image. @@ -96,8 +100,10 @@ jobs: COMMIT=${{ github.sha }} BUILD_DATE=${{ github.event.head_commit.timestamp }} + # Publish the scan results to GitHub code scanning. The scan itself runs + # on PRs; only the release-side publish path stays off PRs. - name: Upload Trivy image scan results - if: github.event_name != 'pull_request' && always() + if: always() uses: github/codeql-action/upload-sarif@8aad20d150bbac5944a9f9d289da16a4b0d87c1e # v4 with: sarif_file: trivy-image.sarif @@ -148,9 +154,13 @@ jobs: image-ref: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:scan format: sarif output: trivy-image.sarif - severity: CRITICAL + severity: CRITICAL,HIGH + # Go reachability is enforced separately by govulncheck in CI. The + # binary also carries the full workspace module graph, including + # non-reachable packages that Trivy reports as binary findings. + vuln-type: os ignore-unfixed: true - exit-code: '0' # Don't fail the build; results are uploaded for review + exit-code: '1' # Block publishing images with actionable vulnerabilities # Second build is a cache hit (layers exported by the scan build), so it # only re-links and pushes the platform image. @@ -168,14 +178,16 @@ jobs: COMMIT=${{ github.sha }} BUILD_DATE=${{ github.event.head_commit.timestamp }} + # Publish the scan results to GitHub code scanning. The scan itself runs + # on PRs; only the release-side publish path stays off PRs. - name: Upload Trivy image scan results - if: github.event_name != 'pull_request' && always() + if: always() uses: github/codeql-action/upload-sarif@8aad20d150bbac5944a9f9d289da16a4b0d87c1e # v4 with: sarif_file: trivy-image.sarif merge-manifest: - name: merge multi-arch manifest + name: merge multi-arch manifest (release only) if: github.event_name != 'pull_request' needs: [build-amd64, build-arm64] runs-on: ubuntu-latest @@ -255,9 +267,13 @@ jobs: image-ref: ${{ env.REGISTRY }}/graycodeai/hawk-sandbox:scan format: sarif output: trivy-sandbox-image.sarif - severity: CRITICAL + severity: CRITICAL,HIGH + # This image gate covers the Debian OS package surface. npm's + # bundled CLI dependency tree is pinned by the Node base image and + # reviewed separately from the runtime OS scan. + vuln-type: os ignore-unfixed: true - exit-code: '0' + exit-code: '1' - name: Build and publish public sandbox image uses: docker/build-push-action@f9f3042f7e2789586610d6e8b85c8f03e5195baf # v7.2.0 @@ -272,8 +288,10 @@ jobs: cache-from: type=gha,scope=hawk-sandbox cache-to: type=gha,mode=max,scope=hawk-sandbox + # Publish the sandbox scan to GitHub code scanning. PRs still run the + # scan; they just do not publish the release image artifacts. - name: Upload sandbox image scan results - if: github.event_name != 'pull_request' && always() + if: always() uses: github/codeql-action/upload-sarif@8aad20d150bbac5944a9f9d289da16a4b0d87c1e # v4 with: sarif_file: trivy-sandbox-image.sarif diff --git a/AGENTS.md b/AGENTS.md index 68a0c8a3..1f88069d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -201,7 +201,7 @@ with its native Responses API (`/v1/responses`) under the ## GitNexus — Code Intelligence -This project is indexed by GitNexus as **hawk** (86470 symbols, 279855 relationships, 300 execution flows). Use the GitNexus MCP tools to understand code, assess impact, and navigate safely. +This project is indexed by GitNexus as **hawk** (97743 symbols, 322940 relationships, 300 execution flows). Use the GitNexus MCP tools to understand code, assess impact, and navigate safely. > Index stale? Run `node .gitnexus/run.cjs analyze` from the project root — it auto-selects an available runner. No `.gitnexus/run.cjs` yet? `npx gitnexus analyze` (npm 11 crash → `npm i -g gitnexus`; #1939). diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..4bd51dd5 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,44 @@ + +# GitNexus — Code Intelligence + +This project is indexed by GitNexus as **hawk** (97743 symbols, 322940 relationships, 300 execution flows). Use the GitNexus MCP tools to understand code, assess impact, and navigate safely. + +> Index stale? Run `node .gitnexus/run.cjs analyze` from the project root — it auto-selects an available runner. No `.gitnexus/run.cjs` yet? `npx gitnexus analyze` (npm 11 crash → `npm i -g gitnexus`; #1939). + +## Always Do + +- **MUST run impact analysis before editing any symbol.** Before modifying a function, class, or method, run `impact({target: "symbolName", direction: "upstream"})` and report the blast radius (direct callers, affected processes, risk level) to the user. +- **MUST run `detect_changes()` before committing** to verify your changes only affect expected symbols and execution flows. For regression review, compare against the default branch: `detect_changes({scope: "compare", base_ref: "main"})`. +- **MUST warn the user** if impact analysis returns HIGH or CRITICAL risk before proceeding with edits. +- When exploring unfamiliar code, use `query({search_query: "concept"})` to find execution flows instead of grepping. It returns process-grouped results ranked by relevance. +- When you need full context on a specific symbol — callers, callees, which execution flows it participates in — use `context({name: "symbolName"})`. +- For security review, `explain({target: "fileOrSymbol"})` lists taint findings (source→sink flows; needs `analyze --pdg`). + +## Never Do + +- NEVER edit a function, class, or method without first running `impact` on it. +- NEVER ignore HIGH or CRITICAL risk warnings from impact analysis. +- NEVER rename symbols with find-and-replace — use `rename` which understands the call graph. +- NEVER commit changes without running `detect_changes()` to check affected scope. + +## Resources + +| Resource | Use for | +|----------|---------| +| `gitnexus://repo/hawk/context` | Codebase overview, check index freshness | +| `gitnexus://repo/hawk/clusters` | All functional areas | +| `gitnexus://repo/hawk/processes` | All execution flows | +| `gitnexus://repo/hawk/process/{name}` | Step-by-step execution trace | + +## CLI + +| Task | Read this skill file | +|------|---------------------| +| Understand architecture / "How does X work?" | `.claude/skills/gitnexus/gitnexus-exploring/SKILL.md` | +| Blast radius / "What breaks if I change X?" | `.claude/skills/gitnexus/gitnexus-impact-analysis/SKILL.md` | +| Trace bugs / "Why is X failing?" | `.claude/skills/gitnexus/gitnexus-debugging/SKILL.md` | +| Rename / extract / split / refactor | `.claude/skills/gitnexus/gitnexus-refactoring/SKILL.md` | +| Tools, resources, schema reference | `.claude/skills/gitnexus/gitnexus-guide/SKILL.md` | +| Index, status, clean, wiki CLI commands | `.claude/skills/gitnexus/gitnexus-cli/SKILL.md` | + + diff --git a/Dockerfile b/Dockerfile index 6b26eaeb..e25e96fa 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,8 +1,7 @@ # Build stage -# TODO(supply-chain): pin base images by digest (tag@sha256:…) — tags are -# mutable and an upstream re-push silently changes the build. Applies to the -# alpine runtime stage below as well. -FROM golang:1.26.5-alpine AS builder +# Supply-chain hardening: both stages are pinned by digest so a mutable tag +# cannot silently change the build. +FROM golang:1.26.5-alpine@sha256:0178a641fbb4858c5f1b48e34bdaabe0350a330a1b1149aabd498d0699ff5fb2 AS builder RUN apk upgrade --no-cache && \ apk add --no-cache git ca-certificates tzdata @@ -56,7 +55,7 @@ RUN --mount=type=cache,target=/go/pkg/mod \ -o hawk ./cmd/hawk # Runtime stage — Alpine (hawk requires git + bash for workspace operations; distroless excluded) -FROM alpine:3.23.5 +FROM alpine:3.23.5@sha256:fd791d74b68913cbb027c6546007b3f0d3bc45125f797758156952bc2d6daf40 RUN apk upgrade --no-cache && \ apk add --no-cache ca-certificates git bash tini && \ diff --git a/Dockerfile.daemon b/Dockerfile.daemon index dab95701..8d0dc2ea 100644 --- a/Dockerfile.daemon +++ b/Dockerfile.daemon @@ -4,7 +4,7 @@ # # Build: docker build -f Dockerfile.daemon -t hawk-daemon . # Run: docker run -p 4590:4590 -e HAWK_DAEMON_API_KEY=... hawk-daemon -FROM golang:1.26.5-alpine AS builder +FROM golang:1.26.5-alpine@sha256:0178a641fbb4858c5f1b48e34bdaabe0350a330a1b1149aabd498d0699ff5fb2 AS builder RUN apk upgrade --no-cache && \ apk add --no-cache git ca-certificates tzdata @@ -35,7 +35,7 @@ RUN --mount=type=cache,target=/go/pkg/mod \ -X main.BuildDate=${BUILD_DATE}" \ -o hawk ./cmd/hawk -FROM alpine:3.23.5 +FROM alpine:3.23.5@sha256:fd791d74b68913cbb027c6546007b3f0d3bc45125f797758156952bc2d6daf40 RUN apk upgrade --no-cache && \ apk add --no-cache ca-certificates git bash curl tini && \ @@ -56,7 +56,7 @@ EXPOSE 4590 # Health check probes the daemon's health endpoint. HEALTHCHECK --interval=30s --timeout=5s --start-period=10s --retries=3 \ - CMD curl -sf http://127.0.0.1:4590/v1/health || exit 1 + CMD curl -ksf https://127.0.0.1:4590/v1/health || curl -sf http://127.0.0.1:4590/v1/health || exit 1 ENTRYPOINT ["tini", "--", "hawk", "daemon", "start"] CMD ["--host", "0.0.0.0", "--port", "4590"] diff --git a/Makefile b/Makefile index 7d7c07ac..d3d9b133 100644 --- a/Makefile +++ b/Makefile @@ -89,8 +89,8 @@ api-docs: ## Generate HTML API reference from OpenAPI spec. @echo "API reference generated: api/reference.html" api-validate: ## Validate the OpenAPI spec. - @command -v @redocly/cli >/dev/null 2>&1 || (echo "install: npm install -g @redocly/cli" && exit 1) - @redocly lint api/openapi.yaml + @command -v redocly >/dev/null 2>&1 || npm install -g @redocly/cli + redocly lint api/openapi.yaml bench: ## Run benchmarks. go test ./... -bench=. -benchmem -count=3 -timeout=300s @@ -101,8 +101,8 @@ bench: ## Run benchmarks. fmt: ## Format source files (gofumpt + goimports). @command -v $(GOFUMPT) >/dev/null 2>&1 || (echo "install: go install mvdan.cc/gofumpt@latest" && exit 1) @command -v $(GOIMPORTS) >/dev/null 2>&1 || (echo "install: go install golang.org/x/tools/cmd/goimports@latest" && exit 1) - $(GOFUMPT) -w . - $(GOIMPORTS) -w . + @git ls-files -- '*.go' ':!external/**' | xargs $(GOFUMPT) -w + @git ls-files -- '*.go' ':!external/**' | xargs $(GOIMPORTS) -w vet: ## Run go vet. go vet ./... diff --git a/api/openapi.yaml b/api/openapi.yaml index 8f1188ea..4f81c14d 100644 --- a/api/openapi.yaml +++ b/api/openapi.yaml @@ -16,7 +16,7 @@ info: url: https://github.com/GrayCodeAI/hawk servers: - - url: http://localhost:4590 + - url: http://127.0.0.1:4590 description: Local daemon (default port) security: @@ -417,6 +417,7 @@ tags: paths: /v1/health: get: + operationId: healthCheck tags: [system] summary: Health check security: [] @@ -427,9 +428,16 @@ paths: application/json: schema: $ref: "#/components/schemas/HealthResponse" + "400": + description: Invalid request + content: + application/json: + schema: + $ref: "#/components/schemas/Error" /v1/ready: get: + operationId: readinessProbe tags: [system] summary: Readiness probe description: | @@ -444,6 +452,12 @@ paths: application/json: schema: $ref: "#/components/schemas/ReadyResponse" + "400": + description: Invalid request + content: + application/json: + schema: + $ref: "#/components/schemas/Error" "503": description: Daemon is not ready content: @@ -453,6 +467,7 @@ paths: /v1/chat: post: + operationId: sendChat tags: [agent] summary: Send a prompt to the agent description: | @@ -510,6 +525,7 @@ paths: /v1/cancel: post: + operationId: cancelGeneration tags: [agent] summary: Cancel an in-flight generation description: | @@ -563,6 +579,7 @@ paths: /v1/sessions: get: + operationId: listSessions tags: [sessions] summary: List active daemon sessions responses: @@ -583,6 +600,7 @@ paths: /v1/sessions/{id}: get: + operationId: getSession tags: [sessions] summary: Get a persisted session parameters: @@ -605,6 +623,7 @@ paths: schema: $ref: "#/components/schemas/Error" delete: + operationId: deleteSession tags: [sessions] summary: Delete a session parameters: @@ -631,6 +650,7 @@ paths: /v1/sessions/{id}/messages: get: + operationId: listSessionMessages tags: [messages] summary: Get session messages with pagination parameters: @@ -663,6 +683,7 @@ paths: /v1/sessions/{id}/graph: get: + operationId: getSessionGraph tags: [graphs] summary: Project a persisted session as a portable execution graph description: | @@ -729,6 +750,7 @@ paths: /v1/stats: get: + operationId: getStats tags: [stats] summary: Aggregated usage statistics responses: @@ -747,6 +769,7 @@ paths: /v1/metrics: get: + operationId: getMetrics tags: [stats] summary: Daemon metrics in Prometheus exposition format description: | @@ -772,6 +795,7 @@ paths: /v1/review: post: + operationId: createReview tags: [review] summary: Trigger an asynchronous code review of a commit requestBody: @@ -796,6 +820,7 @@ paths: /v1/review/status: get: + operationId: getReviewStatus tags: [review] summary: Get current review status responses: @@ -805,6 +830,12 @@ paths: application/json: schema: $ref: "#/components/schemas/ReviewStatusResponse" + "400": + description: Invalid request + content: + application/json: + schema: + $ref: "#/components/schemas/Error" "500": description: Status command failed content: diff --git a/cmd/chat_commands_session.go b/cmd/chat_commands_session.go index 94026148..bf0cc373 100644 --- a/cmd/chat_commands_session.go +++ b/cmd/chat_commands_session.go @@ -15,6 +15,12 @@ import ( "github.com/GrayCodeAI/hawk/internal/storage" ) +type sessionSaveResultMsg struct { + id string + seq uint64 + err error +} + // saveSession persists the current session to disk. func (m *chatModel) saveSession() { raw := m.session.RawMessages() @@ -27,8 +33,13 @@ func (m *chatModel) saveSession() { }) // On successful save, WAL is no longer needed (session file has everything) if err == nil && m.wal != nil { - _ = m.wal.Remove() - m.wal = nil + if removeErr := m.wal.Remove(); removeErr != nil { + m.recordWALError(removeErr) + } else { + m.wal = nil + } + } else if err != nil { + m.recordWALError(err) } } @@ -48,16 +59,13 @@ func (m *chatModel) saveSessionCmd() tea.Cmd { id, modelName, provider := m.sessionID, m.session.Model(), m.session.Provider() msgs := session.FromRuntimeMessages(raw) createdAt := time.Now() - wal := m.wal + seq := m.walSeq return func() tea.Msg { err := session.Save(&session.Session{ ID: id, Model: modelName, Provider: provider, Messages: msgs, CreatedAt: createdAt, }) - if err == nil && wal != nil { - _ = wal.Remove() - } - return nil + return sessionSaveResultMsg{id: id, seq: seq, err: err} } } diff --git a/cmd/chat_model.go b/cmd/chat_model.go index 73ac7f91..a63bf90f 100644 --- a/cmd/chat_model.go +++ b/cmd/chat_model.go @@ -36,6 +36,18 @@ type sessionWAL interface { Close() error } +func (m *chatModel) ensureWAL() { + if m == nil || m.wal != nil || m.sessionID == "" { + return + } + wal, err := session.NewWAL(m.sessionID) + if err != nil { + m.recordWALError(err) + return + } + m.wal = wal +} + // recordWALError captures the first persistence failure so the user can be // told their message may not survive a crash. Subsequent failures are dropped // (the first is surfaced once, in the status area). @@ -224,6 +236,7 @@ type chatModel struct { credentialTimeoutAt time.Time pendingYOLOConfirm bool // user selected YOLO in the picker; awaiting typed confirmation durabilityWarning string // first WAL persistence failure, surfaced once to the user + walSeq uint64 // increments for each append; guards async WAL rotation width int height int quitting bool diff --git a/cmd/chat_submit.go b/cmd/chat_submit.go index cc16a65f..6b04039f 100644 --- a/cmd/chat_submit.go +++ b/cmd/chat_submit.go @@ -148,7 +148,9 @@ func (m chatModel) submitUserMessage() (chatModel, tea.Cmd) { } else { m.session.AddUser(text) } + m.ensureWAL() if m.wal != nil { + m.walSeq++ m.recordWALError(m.wal.Append(session.Message{Role: "user", Content: text})) } m.turnSawThinking = false diff --git a/cmd/chat_tools.go b/cmd/chat_tools.go index 18f0aeaf..48647df3 100644 --- a/cmd/chat_tools.go +++ b/cmd/chat_tools.go @@ -90,10 +90,33 @@ func optionalTools() []tool.Tool { tool.SpecResetTool{}, tool.SpecConfigTool{}, tool.ClarifyTool{}, + // Advanced specification workflows are lazy-loaded here rather than + // left as compile-only tools. ToolSearch can now discover the complete + // spec surface without making these startup-critical. + tool.SpecAdaptiveTool{}, + tool.SpecAdrTool{}, tool.AnalyzeTool{}, + tool.SpecBddTool{}, + tool.SpecBlastTool{}, tool.ChecklistTool{}, tool.ConstitutionTool{}, tool.ConvergeTool{}, + tool.SpecDriftTool{}, + tool.SpecGroundTool{}, + tool.SpecLinksTool{}, + tool.SpecMasterTool{}, + tool.SpecParallelTool{}, + tool.SpecPlanVariationsTool{}, + tool.SpecProgressTool{}, + tool.SpecPropertiesTool{}, + tool.SpecProvenanceTool{}, + tool.SpecReviewTool{}, + tool.SpecScaleTool{}, + tool.SpecSuperTool{}, + tool.SpecTestFirstTool{}, + tool.SpecTestGenTool{}, + tool.SpecTraceTool{}, + tool.SpecVersionTool{}, tool.TasksToIssuesTool{}, tool.NotebookEditTool{}, tool.EnterWorktreeTool{}, diff --git a/cmd/chat_update.go b/cmd/chat_update.go index bdbe60cf..10ea470f 100644 --- a/cmd/chat_update.go +++ b/cmd/chat_update.go @@ -138,6 +138,22 @@ func (m chatModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } switch msg := msg.(type) { + case sessionSaveResultMsg: + if msg.err != nil { + m.recordWALError(msg.err) + return m, nil + } + // Remove the WAL only when no append happened after the snapshot. + // Doing this in the background save closure could race with a new + // submission and lose messages that were not in the saved session. + if msg.id == m.sessionID && msg.seq == m.walSeq && m.wal != nil { + if err := m.wal.Remove(); err != nil { + m.recordWALError(err) + } else { + m.wal = nil + } + } + return m, nil case tea.FocusMsg: m.viewDirty = true m.updateViewportContent() @@ -1272,7 +1288,9 @@ func (m chatModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // Durability: persist completed tool results incrementally so a // crash mid-turn doesn't lose them (they were previously only // written at turn end via saveSession). + m.ensureWAL() if m.wal != nil { + m.walSeq++ m.recordWALError(m.wal.Append(session.Message{Role: "tool_result", Content: msg.content})) } @@ -1417,7 +1435,9 @@ func (m chatModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if m.partial.Len() > 0 { content := sanitizeIdentity(m.partial.String()) m.messages = append(m.messages, displayMsg{role: "assistant", content: content}) + m.ensureWAL() if m.wal != nil { + m.walSeq++ m.recordWALError(m.wal.Append(session.Message{Role: "assistant", Content: content})) } // Generate ghost text suggestion from AI response diff --git a/cmd/exec.go b/cmd/exec.go index b6aec7f6..a01bad27 100644 --- a/cmd/exec.go +++ b/cmd/exec.go @@ -77,6 +77,7 @@ Supports piping from stdin, JSON output format, and autonomy levels. Use --ephemeral to skip session persistence (ideal for CI runs). Use --json for JSON output (alias for --output-format json). +Use --output-format stream-json for newline-delimited progress events. Autonomy Levels: supervised (default) Ask for permission on every tool call @@ -98,7 +99,7 @@ Examples: } func init() { - execCmd.Flags().StringVarP(&execOutputFormat, "output-format", "o", "text", "Output format: text or json") + execCmd.Flags().StringVarP(&execOutputFormat, "output-format", "o", "text", "Output format: text, json, or stream-json") execCmd.Flags().StringVar(&execAutoLevel, "auto", "", "Autonomy level: supervised|basic|semi|full|yolo") execCmd.Flags().StringVarP(&execModel, "model", "m", "", "Model ID to use") execCmd.Flags().IntVar(&execMaxTurns, "max-turns", 0, "Maximum agentic turns (0 = unlimited)") @@ -118,6 +119,9 @@ func runExec(_ *cobra.Command, args []string) error { if execJSON { execOutputFormat = "json" } + if execOutputFormat != "text" && execOutputFormat != "json" && execOutputFormat != "stream-json" { + return fmt.Errorf("--output-format must be one of: text, json, stream-json") + } prompt, err := resolveExecPrompt(args) if err != nil { @@ -291,7 +295,7 @@ func runExec(_ *cobra.Command, args []string) error { if execOutputFormat == "text" { fmt.Print(ev.Content) } - if execOutputFormat == "json" { + if execOutputFormat == "stream-json" { _ = jsonEnc.Encode(map[string]interface{}{ "type": "content", "content": ev.Content, @@ -308,21 +312,21 @@ func runExec(_ *cobra.Command, args []string) error { if execOutputFormat == "text" { _, _ = fmt.Fprintf(os.Stderr, "\nerror: %s\n", ev.Content) } - if execOutputFormat == "json" { + if execOutputFormat == "stream-json" { _ = jsonEnc.Encode(map[string]interface{}{ "type": "error", "content": ev.Content, }) } case "tool_use": - if execOutputFormat == "json" { + if execOutputFormat == "stream-json" { _ = jsonEnc.Encode(map[string]interface{}{ "type": "tool_use", "tool": ev.ToolName, }) } case "tool_result": - if execOutputFormat == "json" { + if execOutputFormat == "stream-json" { _ = jsonEnc.Encode(map[string]interface{}{ "type": "tool_result", "tool": ev.ToolName, @@ -330,7 +334,7 @@ func runExec(_ *cobra.Command, args []string) error { }) } case "done": - if execOutputFormat == "json" { + if execOutputFormat == "stream-json" { _ = jsonEnc.Encode(map[string]interface{}{ "type": "done", }) @@ -377,7 +381,7 @@ func runExec(_ *cobra.Command, args []string) error { return nil } - if execOutputFormat == "json" { + if execOutputFormat == "json" || execOutputFormat == "stream-json" { result := ExecResult{ SessionID: sessionID, Response: response.String(), @@ -390,13 +394,13 @@ func runExec(_ *cobra.Command, args []string) error { Worktree: wtPath, Branch: wtBranch, } - if execOutputFormat == "json" && execEphemeral { + if execOutputFormat == "stream-json" { _ = jsonEnc.Encode(map[string]interface{}{ "type": "result", "result": result, }) } - if !execEphemeral { + if execOutputFormat == "json" { enc := json.NewEncoder(os.Stdout) enc.SetIndent("", " ") return enc.Encode(result) diff --git a/cmd/root.go b/cmd/root.go index e836bf41..c0d6c4b3 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -815,9 +815,22 @@ func init() { // Execute runs the root command. func Execute() error { + // Cobra defaults command output to stderr when no writer is configured. + // The process entrypoint must make stdout/stderr semantics explicit so + // scripts can safely pipe data and diagnostics never corrupt structured + // output. + setCommandWriters(rootCmd) return rootCmd.Execute() } +func setCommandWriters(cmd *cobra.Command) { + cmd.SetOut(os.Stdout) + cmd.SetErr(os.Stderr) + for _, child := range cmd.Commands() { + setCommandWriters(child) + } +} + var recoverCmd = &cobra.Command{ Use: "recover [session-id]", Short: "Scan for interrupted sessions and resume", diff --git a/docs/operations-checklist.md b/docs/operations-checklist.md index 8eee170a..60c77346 100644 --- a/docs/operations-checklist.md +++ b/docs/operations-checklist.md @@ -12,13 +12,13 @@ documentation section. ```bash export HAWK_DAEMON_API_KEY=$(openssl rand -base64 32) ``` -- [ ] **Bind address** — Daemon binds to `0.0.0.0` (not just loopback) if - remote access is needed. If bound to non-loopback without TLS, the daemon - prints a warning and refuses to start without an API key. -- [ ] **TLS configured** — For production, either: - - Terminate TLS at a reverse proxy (nginx, Caddy, ALB) and set - `X-Forwarded-Proto: https`, **or** - - Enable native TLS with `--tls-cert` / `--tls-key` flags. +- [ ] **Bind address** — Daemon binds to `0.0.0.0` (not just loopback) only + when remote access is needed. A non-loopback bind requires an API key and + native TLS; otherwise startup fails closed. +- [ ] **TLS configured** — Enable native TLS with `--tls-cert` / `--tls-key` + flags. If a reverse proxy terminates TLS, keep Hawk bound to loopback or + an internal-only interface and restrict that network path at the firewall; + the daemon does not treat `X-Forwarded-Proto` as transport encryption. - [ ] **CORS configured** — If serving browser-based clients, set `--cors https://app.example.com` to allow cross-origin requests from trusted origins only. Use `--cors '*'` only for development. diff --git a/docs/troubleshooting-guide.md b/docs/troubleshooting-guide.md index 0b7b559e..ccddc737 100644 --- a/docs/troubleshooting-guide.md +++ b/docs/troubleshooting-guide.md @@ -350,14 +350,17 @@ large contexts can consume significant memory. Consider: ### Daemon exits immediately The default entrypoint runs `hawk daemon start --host 0.0.0.0 --port 4590`. -If no API key is set, the daemon will refuse to start on a non-loopback bind. +Non-loopback binds require both an API key and native TLS; an API key alone +does not protect credentials or conversation data from plaintext interception. **Fix:** ```bash docker run -p 4590:4590 \ -e HAWK_DAEMON_API_KEY=$(openssl rand -base64 32) \ - ghcr.io/graycodeai/hawk-daemon:latest + -v "$PWD/certs:/certs:ro" \ + ghcr.io/graycodeai/hawk-daemon:latest \ + --tls-cert /certs/server.crt --tls-key /certs/server.key ``` ### Health check fails in container diff --git a/docs/user-guide/01-getting-started.md b/docs/user-guide/01-getting-started.md index f261569c..26b37040 100644 --- a/docs/user-guide/01-getting-started.md +++ b/docs/user-guide/01-getting-started.md @@ -196,7 +196,7 @@ Output formats: |--------|------|-------------| | `plain` | (default) | Human-readable text | | `json` | `--output-format json` | Single JSON object with response | -| `streaming-json` | `--output-format streaming-json` | NDJSON event stream | +| `stream-json` | `--output-format stream-json` | NDJSON event stream | --- @@ -225,4 +225,4 @@ Deeper files take precedence. Hawk also reads `CLAUDE.md` files for compatibilit --- -© 2026 GrayCode AI. All rights reserved. \ No newline at end of file +© 2026 GrayCode AI. All rights reserved. diff --git a/docs/user-guide/14-headless-mode.md b/docs/user-guide/14-headless-mode.md index fbb91c7d..12ce192c 100644 --- a/docs/user-guide/14-headless-mode.md +++ b/docs/user-guide/14-headless-mode.md @@ -31,26 +31,26 @@ hawk -p "Summarize this codebase" Single JSON object after completion: ```bash -hawk -p "Summarize this codebase" --output-format json | jq -r '.text' +hawk -p "Summarize this codebase" --output-format json | jq -r '.response' ``` Output includes: -- `text` — Response content -- `stopReason` — Why the response ended -- `sessionId` — Session ID for resuming +- `response` — Response content +- `exit_code` — 0 for success, non-zero for a failed run +- `session_id` — Session ID for resuming -### streaming-json +### stream-json NDJSON events in real time: ```bash -hawk -p "Summarize" --output-format streaming-json | jq -r 'select(.type=="text") | .data' +hawk -p "Summarize" --output-format stream-json | jq -r 'select(.type=="content") | .content' ``` Event types: -- `text` — Response chunk -- `thought` — Reasoning (thinking tokens) -- `end` — Final event with metadata +- `content` — Response chunk +- `tool_use` / `tool_result` — Tool lifecycle events +- `done` — Final event with metadata - `error` — Error occurred --- @@ -96,11 +96,11 @@ hawk -p "Review" --disallowed-tools "Bash,WebSearch" Control tool permissions: ```bash -# Allow shell commands -hawk -p "Build" --allow "Bash(git*)" --allow "Bash(npm*)" +# Allow shell commands through the explicit tool policy flag +hawk -p "Build" --allowed-tools "Bash(git:*) Bash(npm:*)" # Deny dangerous commands -hawk -p "Clean" --deny "Bash(rm*)" --deny "Bash(sudo*)" +hawk -p "Clean" --disallowed-tools "Bash(rm:*) Bash(sudo:*)" ``` --- @@ -110,7 +110,7 @@ hawk -p "Clean" --deny "Bash(rm*)" --deny "Bash(sudo*)" Use `--auto` for fully automated runs: ```bash -hawk -p "Format all files" --auto +hawk -p "Format all files" --dangerously-skip-permissions hawk exec --auto full "Add error handling" ``` @@ -125,14 +125,14 @@ hawk exec --auto full "Add error handling" ```bash #!/bin/bash hawk -p "Review staged changes for bugs. Reply OK if fine." \ - --auto --output-format json | jq -r '.text' | grep -q "^OK" || exit 1 + --dangerously-skip-permissions --output-format json | jq -r '.response' | grep -q "^OK" || exit 1 ``` ### Code Review ```bash hawk -p "Review PR for security issues" \ - --output-format json --auto | jq -r '.text' > review.md + --output-format json --dangerously-skip-permissions | jq -r '.response' > review.md ``` ### Batch Processing @@ -175,4 +175,4 @@ export HAWK_LOG_FILE="/tmp/hawk.log" # Log file --- -© 2026 GrayCode AI. All rights reserved. \ No newline at end of file +© 2026 GrayCode AI. All rights reserved. diff --git a/internal/container/Dockerfile b/internal/container/Dockerfile index 397875ed..7575c25c 100644 --- a/internal/container/Dockerfile +++ b/internal/container/Dockerfile @@ -1,4 +1,4 @@ -FROM ubuntu:24.04 +FROM ubuntu:24.04@sha256:561618e2c15bf2397621dd04f96926663a3b5616c189cf7e38db7e82f5c538ea ENV DEBIAN_FRONTEND=noninteractive diff --git a/internal/daemon/daemon_test.go b/internal/daemon/daemon_test.go index 89b0fbe2..3dac3ca5 100644 --- a/internal/daemon/daemon_test.go +++ b/internal/daemon/daemon_test.go @@ -479,7 +479,6 @@ func TestDaemon_ChatRejectsUnsafeSessionIDAndInvalidCWD(t *testing.T) { } func TestDaemon_ChatSSEExposesRetrievableSessionID(t *testing.T) { - t.Skip("TODO: https://github.com/GrayCodeAI/hawk/issues/153") t.Setenv("HAWK_STATE_DIR", t.TempDir()) srv := New(Config{Port: 0, Host: testutil.LoopbackHost}, daemonTestSessionFactory(nil)) addr := startTestDaemon(t, srv) diff --git a/internal/daemon/middleware.go b/internal/daemon/middleware.go index 212074b2..a69d7c7b 100644 --- a/internal/daemon/middleware.go +++ b/internal/daemon/middleware.go @@ -136,18 +136,38 @@ func (s *Server) corsMiddleware(next http.Handler) http.Handler { // responseWriter wraps http.ResponseWriter to capture the status code. type responseWriter struct { http.ResponseWriter - status int + status int + wroteHeader bool } func (rw *responseWriter) WriteHeader(code int) { + if rw.wroteHeader { + return + } rw.status = code + rw.wroteHeader = true rw.ResponseWriter.WriteHeader(code) } func (rw *responseWriter) Write(b []byte) (int, error) { + if !rw.wroteHeader { + rw.WriteHeader(http.StatusOK) + } return rw.ResponseWriter.Write(b) } +// Unwrap preserves optional net/http capabilities (notably +// ResponseController's deadline methods) through the logging middleware. +func (rw *responseWriter) Unwrap() http.ResponseWriter { return rw.ResponseWriter } + +// Flush keeps SSE and other streaming handlers working when the response +// writer is wrapped for logging. +func (rw *responseWriter) Flush() { + if f, ok := rw.ResponseWriter.(http.Flusher); ok { + f.Flush() + } +} + // isCORSSettingAllowed reports whether the given origin is permitted // by the configured CORS origins. func (s *Server) isCORSSettingAllowed(origin string) bool { diff --git a/internal/engine/approval_gate.go b/internal/engine/approval_gate.go index 73456075..ef748e6b 100644 --- a/internal/engine/approval_gate.go +++ b/internal/engine/approval_gate.go @@ -23,9 +23,10 @@ import ( type ApprovalCategory string const ( - ApprovalFileDeletion ApprovalCategory = "file_deletion" - ApprovalNetwork ApprovalCategory = "network" - ApprovalExternalAPI ApprovalCategory = "external_api" + ApprovalFileDeletion ApprovalCategory = "file_deletion" + ApprovalNetwork ApprovalCategory = "network" + ApprovalExternalAPI ApprovalCategory = "external_api" + ApprovalDatabaseWrite ApprovalCategory = "database_write" ) // ApprovalResponse is the typed result of a human approval gate decision. @@ -129,6 +130,10 @@ func (g *ApprovalGate) classifyAction(toolName string, args map[string]interface switch canon { case "WebFetch", "WebSearch": return ApprovalNetwork, true + case "SQL": + if allow, ok := args["allow_write"].(bool); ok && allow { + return ApprovalDatabaseWrite, true + } case "Bash": if cmd, ok := args["command"].(string); ok { if isDestructiveDelete(cmd) { diff --git a/internal/engine/approval_gate_test.go b/internal/engine/approval_gate_test.go index 2722ae67..994a13d3 100644 --- a/internal/engine/approval_gate_test.go +++ b/internal/engine/approval_gate_test.go @@ -167,3 +167,54 @@ func TestApprovalGate_FallbackAskUserFn(t *testing.T) { t.Fatal("AskUserFn returning yes should approve") } } + +func TestApprovalGate_ApproveForNDefaultsToFive(t *testing.T) { + s := NewSession("test", "m", "", nil) + s.PermSvc().SetAutonomy(AutonomyFull) + confirmations := 0 + s.SetApproval(&ApprovalGate{ + Enabled: true, + ConfirmFn: func(req ApprovalRequest) ApprovalResponse { + if req.Category != ApprovalNetwork { + t.Fatalf("unexpected category: %s", req.Category) + } + confirmations++ + if confirmations == 1 { + return ApprovalApproveForN + } + return ApprovalReject + }, + }) + for i := 0; i < 6; i++ { + ok, _ := s.CheckApproval(context.Background(), "WebFetch", map[string]interface{}{"url": "https://example.com"}) + if !ok { + t.Fatalf("approval %d should pass", i+1) + } + } + ok, _ := s.CheckApproval(context.Background(), "WebFetch", map[string]interface{}{"url": "https://example.com"}) + if ok { + t.Fatal("sixth action should require a new approval") + } +} + +func TestApprovalGate_SQLWriteCategory(t *testing.T) { + for _, toolName := range []string{"SQL", "sql", "sql_query"} { + t.Run(toolName, func(t *testing.T) { + s := NewSession("test", "m", "", nil) + s.PermSvc().SetAutonomy(AutonomyFull) + s.SetApproval(&ApprovalGate{ + Enabled: true, + ConfirmFn: func(req ApprovalRequest) ApprovalResponse { + if req.Category != ApprovalDatabaseWrite { + t.Fatalf("expected database_write category, got %s", req.Category) + } + return ApprovalReject + }, + }) + ok, _ := s.CheckApproval(context.Background(), toolName, map[string]interface{}{"allow_write": true}) + if ok { + t.Fatalf("%s writes should require approval", toolName) + } + }) + } +} diff --git a/internal/engine/l2_home_paths_test.go b/internal/engine/l2_home_paths_test.go index e68f10c8..f4bfe7fb 100644 --- a/internal/engine/l2_home_paths_test.go +++ b/internal/engine/l2_home_paths_test.go @@ -1,7 +1,6 @@ package engine import ( - "os" "path/filepath" "strings" "testing" @@ -17,14 +16,12 @@ import ( // constructors, which leaked into /cmd/.hawk/ when hawk was run // from its own source tree. func TestL2PipelineStatePathsAreHomeRelative(t *testing.T) { - home, err := os.UserHomeDir() - if err != nil { - t.Fatalf("os.UserHomeDir: %v", err) - } - if home == "" { - t.Fatal("os.UserHomeDir returned empty string") - } - wantPrefix := filepath.Clean(home) + string(filepath.Separator) + // Make the test independent of the caller's HOME/HAWK_STATE_DIR. The + // production contract is the configured per-user state root, and tests may + // intentionally redirect that root to an isolated temporary directory. + stateRoot := t.TempDir() + t.Setenv("HAWK_STATE_DIR", stateRoot) + wantPrefix := filepath.Clean(stateRoot) + string(filepath.Separator) check := func(name, got string) { t.Helper() @@ -32,8 +29,8 @@ func TestL2PipelineStatePathsAreHomeRelative(t *testing.T) { t.Errorf("%s: path %q is not absolute", name, got) return } - if !strings.HasPrefix(got, wantPrefix) && !strings.HasPrefix(got, filepath.Clean(home)) { - t.Errorf("%s: path %q does not start with home dir %q", name, got, home) + if !strings.HasPrefix(got, wantPrefix) { + t.Errorf("%s: path %q does not start with state root %q", name, got, stateRoot) } } diff --git a/internal/engine/permission_service.go b/internal/engine/permission_service.go index a5dbcba7..e206762d 100644 --- a/internal/engine/permission_service.go +++ b/internal/engine/permission_service.go @@ -253,7 +253,11 @@ func (s *PermissionService) CheckApproval(_ context.Context, toolName string, ar return true, "" case ApprovalApproveForN: // Default N=5 when the typed response carries no count. - g.nApprove(cat, req.N) + n := req.N + if n <= 0 { + n = 5 + } + g.nApprove(cat, n) return true, "" case ApprovalApprove: return true, "" diff --git a/internal/engine/permission_session_methods.go b/internal/engine/permission_session_methods.go index 13ec8826..15690f7d 100644 --- a/internal/engine/permission_session_methods.go +++ b/internal/engine/permission_session_methods.go @@ -162,6 +162,8 @@ func canonicalToolName(name string) string { return "WebFetch" case "web_search", "websearch": return "WebSearch" + case "sql", "sql_query": + return "SQL" case "agent", "task": return "Agent" case "ask_user", "askuserquestion": diff --git a/internal/engine/safety/permission.go b/internal/engine/safety/permission.go index 64eeaef0..9467ac3e 100644 --- a/internal/engine/safety/permission.go +++ b/internal/engine/safety/permission.go @@ -315,6 +315,8 @@ func canonicalToolName(name string) string { return "WebFetch" case "web_search", "websearch": return "WebSearch" + case "sql", "sql_query": + return "SQL" case "agent", "task": return "Agent" case "ask_user", "askuser", "askuserquestion": diff --git a/internal/engine/stream.go b/internal/engine/stream.go index ab6ec7f9..de9fc637 100644 --- a/internal/engine/stream.go +++ b/internal/engine/stream.go @@ -694,10 +694,10 @@ func (s *Session) agentLoop(ctx context.Context, ch chan<- StreamEvent) { break } } - // Use context.WithoutCancel: this goroutine outlives the caller, - // and ctx would be cancelled on return, killing the end-session - // pipeline before it can assess, learn, and persist experience. - go s.LifecycleSvc().Pipeline().EndSession(context.WithoutCancel(ctx), ctx.Err() == nil, taskGoal) + // End-session persistence must complete before the stream closes. A + // detached goroutine can write after callers tear down their state + // directory, losing feedback and racing test/process cleanup. + s.LifecycleSvc().Pipeline().EndSession(context.WithoutCancel(ctx), ctx.Err() == nil, taskGoal) } // Session end hook hooks.ExecuteAsync(context.WithoutCancel(ctx), hooks.EventSessionEnd, map[string]interface{}{ diff --git a/internal/engine/tool_service.go b/internal/engine/tool_service.go index 3b592d81..5033c191 100644 --- a/internal/engine/tool_service.go +++ b/internal/engine/tool_service.go @@ -695,39 +695,14 @@ func (s *ToolService) EstimateBlastRadius(planned []PlannedCall) *BlastRadiusRep return EstimateBlastRadius(planned) } -// ExecuteRegistered runs a single registered tool call with the configured isolation + -// retry policy. Returns the (output, isErr) pair. The tool_result -// StreamEvent is emitted on ch. +// ExecuteRegistered is the compatibility entry point for callers that still +// use the legacy API. Delegate to the canonical ExecuteOne/CompleteResult +// pipeline so permission, approval, context, timeout, hooks, and redaction +// cannot be bypassed. func (s *ToolService) ExecuteRegistered(ctx context.Context, tc types.ToolCall, ch chan<- StreamEvent) (string, bool) { - containerExecutor, containerRequired := s.containerState() - if containerRequired { - if containerExecutor == nil || !containerExecutor.Running() { - msg := "Container not ready — tools are disabled until the sandbox is running." - ch <- StreamEvent{Type: "tool_result", ToolName: tc.Name, Content: msg} - return msg, true - } - } - if s.tracer != nil { - _, _ = oteltrace.StartToolSpan(ctx, s.tracer, tc.Name, tc.ID) - } - t, _ := s.registry.Get(tc.Name) - var output string - var execErr error - if rpp, ok := t.(tool.RetryPolicyProvider); ok { - output, execErr = tool.RetryExecutor(ctx, t, marshalInput(tc), rpp.RetryPolicy()) - } else { - output, execErr = tool.RetryExecutor(ctx, t, marshalInput(tc), tool.DefaultRetryPolicy()) - } - isErr := execErr != nil - if isErr { - output = fmt.Sprintf("Error: %s", execErr.Error()) - } - // Redact user-facing tool output (the model copy is redacted separately). - if s.deps.redactOutput != nil { - output = s.deps.redactOutput(output) - } - ch <- StreamEvent{Type: "tool_result", ToolName: tc.Name, Content: output} - return output, isErr + result := s.ExecuteOne(ctx, tc, nil, ch, 0, "") + result = s.CompleteResult(ctx, result, ch) + return result.output, result.isErr } // BackgroundManager returns the background sub-agent manager, or nil @@ -774,9 +749,3 @@ func (s *ToolService) Sandbox() *diff.DiffSandbox { return s.sandbox } // SetSandbox attaches the diff sandbox. func (s *ToolService) SetSandbox(sb *diff.DiffSandbox) { s.sandbox = sb } - -// marshalInput serializes a tool call's args to JSON. -func marshalInput(tc types.ToolCall) json.RawMessage { - b, _ := json.Marshal(tc.Arguments) - return b -} diff --git a/internal/sandbox/container.go b/internal/sandbox/container.go index 4adca799..f1dca8af 100644 --- a/internal/sandbox/container.go +++ b/internal/sandbox/container.go @@ -231,17 +231,12 @@ func (c *ContainerSandbox) dockerRunArgs(name, attachDir, cacheDir string) []str "-w", c.projectDir, "--entrypoint", "sleep", } - // User-namespace remapping further isolates the container from the host - // kernel (H16); only added when the daemon supports it. Without userns - // the container would run as root against the rw project mount, so fall - // back to --user with the host uid:gid (M12). exec.CommandContext runs - // the container process as that uid inside the container regardless of - // whether /etc/passwd knows it. - if usernsRemapAvailable() { - args = append(args, "--userns-remap", "default") - } else { - args = append(args, "--user", fmt.Sprintf("%d:%d", os.Getuid(), os.Getgid())) - } + // User namespace remapping is a Docker-daemon setting, not a valid + // `docker run` option. Always set the container process identity explicitly + // so a daemon without remapping cannot fall back to root on the project + // mount. A daemon configured with userns remapping still applies its UID + // mapping to this non-root container user. + args = append(args, "--user", fmt.Sprintf("%d:%d", os.Getuid(), os.Getgid())) // SSH Agent Socket Passthrough: Forward host SSH auth socket so git push/fetch works // over SSH without copying or mounting raw SSH private keys into the container. if sshSock := os.Getenv("SSH_AUTH_SOCK"); sshSock != "" { diff --git a/internal/sandbox/container_test.go b/internal/sandbox/container_test.go index 2ee4cfb9..61d7e045 100644 --- a/internal/sandbox/container_test.go +++ b/internal/sandbox/container_test.go @@ -279,9 +279,8 @@ func containsStr(s, sub string) bool { return false } -// TestContainerSandbox_DockerRunArgs_UserFallback verifies that without -// userns remapping the container runs as the host uid:gid instead of root -// (M12), and that userns remapping suppresses the --user fallback. +// TestContainerSandbox_DockerRunArgs_User verifies that the container always +// runs as the host uid:gid and never emits the invalid --userns-remap run flag. func TestContainerSandbox_DockerRunArgs_UserFallback(t *testing.T) { original := usernsProbe t.Cleanup(func() { usernsProbe = original; resetUsernsCache() }) @@ -290,7 +289,7 @@ func TestContainerSandbox_DockerRunArgs_UserFallback(t *testing.T) { cs := NewContainerSandbox(projectDir) cs.SetImage("hawk:test") - // userns unavailable -> --user fallback with host uid:gid. + // userns unavailable -> --user with host uid:gid. resetUsernsCache() usernsProbe = func() (bool, error) { return false, nil } args := strings.Join(cs.dockerRunArgs("hawk-test", "/tmp/attach", "/tmp/cache"), " ") @@ -302,15 +301,15 @@ func TestContainerSandbox_DockerRunArgs_UserFallback(t *testing.T) { t.Fatalf("userns-remap must not be added when unavailable:\n%s", args) } - // userns available -> --userns-remap, no --user fallback. + // userns available still uses the valid --user flag; remapping is daemon-side. resetUsernsCache() usernsProbe = func() (bool, error) { return true, nil } args = strings.Join(cs.dockerRunArgs("hawk-test", "/tmp/attach", "/tmp/cache"), " ") - if !strings.Contains(args, "--userns-remap default") { - t.Fatalf("expected --userns-remap default in run args, got:\n%s", args) + if !strings.Contains(args, wantUser) { + t.Fatalf("expected %q in run args with daemon userns, got:\n%s", wantUser, args) } - if strings.Contains(args, "--user ") { - t.Fatalf("--user fallback must not be added when userns is available:\n%s", args) + if strings.Contains(args, "--userns-remap") { + t.Fatalf("docker run must not receive daemon-only --userns-remap:\n%s", args) } } diff --git a/internal/sandbox/sandbox.Dockerfile b/internal/sandbox/sandbox.Dockerfile index 9e6c3b42..baa41e4d 100644 --- a/internal/sandbox/sandbox.Dockerfile +++ b/internal/sandbox/sandbox.Dockerfile @@ -1,6 +1,9 @@ -FROM node:22-bookworm-slim +FROM node:22-bookworm-slim@sha256:d649c27dae7ba0137b3cef5dd75baa422c08dc3d9e3fc0c23dfb172dc3cc6436 -RUN apt-get update && \ +RUN npm install --global npm@12.0.2 && \ + npm cache clean --force && \ + rm -rf /root/.npm && \ + apt-get update && \ apt-get install -y --no-install-recommends \ bash \ ca-certificates \ diff --git a/internal/sandbox/sandbox.go b/internal/sandbox/sandbox.go index ec5a4762..b60ef9a6 100644 --- a/internal/sandbox/sandbox.go +++ b/internal/sandbox/sandbox.go @@ -183,9 +183,9 @@ func (s *Sandbox) runDocker(ctx context.Context, command string) (*exec.Cmd, err "--memory", fmt.Sprintf("%dm", s.config.MaxMemoryMB), "--cpus", fmt.Sprintf("%.2f", float64(s.config.MaxCPUPct)/100.0), } - if usernsRemapAvailable() { - args = append(args, "--userns-remap", "default") - } + // `--userns-remap` is a daemon configuration flag, not accepted by + // `docker run`. Run as the invoking host UID/GID in either daemon mode. + args = append(args, "--user", fmt.Sprintf("%d:%d", os.Getuid(), os.Getgid())) if !s.config.AllowNetwork { args = append(args, "--network", "none") } diff --git a/internal/session/wal_batch.go b/internal/session/wal_batch.go index ebdeb994..649f6817 100644 --- a/internal/session/wal_batch.go +++ b/internal/session/wal_batch.go @@ -103,13 +103,19 @@ func (b *BatchedWAL) ensureTimerLocked() { // Close flushes remaining entries and closes the underlying WAL. func (b *BatchedWAL) Close() error { - _ = b.Flush() - return b.wal.Close() + flushErr := b.Flush() + closeErr := b.wal.Close() + if flushErr != nil { + return flushErr + } + return closeErr } // Remove flushes buffered entries, closes the underlying WAL, and deletes the // WAL file. Mirrors WAL.Remove so callers can use BatchedWAL interchangeably. func (b *BatchedWAL) Remove() error { - _ = b.Flush() + if err := b.Flush(); err != nil { + return err + } return b.wal.Remove() } diff --git a/internal/snapshot/l2_home_paths_test.go b/internal/snapshot/l2_home_paths_test.go index 17f16124..480803f9 100644 --- a/internal/snapshot/l2_home_paths_test.go +++ b/internal/snapshot/l2_home_paths_test.go @@ -1,7 +1,6 @@ package snapshot import ( - "os" "path/filepath" "strings" "testing" @@ -14,17 +13,12 @@ import ( // like ".hawk/snapshots" and ".hawk/experience" which leaked into // /cmd/.hawk/ when hawk was run from its own source tree. func TestL2DefaultPathsAreHomeRelative(t *testing.T) { - home, err := os.UserHomeDir() - if err != nil { - t.Fatalf("os.UserHomeDir: %v", err) - } - if home == "" { - t.Fatal("os.UserHomeDir returned empty string") - } - - // Sanitize HOME so we can compare reliably (filepath.Clean strips - // trailing separators). - wantPrefix := filepath.Clean(home) + string(filepath.Separator) + // Make the regression deterministic under CI and sandboxed runners. The + // production default is the configured state root, which may intentionally + // differ from HOME via HAWK_STATE_DIR. + stateRoot := t.TempDir() + t.Setenv("HAWK_STATE_DIR", stateRoot) + wantPrefix := filepath.Clean(stateRoot) + string(filepath.Separator) check := func(name, got string) { t.Helper() @@ -32,10 +26,8 @@ func TestL2DefaultPathsAreHomeRelative(t *testing.T) { t.Errorf("%s: default path %q is not absolute", name, got) return } - // On macOS temp dirs may live under /private/var/... while HOME - // resolves to /var/...; compare both forms. - if !strings.HasPrefix(got, wantPrefix) && !strings.HasPrefix(got, filepath.Clean(home)) { - t.Errorf("%s: default path %q does not start with home dir %q", name, got, home) + if !strings.HasPrefix(got, wantPrefix) { + t.Errorf("%s: default path %q does not start with state root %q", name, got, stateRoot) } } diff --git a/internal/tool/sql.go b/internal/tool/sql.go index ae7d39f0..cd7d86af 100644 --- a/internal/tool/sql.go +++ b/internal/tool/sql.go @@ -5,6 +5,7 @@ import ( "database/sql" "encoding/json" "fmt" + "net/url" "strings" "time" @@ -111,13 +112,108 @@ var writeVerbs = map[string]bool{ // mode. It inspects the first meaningful keyword after stripping leading // comments and whitespace. func isReadOnlyQuery(query string) bool { - first := firstSQLKeyword(query) + statements, err := splitSQLStatements(query) + if err != nil || len(statements) != 1 { + return false + } + first := firstSQLKeyword(statements[0]) if first == "" { return false } + // Allow read-only CTEs. SQLite's connection-level query_only guard still + // blocks mutations when allow_write is false, and splitSQLStatements keeps + // stacked statements out. + if first == "with" { + return true + } return !writeVerbs[first] } +// splitSQLStatements accepts one SQL statement and ignores semicolons inside +// quoted strings and comments. SQLite permits stacked statements, so callers +// must reject more than one statement even when writes are explicitly allowed. +func splitSQLStatements(query string) ([]string, error) { + var statements []string + start := 0 + var quote byte + lineComment, blockComment := false, false + for i := 0; i < len(query); i++ { + c := query[i] + if lineComment { + if c == '\n' { + lineComment = false + } + continue + } + if blockComment { + if c == '*' && i+1 < len(query) && query[i+1] == '/' { + blockComment = false + i++ + } + continue + } + if quote != 0 { + if c == quote { + if i+1 < len(query) && query[i+1] == quote { + i++ // SQL escapes a quote by doubling it. + } else { + quote = 0 + } + } + continue + } + switch c { + case '-', '/': + if c == '-' && i+1 < len(query) && query[i+1] == '-' { + lineComment = true + i++ + } else if c == '/' && i+1 < len(query) && query[i+1] == '*' { + blockComment = true + i++ + } + case '\'', '"', '`': + quote = c + case ';': + if statement := strings.TrimSpace(query[start:i]); statement != "" { + statements = append(statements, statement) + } + start = i + 1 + } + } + if blockComment || quote != 0 { + return nil, fmt.Errorf("query contains an unterminated SQL comment or quoted string") + } + if statement := strings.TrimSpace(query[start:]); statement != "" { + statements = append(statements, statement) + } + return statements, nil +} + +func validateSQLiteDSN(ctx context.Context, dsn string) error { + trimmed := strings.TrimSpace(dsn) + if trimmed == ":memory:" || strings.HasPrefix(trimmed, "file::memory:") { + return nil + } + path := trimmed + if strings.HasPrefix(trimmed, "file:") { + u, err := url.Parse(trimmed) + if err != nil || u.Host != "" { + return fmt.Errorf("sqlite dsn must reference a local file") + } + path = u.Path + if path == "" { + return fmt.Errorf("sqlite file dsn is missing a path") + } + if decoded, err := url.PathUnescape(path); err == nil { + path = decoded + } + } + if err := validatePathAllowed(ctx, path); err != nil { + return fmt.Errorf("sqlite dsn: %w", err) + } + return nil +} + // firstSQLKeyword returns the lowercased first keyword of a statement, skipping // leading line (--) and block (/* */) comments and whitespace. func firstSQLKeyword(query string) string { @@ -182,9 +278,21 @@ func (t SQLTool) Execute(ctx context.Context, input json.RawMessage) (string, er return "", fmt.Errorf("the %s driver is not compiled into this build of hawk; only sqlite is available", driver) } - if !p.AllowWrite && !isReadOnlyQuery(p.Query) { + statements, err := splitSQLStatements(p.Query) + if err != nil { + return "", fmt.Errorf("invalid query: %w", err) + } + if len(statements) != 1 { + return "", fmt.Errorf("exactly one SQL statement is required") + } + if !p.AllowWrite && !isReadOnlyQuery(statements[0]) { return "", fmt.Errorf("refusing to run a destructive statement in read-only mode; set allow_write=true to override") } + if driver == "sqlite" { + if err := validateSQLiteDSN(ctx, p.DSN); err != nil { + return "", err + } + } db, err := sql.Open(driver, p.DSN) if err != nil { @@ -194,8 +302,16 @@ func (t SQLTool) Execute(ctx context.Context, input json.RawMessage) (string, er queryCtx, cancel := context.WithTimeout(ctx, 30*time.Second) defer cancel() + if !p.AllowWrite { + // PRAGMA query_only is connection-local. Restrict the pool to one + // connection so the guard applies to the subsequent query as well. + db.SetMaxOpenConns(1) + if _, err := db.ExecContext(queryCtx, "PRAGMA query_only=ON"); err != nil { + return "", fmt.Errorf("enable sqlite read-only mode: %w", err) + } + } - return runSQLQuery(queryCtx, db, p.Query, p.MaxRows) + return runSQLQuery(queryCtx, db, statements[0], p.MaxRows) } // querier is the subset of *sql.DB used by runSQLQuery, extracted so tests can diff --git a/internal/tool/sql_test.go b/internal/tool/sql_test.go index 44b5ff32..66846445 100644 --- a/internal/tool/sql_test.go +++ b/internal/tool/sql_test.go @@ -151,6 +151,33 @@ func TestIsReadOnlyQuery(t *testing.T) { } } +func TestSQLToolRejectsStackedStatements(t *testing.T) { + tool := SQLTool{} + in, _ := json.Marshal(map[string]any{ + "driver": "sqlite", + "dsn": ":memory:", + "query": "SELECT 1; DROP TABLE users", + "allow_write": true, + }) + _, err := tool.Execute(context.Background(), in) + if err == nil || !strings.Contains(err.Error(), "exactly one") { + t.Fatalf("expected stacked statements to be rejected, got %v", err) + } +} + +func TestSQLToolRejectsUnterminatedSQL(t *testing.T) { + tool := SQLTool{} + in, _ := json.Marshal(map[string]any{ + "driver": "sqlite", + "dsn": ":memory:", + "query": "SELECT 'unterminated", + }) + _, err := tool.Execute(context.Background(), in) + if err == nil || !strings.Contains(err.Error(), "unterminated") { + t.Fatalf("expected unterminated query error, got %v", err) + } +} + func TestSQLDriverName(t *testing.T) { tests := []struct { in string