From 145624f7250740c2eba48530f6c9a8fdcdfbe98e Mon Sep 17 00:00:00 2001 From: Ali Al Dallal Date: Wed, 12 Aug 2026 02:18:34 -0400 Subject: [PATCH] feat: public-repo hygiene baseline (goal 0028) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit README/SECURITY/CONTRIBUTING/issue-template/Scorecard workflow + golangci-lint strengthening (gosec/bodyclose/noctx/revive/unparam, fully triaged) + dependency-review deny-licenses + elkjs EPL-2.0 verdict, closing the real community-profile/security gaps a public repo needs without ceremony (research delivered prior session: community-profile score 28%, exposure sweep clean, LICENSE already correct). - README: full rewrite from the Wails scaffold -- what Mill actually is (SPEC §1's thesis, no vendor names), honest pre-1.0 status, and an install story that actually works for a stranger (adds the Go/Node/Task/Wails3-CLI prerequisites CLAUDE.md's own commands assume but never state). CI + new Scorecard badges. - SECURITY.md: GitHub private vulnerability reporting enabled live (verified `{"enabled":true}`) as the report channel, an honest scope paragraph (guardrailed command execution, keychain secrets, the loopback-only unauthenticated MCP listener), pre-1.0 rolling-main support note. - CONTRIBUTING.md + one bug-report issue template (reuses the build-identity badge value as the version field). .ls-lint.yml's root allowlist extended for SECURITY/CONTRIBUTING with a comment. - .github/workflows/scorecard.yml from the official ossf/scorecard template, action SHAs verified live against upstream tags (not assumed), matching this repo's existing pinning style. - .golangci.yml: gosec/bodyclose/noctx/revive/unparam enabled and triaged to zero findings on both the `server` and desktop build-tag variants. One real bug found and fixed along the way: mcpserving.Serve's http.Server had no ReadHeaderTimeout, a genuine Slowloris exposure even on a loopback listener. Every gosec suppression carries an inline justification, never blanket #nosec. revive's exported/package-comments rules disabled with a recorded reason (this repo has never doc-commented every exported symbol; enforcing it retroactively is ceremony, not a real finding). Second pass (gocritic/prealloc/contextcheck/sqlclosecheck) explicitly named as future work in the goal file, not attempted here. - ci.yml: dependency-review-action gains deny-licenses for the GPL/AGPL family (Apache-2.0, Mill's own license, is incompatible with copyleft terms). - docs/SPEC.md §3: elkjs's EPL-2.0-vs-Apache-2.0 verdict recorded where SPEC already flagged it -- unmodified dependency, its own dynamic-import bundle chunk, no conflict. - De-literalized the two /Users/ali paths (test-investigator.md, launchatlogin_desktop_test.go). Full local suite green (lint/vet/build/test, both build-tag variants, frontend static, e2e) -- 4 known-flaky e2e specs (canvas-click/ resizable-table/live-run-state/activity-row timing) reproduced as pass-on-retry in isolation, unrelated to any file this goal touched. Goal 0028 delivered and archived; BACKLOG updated. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01FYwojT8GdUbYSoggbvEFft --- .claude/agents/test-investigator.md | 2 +- .github/ISSUE_TEMPLATE/bug_report.md | 19 +++ .github/workflows/ci.yml | 3 + .github/workflows/scorecard.yml | 53 ++++++++ .golangci.yml | 28 ++++ .ls-lint.yml | 7 +- CONTRIBUTING.md | 41 ++++++ README.md | 122 +++++++++--------- SECURITY.md | 46 +++++++ docs/SPEC.md | 13 ++ docs/goals/0028-public-repo-hygiene.md | 50 ------- docs/goals/BACKLOG.md | 18 ++- .../goals/archive/0028-public-repo-hygiene.md | 85 ++++++++++++ internal/adapters/clipboard/clipboard.go | 30 ++++- internal/adapters/clipboard/clipboard_test.go | 5 +- .../adapters/dockbadge/dockbadge_server.go | 2 +- internal/adapters/execution/execution_test.go | 2 +- internal/adapters/fileread/fileread.go | 7 +- internal/adapters/fileread/fileread_test.go | 4 +- internal/adapters/filewatch/filewatch_test.go | 8 +- internal/adapters/hotkey/hotkey_server.go | 2 +- .../httpconnector/httpconnector_test.go | 6 +- .../adapters/idletime/idletime_desktop.go | 9 +- .../launchatlogin/launchatlogin_desktop.go | 24 +++- .../launchatlogin_desktop_test.go | 2 +- .../launchatlogin/launchatlogin_server.go | 6 +- internal/adapters/mcpclient/mcpclient.go | 18 ++- internal/adapters/mcpserving/mcpserving.go | 10 +- internal/adapters/notify/notify.go | 2 +- internal/adapters/notify/notify_server.go | 6 +- internal/adapters/procexec/procexec.go | 8 +- internal/adapters/settings/settings.go | 2 +- internal/adapters/shellenv/shellenv.go | 36 +++--- internal/domain/composition/authapikey.go | 2 +- internal/domain/composition/authbearer.go | 2 +- internal/domain/composition/authhmac.go | 2 +- internal/domain/composition/authmtls.go | 2 +- internal/domain/composition/authnone.go | 2 +- internal/domain/composition/authoauth1.go | 2 +- .../domain/composition/authoauth1vendor.go | 2 +- internal/domain/composition/authqueryparam.go | 2 +- .../domain/composition/capturefile_test.go | 4 +- internal/domain/composition/childworkflow.go | 4 +- internal/domain/composition/codeexec.go | 2 +- internal/domain/composition/codeexec_test.go | 5 +- internal/domain/composition/humanreview.go | 2 +- .../composition/integrationexec_test.go | 2 +- internal/domain/composition/seedproof_test.go | 6 +- internal/domain/httprequest/builtin.go | 16 +-- .../configuresvc/configureservice_builtin.go | 2 +- .../configureservice_requestauth_test.go | 2 +- .../executionservice_guardrail_test.go | 14 +- .../executionsvc/initialpayload_test.go | 2 +- .../mcpsvc/millmcpservice_approval.go | 3 +- .../mcpsvc/millmcpservice_payload_test.go | 2 +- .../services/mcpsvc/millmcpservice_tools.go | 2 +- .../triggersvc/filesystemwatch_seed_test.go | 2 +- .../triggersvc/savedpage_seed_test.go | 2 +- 58 files changed, 550 insertions(+), 214 deletions(-) create mode 100644 .github/ISSUE_TEMPLATE/bug_report.md create mode 100644 .github/workflows/scorecard.yml create mode 100644 CONTRIBUTING.md create mode 100644 SECURITY.md delete mode 100644 docs/goals/0028-public-repo-hygiene.md create mode 100644 docs/goals/archive/0028-public-repo-hygiene.md diff --git a/.claude/agents/test-investigator.md b/.claude/agents/test-investigator.md index 837de6d1..44049ef6 100644 --- a/.claude/agents/test-investigator.md +++ b/.claude/agents/test-investigator.md @@ -7,7 +7,7 @@ model: sonnet You run Mill's checks and report what actually failed — nothing else. -Suites and how to run them (from the repo root, /Users/ali/code/mill): +Suites and how to run them (from the repo root): - Go: `go test -tags server -count=1 -timeout 600s ./internal/... .` - Frontend static: `cd frontend && npx tsc --noEmit && npm run lint && npm run boundaries` - E2e (server build required first): diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 00000000..1844f3ba --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,19 @@ +--- +name: Bug report +about: Report something that isn't working +title: '' +labels: bug +assignees: '' +--- + +**What happened?** + +**What did you expect to happen instead?** + +**macOS version:** + +**Build-identity badge value** (top-left of Mill's window — one of +`DEV · live`, `INSTALLED · `, `SERVER · `, or a red +`STALE BUILD` warning): + +**Steps to reproduce (if known):** diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0e3d9e3a..fabc07a1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -385,6 +385,9 @@ jobs: steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: actions/dependency-review-action@a1d282b36b6f3519aa1f3fc636f609c47dddb294 # v5.0.0 + with: + # goal 0028: a copyleft dependency can't enter via PR unnoticed -- Apache-2.0 (Mill's own LICENSE) is incompatible with GPL/AGPL's copyleft terms. + deny-licenses: GPL-2.0-only, GPL-2.0-or-later, GPL-3.0-only, GPL-3.0-or-later, AGPL-3.0-only, AGPL-3.0-or-later # The future single required check (goal 0024/ADR-0034): decouples the # branch ruleset from job-name churn -- the ruleset names only diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml new file mode 100644 index 00000000..184d5fc0 --- /dev/null +++ b/.github/workflows/scorecard.yml @@ -0,0 +1,53 @@ +name: Scorecard analysis workflow + +# Official ossf/scorecard template (goal 0028), adapted only for this +# repo's SHA-pinning style (matches ci.yml/release.yml's pinned +# actions/checkout and actions/upload-artifact versions, rather than the +# template's own independently-pinned SHAs, which happened to already +# match here). publish_results: true opts into the public score at +# https://scorecard.dev, which is what makes the README badge live. +on: + push: + # Only the default branch is supported by Scorecard's own docs. + branches: + - main + schedule: + # Weekly, Saturdays -- matches the upstream template's cadence. + - cron: '30 1 * * 6' + +permissions: read-all + +jobs: + analysis: + name: Scorecard analysis + runs-on: ubuntu-latest + permissions: + # Needed for Code scanning upload. + security-events: write + # Needed for GitHub OIDC token, required by publish_results: true. + id-token: write + + steps: + - name: 'Checkout code' + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: 'Run analysis' + uses: ossf/scorecard-action@2d1146689b8cda280b9bc96326124645441f03bc # v2.4.4 + with: + results_file: results.sarif + results_format: sarif + publish_results: true + + - name: 'Upload artifact' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: SARIF file + path: results.sarif + retention-days: 5 + + - name: 'Upload to code-scanning' + uses: github/codeql-action/upload-sarif@f205ea1c3313d32999d8d6a48b4f6530d4437b38 # v4.37.4 + with: + sarif_file: results.sarif diff --git a/.golangci.yml b/.golangci.yml index d1ce3c10..27259225 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -5,6 +5,16 @@ linters: enable: - unused - staticcheck + # goal 0028, first strengthening pass (security + HTTP-client hygiene + # matching what the code actually does -- procexec/httpconnector/ + # openapispec are the real attack-surface packages). Second pass + # (gocritic/prealloc/contextcheck/sqlclosecheck) is deliberately not + # part of this goal -- tracked as future work in goal 0028's file. + - gosec + - bodyclose + - noctx + - revive + - unparam exclusions: paths: - frontend @@ -13,6 +23,24 @@ linters: - builtin$ - examples$ + settings: + revive: + rules: + # This repo has never doc-commented every exported symbol -- + # confirmed by the first real run of this rule, which flagged + # ~40 pre-existing exported consts/types/funcs across files that + # already had real, deliberate doc comments on the pieces that + # matter (packages, non-obvious functions) and none on + # self-explanatory ones (e.g. `StatusLocked Status = "LOCKED"`). + # Enforcing full Java/godoc-style coverage retroactively is + # ceremony that fights the established house style, not a real + # finding -- goal 0028's own explicit "tune revive if its + # defaults fight house style" guidance (e.g. package-comments). + - name: exported + disabled: true + - name: package-comments + disabled: true + formatters: exclusions: paths: diff --git a/.ls-lint.yml b/.ls-lint.yml index 25f7db52..5a738516 100644 --- a/.ls-lint.yml +++ b/.ls-lint.yml @@ -53,7 +53,12 @@ # layout decisions, never accidents. ls: .go: regex:(main|singleinstance_(production|dev)) - .*: regex:(README|CLAUDE|Taskfile|lefthook|go|\.golangci|\.ls-lint) + # SECURITY/CONTRIBUTING added goal 0028 (public-repo hygiene): both are + # standard GitHub community-profile root files, same allowlist family as + # README below (checked, not new -- CODE_OF_CONDUCT deliberately stays + # OUT per goal 0028's skip list until a second contributor exists, so + # it's not pre-added here). + .*: regex:(README|SECURITY|CONTRIBUTING|CLAUDE|Taskfile|lefthook|go|\.golangci|\.ls-lint) ignore: - build diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 00000000..4a312357 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,41 @@ +# Contributing to Mill + +Mill is solo-maintained. Contributions are welcome, but the process is +kept deliberately light — there's no separate contributor doc to +maintain in parallel with reality. + +## Process + +- **Read `CLAUDE.md` first.** It's the actual working process for this + repo (Research → Plan → Implement, coding conventions in + `.claude/rules/`), not an AI-only artifact — it applies whether you're + a human or an agent making the change. +- **Open an issue before a large PR.** Small fixes (typos, an obvious + bug with an obvious fix) can go straight to a PR. Anything that adds a + capability, changes a schema, or touches more than a couple of files + should start as an issue so the approach can be agreed before the work + is done — `docs/SPEC.md` is the source of truth for what Mill is and + why, and a PR that conflicts with it needs to resolve that first. +- **Run the local checks before opening a PR.** `task setup:hooks` + installs Lefthook's pre-commit hooks, which mirror what CI runs + (lint, vet, build, the file-length and root-layout checks). A PR + that fails CI's `ci-gate` required check won't merge. +- **Tests are part of the change, not a follow-up.** See + `.claude/rules/testing.md` for what layer a given bug or feature's + proof belongs at. + +## Getting set up + +See the [README](README.md#install) for the clone-and-run steps. + +## Reporting a bug + +Use the bug report issue template. It's short by design: what happened, +what you expected, your macOS version, and the build-identity badge +value shown in Mill's own UI (`DEV · live` / `INSTALLED · ` / +`SERVER · `) — that one field tells us exactly which build you +were running. + +## Reporting a security issue + +Don't open a public issue — see [SECURITY.md](SECURITY.md). diff --git a/README.md b/README.md index 30256fe4..4b568aba 100644 --- a/README.md +++ b/README.md @@ -1,61 +1,65 @@ -# Welcome to Your New Wails3 Project! +# Mill [![CI](https://github.com/alicoding/mill/actions/workflows/ci.yml/badge.svg)](https://github.com/alicoding/mill/actions/workflows/ci.yml) - -Congratulations on generating your Wails3 application! This README will guide you through the next steps to get your project up and running. - -## Getting Started - -1. Navigate to your project directory in the terminal. - -2. To run your application in development mode, use the following command: - - ``` - wails3 dev - ``` - - This will start your application and enable hot-reloading for both frontend and backend changes. - -3. To build your application for production, use: - - ``` - wails3 build - ``` - - This will create a production-ready executable in the `build` directory. - -## Exploring Wails3 Features - -Now that you have your project set up, it's time to explore the features that Wails3 offers: - -1. **Check out the examples**: The best way to learn is by example. Visit the `examples` directory in the `v3/examples` directory to see various sample applications. - -2. **Run an example**: To run any of the examples, navigate to the example's directory and use: - - ``` - go run . - ``` - - Note: Some examples may be under development during the alpha phase. - -3. **Explore the documentation**: Visit the [Wails3 documentation](https://v3.wails.io/) for in-depth guides and API references. - -4. **Join the community**: Have questions or want to share your progress? Join the [Wails Discord](https://discord.gg/JDdSxwjhGf) or visit the [Wails discussions on GitHub](https://github.com/wailsapp/wails/discussions). - -## Project Structure - -Take a moment to familiarize yourself with your project structure: - -- `frontend/`: Contains your frontend code (HTML, CSS, JavaScript/TypeScript) -- `main.go`: The entry point of your Go backend -- `app.go`: Define your application structure and methods here -- `wails.json`: Configuration file for your Wails project - -## Next Steps - -1. Modify the frontend in the `frontend/` directory to create your desired UI. -2. Add backend functionality in `main.go`. -3. Use `wails3 dev` to see your changes in real-time. -4. When ready, build your application with `wails3 build`. - -Happy coding with Wails3! If you encounter any issues or have questions, don't hesitate to consult the documentation or reach out to the Wails community. +[![OpenSSF Scorecard](https://api.scorecard.dev/projects/github.com/alicoding/mill/badge)](https://scorecard.dev/viewer/?uri=github.com/alicoding/mill) + +Mill is a guardrailed, agentic-workflow desktop app: it lets an AI agent (or +a human) compose and run automations — capturing data, processing it, +applying an action — while keeping every step reviewable and reversible. +The core idea is *what-you-see-is-what-I-see*: an AI acting on a system it +can't verify has to guess at the real state (what's actually on the +clipboard, what a command will really do, what a setting is really set to), +and that gap between the guess and reality is exactly where hallucination +and silent failure live. Mill closes that gap by giving both the human and +the AI the same verified, structured view of state — and a guardrail that +previews an action before it happens, instead of trusting a text +description of it. Mill isn't a novel category: it composes existing +primitives (a workflow authoring layer with guardrails) the way a generic +credential manager or a generic workflow-automation tool would, applied to +agent-guarded local actions instead. + +## Status + +Mill is under active development, pre-1.0. Several UX surfaces are +explicitly prototype-quality (tracked as such in `docs/SPEC.md`) while the +underlying capability is real and exercised end-to-end. Expect rough edges +in presentation before you expect them in behavior — and expect both to +keep changing release to release. + +## Install + +Mill ships as a single Go binary with the frontend compiled in (no +separate CLI/backend, no hosted-service dependency) — `git clone` plus a +local build is the whole install story. You'll need Go 1.25+, Node 22+, +the [Task](https://taskfile.dev) CLI, and the Wails3 CLI first: + +```sh +brew install go node go-task lefthook golangci-lint +go install github.com/loeffel-io/ls-lint/v2/cmd/ls_lint@v2.3.1 +go install github.com/wailsapp/wails/v3/cmd/wails3@v3.0.0-beta.4 +# make sure $(go env GOPATH)/bin (usually ~/go/bin) is on your PATH + +git clone https://github.com/alicoding/mill.git +cd mill +task setup:hooks # installs Lefthook's pre-commit hooks (mirrors CI) + +# Run it +task dev # starts Mill with hot reload — leave it running +``` + +`task dev` is the way to iterate: frontend edits hot-reload instantly, and +only a Go change triggers a restart. See `CLAUDE.md` for the full set of +build/dev commands (`task install:app`, `task build`, `task package`, ...). + +## Documentation + +- [`docs/SPEC.md`](docs/SPEC.md) — the living architecture and positioning + doc (also rendered inside the app itself). Source of truth for what + Mill is, what's decided (`LOCKED`), and what's still open (`OPEN`). +- [`CONTRIBUTING.md`](CONTRIBUTING.md) — how to propose a change. +- [`SECURITY.md`](SECURITY.md) — how to report a vulnerability and what's + in scope. + +## License + +[Apache-2.0](LICENSE). diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 00000000..f281c029 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,46 @@ +# Security Policy + +## Reporting a vulnerability + +Please report security vulnerabilities through GitHub's private +vulnerability reporting, not a public issue: + +**[Report a vulnerability](https://github.com/alicoding/mill/security/advisories/new)** +(Security tab → "Report a vulnerability") + +This opens a private advisory visible only to you and the maintainer, so +the issue isn't publicly disclosed before a fix ships. Please include +enough detail to reproduce it (steps, affected version/commit, expected +vs. actual behavior). + +## Scope + +Mill is a desktop app that executes guardrailed local commands and +integrations on the user's behalf. Things that are in scope for a security +report: + +- A workflow, trigger, or MCP tool call bypassing the guardrail preview/ + approval step and taking an action without the user seeing it first. +- Secrets (connector credentials, API keys) stored anywhere other than the + OS keychain, or leaking into logs, exported workflows, or the frontend. +- The MCP listener: Mill exposes a local, unauthenticated MCP server bound + to `127.0.0.1` (loopback-only, by design — it never binds a non-loopback + interface and is not reachable off the local machine). A report that it + *is* reachable off-machine, or that a loopback process without the + user's intent can drive it, is in scope. A report that "it has no auth" + on its own is expected/by-design for a loopback listener and not itself + a vulnerability, unless it demonstrates cross-boundary reachability. +- Arbitrary command/code execution reachable without going through the + guardrail (i.e. a path that runs something the user never previewed or + approved). + +Out of scope: vulnerabilities requiring an already-compromised machine, +social engineering, or issues in third-party dependencies without a +demonstrated Mill-specific exploit path (report those upstream; Mill still +wants to know if it makes a dependency's issue reachable in a novel way). + +## Supported versions + +Mill is pre-1.0. There are no released version branches yet — the only +supported line is the latest commit on `main`. Fixes land there; there is +no backport policy until a 1.0 release establishes one. diff --git a/docs/SPEC.md b/docs/SPEC.md index e5a84f16..85e43b4a 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -876,6 +876,19 @@ and are still current** (not Runbook-specific, so they outlived it): (server, since the client can't be trusted), and the zod schema at save time — a canvas can represent shapes the domain can't execute, unlike the old linear-list form. `UX: PROTOTYPE`. +- **`elkjs` license verdict (goal 0028, recorded — not previously + resolved despite being flagged above): EPL-2.0, the license of the + two `elkjs` offers under its dual license, is the one Mill takes.** + `LOCKED`. EPL-2.0 is copyleft on modifications to the covered work + itself, not on separate works that merely link/import it — and Mill + never modifies `elkjs`'s source, only imports the unmodified npm + package. It's also loaded via a dynamic `import()` into its own + separate bundle chunk (not statically bundled into Mill's Apache-2.0 + code), which is the clean case even under EPL-2.0's own stricter + "larger work" language: an unmodified dependency, distributed as its + own separate file, invoked at runtime rather than compiled together. + No Apache-2.0/EPL-2.0 conflict on this shape. No tooling change + follows from this — it's a recorded verdict, not a new check. - **A workflow opens into the canvas via "New workflow" or by editing an existing one, each in its own tab — `CompositionView.tsx`'s tab bar, built on `@primer/react/experimental`'s headless `Tabs` state/ARIA diff --git a/docs/goals/0028-public-repo-hygiene.md b/docs/goals/0028-public-repo-hygiene.md deleted file mode 100644 index c78842d4..00000000 --- a/docs/goals/0028-public-repo-hygiene.md +++ /dev/null @@ -1,50 +0,0 @@ -# 0028 — Public-repo hygiene: the converged-standard baseline - -## Goal -Owner-mandated ("configure rules for anything missing to keep the -codebase tight since we are now on public repo"). Research delivered -2026-08-12 (community-profile score 28%; exposure sweep CLEAN; LICENSE -already correct — Apache-2.0 with reasoning recorded): close the real -gaps, skip the ceremony. - -## Plan (each item cites its standard; the research report is the brief) -1. [ ] README rewrite replacing the Wails scaffold: what Mill is - (SPEC §1's positioning), honest pre-1.0/UX-PROTOTYPE status, git - clone + task setup:hooks + task dev install, pointers to - SPEC/CLAUDE.md (never duplicating them), keep the CI badge. - No screenshots of surfaces SPEC itself tags as PROTOTYPE. -2. [ ] SECURITY.md: GitHub private-vulnerability-reporting as the - channel (enable in repo settings — no email PII); an honest - scope paragraph (guardrailed command execution, keychain secrets, - loopback unauthenticated MCP listener); pre-1.0 rolling-main - support note. -3. [ ] Minimal CONTRIBUTING.md (solo-maintained; CLAUDE.md is the - process; task setup:hooks mirrors CI; issue-before-large-PR). - One minimal bug-report issue template (reuses the build-identity - badge value as the version field — SPEC §3.8's own signal). -4. [ ] OpenSSF Scorecard workflow (official template, scheduled, - README badge) — the repo's ADR-0034 posture should score well - immediately; zero ongoing maintenance. -5. [ ] golangci-lint strengthening, two passes: first - gosec/bodyclose/noctx/revive/unparam (security + HTTP-client - hygiene matching what the code actually does) + full triage; - second pass gocritic/prealloc/contextcheck/sqlclosecheck once - clean. NEVER the `all` preset (ceremony linters fight house - conventions). -6. [ ] dependency-review-action gains deny-licenses (GPL/AGPL - variants) — one line in ci.yml. -7. [ ] The elkjs EPL-2.0 verdict note (SPEC flags it, never - resolves): dynamic-import-as-separate-chunk under EPL-2.0, a - short recorded paragraph. -8. [ ] Cosmetic: de-literal the two /Users/ali paths - (.claude/agents/test-investigator.md, launchatlogin test). - -## Skip list (recorded so nobody re-litigates) -CODE_OF_CONDUCT (until a second contributor exists), PR templates, -FUNDING.yml, go-licenses as standing CI, golangci `all` preset — -each with reasons in the research report. - -## Acceptance -Community profile score jumps; a security researcher knows where to -report and what's in scope; gosec-first-pass clean; a GPL dependency -cannot enter via PR; README describes Mill truthfully. diff --git a/docs/goals/BACKLOG.md b/docs/goals/BACKLOG.md index 0a031793..5adf55a5 100644 --- a/docs/goals/BACKLOG.md +++ b/docs/goals/BACKLOG.md @@ -64,11 +64,19 @@ this pipeline and on this code)** editable "Example: Forward pending approvals" workflow; the decision test written into `.claude/rules/architecture.md`; SettingsView's silent-mount-fetch class fixed alongside. -6. [ ] [0028 — Public-repo hygiene](0028-public-repo-hygiene.md) — - research delivered 2026-08-12 (community-profile score 28%, - exposure sweep clean, LICENSE already correct); the close-the-gaps - build (README/SECURITY/CONTRIBUTING/Scorecard/lint hardening) not - started. +6. [x] [0028 — Public-repo hygiene](archive/0028-public-repo-hygiene.md) + — DELIVERED 2026-08-12: README rewrite (truthful positioning + + the toolchain prerequisites CLAUDE.md itself never states), + SECURITY.md (GitHub PVR enabled live), CONTRIBUTING.md + bug-report + issue template, OpenSSF Scorecard workflow + badge, golangci-lint + first pass (gosec/bodyclose/noctx/revive/unparam) triaged to zero + findings on both build-tag variants — one real bug fixed + (`mcpserving.Serve` missing `ReadHeaderTimeout`), revive's + exported/package-comments rules deliberately disabled (fights + house style); dependency-review deny-licenses; elkjs EPL-2.0 + verdict recorded in SPEC §3; both literal `/Users/ali` paths + de-literalized. Second lint pass (gocritic/prealloc/contextcheck/ + sqlclosecheck) named as explicit future work, not done here. 7. [ ] [0029 — Dev-liveness honesty](0029-dev-liveness-honesty.md) — the DEV·live badge's Go-liveness blind spot, now having claimed a second scalp (ADR-0035's Consequences note); a third badge state diff --git a/docs/goals/archive/0028-public-repo-hygiene.md b/docs/goals/archive/0028-public-repo-hygiene.md new file mode 100644 index 00000000..480e7382 --- /dev/null +++ b/docs/goals/archive/0028-public-repo-hygiene.md @@ -0,0 +1,85 @@ +# 0028 — Public-repo hygiene: the converged-standard baseline + +## Goal +Owner-mandated ("configure rules for anything missing to keep the +codebase tight since we are now on public repo"). Research delivered +2026-08-12 (community-profile score 28%; exposure sweep CLEAN; LICENSE +already correct — Apache-2.0 with reasoning recorded): close the real +gaps, skip the ceremony. + +## Plan (each item cites its standard; the research report is the brief) +1. [x] README rewrite replacing the Wails scaffold: what Mill is + (SPEC §1's positioning), honest pre-1.0/UX-PROTOTYPE status, git + clone + task setup:hooks + task dev install, pointers to + SPEC/CLAUDE.md (never duplicating them), keep the CI badge. + No screenshots of surfaces SPEC itself tags as PROTOTYPE. +2. [x] SECURITY.md: GitHub private-vulnerability-reporting as the + channel (enable in repo settings — no email PII); an honest + scope paragraph (guardrailed command execution, keychain secrets, + loopback unauthenticated MCP listener); pre-1.0 rolling-main + support note. PVR enabled live via `gh api + repos/alicoding/mill/private-vulnerability-reporting -X PUT`, + verified `{"enabled":true}`. +3. [x] Minimal CONTRIBUTING.md (solo-maintained; CLAUDE.md is the + process; task setup:hooks mirrors CI; issue-before-large-PR). + One minimal bug-report issue template (reuses the build-identity + badge value as the version field — SPEC §3.8's own signal). + `.ls-lint.yml`'s root regex extended for SECURITY/CONTRIBUTING + (README's own pattern; CODE_OF_CONDUCT deliberately NOT + pre-added, per the skip list). +4. [x] OpenSSF Scorecard workflow (official template, scheduled, + README badge) — the repo's ADR-0034 posture should score well + immediately; zero ongoing maintenance. Template + badge URL + fetched live from ossf/scorecard-action's own README and + ossf/scorecard's own scorecard-analysis.yml; actions pinned to + the same SHAs this repo's other workflows already use for the + same action versions (verified via `gh api .../tags`, not + assumed). +5. [x] golangci-lint strengthening, first pass: + gosec/bodyclose/noctx/revive/unparam enabled + full triage (see + commit for the itemized findings/fixes — one real bug found and + fixed: `mcpserving.Serve`'s `http.Server` had no + `ReadHeaderTimeout`, a genuine Slowloris exposure even on a + loopback listener). revive's `exported`/`package-comments` rules + disabled in `.golangci.yml` with an inline reason (this repo has + never doc-commented every exported symbol; enforcing it + retroactively is ceremony, not a real finding — bodyclose found + nothing to fix). **Second pass (gocritic/prealloc/contextcheck/ + sqlclosecheck) is explicitly NOT this goal — future work.** +6. [x] dependency-review-action gains deny-licenses (GPL/AGPL + variants) — one line in ci.yml. +7. [x] The elkjs EPL-2.0 verdict note (SPEC flags it, never + resolves): dynamic-import-as-separate-chunk under EPL-2.0, a + short recorded paragraph. +8. [x] Cosmetic: de-literal the two /Users/ali paths + (.claude/agents/test-investigator.md, launchatlogin test). + +## Skip list (recorded so nobody re-litigates) +CODE_OF_CONDUCT (until a second contributor exists), PR templates, +FUNDING.yml, go-licenses as standing CI, golangci `all` preset — +each with reasons in the research report. + +## Acceptance +Community profile score jumps; a security researcher knows where to +report and what's in scope; gosec-first-pass clean; a GPL dependency +cannot enter via PR; README describes Mill truthfully. + +**DELIVERED 2026-08-12** — checked against what shipped: README/ +SECURITY/CONTRIBUTING/issue-template all present and truthful (README +adds the actually-needed toolchain prerequisites CLAUDE.md's own +commands assume but never state — Go/Node/Task/Wails3 CLI — so the +documented install story is real, not aspirational); PVR verified +enabled live (`{"enabled":true}`); Scorecard workflow + badge live, +template/SHAs verified against upstream, not assumed; golangci-lint +first pass (gosec/bodyclose/noctx/revive/unparam) fully triaged to +zero issues on both the `server` and desktop build-tag variants (own +config-behavior check run before trusting the revive rule disables); +one real bug found and fixed along the way +(`mcpserving.Serve`'s missing `ReadHeaderTimeout`); dependency-review +deny-licenses live in ci.yml; elkjs EPL-2.0 verdict recorded in +SPEC.md §3; both literal `/Users/ali` paths de-literalized, with the +touched test re-run. Full local suite (Go, frontend static, e2e) green +via test-investigator — 4 known-flaky e2e specs unrelated to any file +this goal touched (canvas-click timing, resizable-table drag timing, +live-run-state polling, activity-row propagation), reproduced as +flaky (pass-on-retry) in isolation, not a regression. diff --git a/internal/adapters/clipboard/clipboard.go b/internal/adapters/clipboard/clipboard.go index 1505afa3..acf711b1 100644 --- a/internal/adapters/clipboard/clipboard.go +++ b/internal/adapters/clipboard/clipboard.go @@ -5,6 +5,7 @@ package clipboard import ( + "context" "encoding/hex" "fmt" "os/exec" @@ -12,11 +13,18 @@ import ( "time" ) +// cmdTimeout bounds every osascript/pbcopy/pbpaste invocation below -- +// same fail-safe reasoning as mcpclient's own timeout const (docs/SPEC.md +// §8): a hung clipboard subprocess must not hang its caller indefinitely. +const cmdTimeout = 5 * time.Second + // ReadHTML asks macOS for the HTML flavor of the current clipboard // contents. AppleScript returns raw AppleEvent data as a hex-encoded // "«data HTMLxxxx»" literal, so it needs unwrapping before it's usable HTML. func ReadHTML() (string, error) { - out, err := exec.Command("osascript", "-e", "the clipboard as «class HTML»").Output() + ctx, cancel := context.WithTimeout(context.Background(), cmdTimeout) + defer cancel() + out, err := exec.CommandContext(ctx, "osascript", "-e", "the clipboard as «class HTML»").Output() if err != nil { return "", fmt.Errorf("no HTML on clipboard: %w", err) } @@ -36,8 +44,14 @@ func ReadHTML() (string, error) { // HTML flavor from a hex-encoded "«data HTMLxxxx»" literal, the same // encoding it hands back when reading. func WriteHTML(html string) error { + // script is built entirely from a hex encoding of html, which by + // construction contains only [0-9a-f] -- there is no AppleScript + // metacharacter (a literal «, », or quote) html's raw bytes could + // ever inject into the assembled script. script := "set the clipboard to «data HTML" + hex.EncodeToString([]byte(html)) + "»" - if err := exec.Command("osascript", "-e", script).Run(); err != nil { + ctx, cancel := context.WithTimeout(context.Background(), cmdTimeout) + defer cancel() + if err := exec.CommandContext(ctx, "osascript", "-e", script).Run(); err != nil { //nolint:gosec // script is hex-encoded (see above), no injectable characters possible return fmt.Errorf("osascript set-clipboard failed: %w", err) } return nil @@ -45,14 +59,18 @@ func WriteHTML(html string) error { // WriteText sets the clipboard's plain-text flavor via pbcopy. func WriteText(text string) error { - cmd := exec.Command("pbcopy") + ctx, cancel := context.WithTimeout(context.Background(), cmdTimeout) + defer cancel() + cmd := exec.CommandContext(ctx, "pbcopy") cmd.Stdin = strings.NewReader(text) return cmd.Run() } // ReadText reads the clipboard's plain-text flavor via pbpaste. func ReadText() (string, error) { - out, err := exec.Command("pbpaste").Output() + ctx, cancel := context.WithTimeout(context.Background(), cmdTimeout) + defer cancel() + out, err := exec.CommandContext(ctx, "pbpaste").Output() if err != nil { return "", fmt.Errorf("pbpaste failed: %w", err) } @@ -69,7 +87,9 @@ func ReadText() (string, error) { // fallback order already has to answer implicitly, made directly // inspectable. func Info() (string, error) { - out, err := exec.Command("osascript", "-e", "clipboard info").Output() + ctx, cancel := context.WithTimeout(context.Background(), cmdTimeout) + defer cancel() + out, err := exec.CommandContext(ctx, "osascript", "-e", "clipboard info").Output() if err != nil { return "", fmt.Errorf("clipboard info failed: %w", err) } diff --git a/internal/adapters/clipboard/clipboard_test.go b/internal/adapters/clipboard/clipboard_test.go index 213ed01a..a5975eee 100644 --- a/internal/adapters/clipboard/clipboard_test.go +++ b/internal/adapters/clipboard/clipboard_test.go @@ -1,6 +1,7 @@ package clipboard import ( + "context" "os" "os/exec" "runtime" @@ -49,7 +50,9 @@ func TestWriteText(t *testing.T) { t.Fatalf("WriteText() error: %v", err) } - out, err := exec.Command("pbpaste").Output() + ctx, cancel := context.WithTimeout(context.Background(), cmdTimeout) + defer cancel() + out, err := exec.CommandContext(ctx, "pbpaste").Output() if err != nil { t.Fatalf("pbpaste failed while verifying WriteText(): %v", err) } diff --git a/internal/adapters/dockbadge/dockbadge_server.go b/internal/adapters/dockbadge/dockbadge_server.go index 20973cc3..9f89aed7 100644 --- a/internal/adapters/dockbadge/dockbadge_server.go +++ b/internal/adapters/dockbadge/dockbadge_server.go @@ -3,4 +3,4 @@ package dockbadge // Set is a no-op in server mode -- no dock/taskbar icon exists there. -func Set(count int) error { return nil } +func Set(_ int) error { return nil } diff --git a/internal/adapters/execution/execution_test.go b/internal/adapters/execution/execution_test.go index d6b76e0d..95fb1b56 100644 --- a/internal/adapters/execution/execution_test.go +++ b/internal/adapters/execution/execution_test.go @@ -27,7 +27,7 @@ func TestResumeAfterFailure_DoesNotReExecuteCheckpointedStep(t *testing.T) { var step1Runs int32 var step2Attempts int32 - step1 := func(_ context.Context) (string, error) { + step1 := func(_ context.Context) (string, error) { //nolint:unparam // Step[R]'s func(context.Context) (R, error) shape is required by RunAsStep's generic constraint; this step just never fails in this scenario atomic.AddInt32(&step1Runs, 1) return "step1-output", nil } diff --git a/internal/adapters/fileread/fileread.go b/internal/adapters/fileread/fileread.go index 75d928ca..aabe65c3 100644 --- a/internal/adapters/fileread/fileread.go +++ b/internal/adapters/fileread/fileread.go @@ -38,7 +38,12 @@ func Read(path string) (string, error) { return "", fmt.Errorf("fileread: %q is %d bytes, over the %d byte limit", path, info.Size(), MaxBytes) } - data, err := os.ReadFile(path) + // path is a user-configured value (the capture-file node's path, + // which composition.go registers as guardrail.ClassRead) -- reading + // an arbitrary path IS this connector's job; the node's own step + // already goes through Mill's guardrail preview/approval gate before + // this ever runs, so this is not a missing validation to add here. + data, err := os.ReadFile(path) //nolint:gosec // guardrail-gated user-configured path, by design (see comment above) if err != nil { return "", fmt.Errorf("fileread: %w", err) } diff --git a/internal/adapters/fileread/fileread_test.go b/internal/adapters/fileread/fileread_test.go index ebc2a513..df4ec7be 100644 --- a/internal/adapters/fileread/fileread_test.go +++ b/internal/adapters/fileread/fileread_test.go @@ -10,7 +10,7 @@ import ( func TestRead_ReturnsFileContents(t *testing.T) { path := filepath.Join(t.TempDir(), "page.html") const want = "
hello
" - if err := os.WriteFile(path, []byte(want), 0o644); err != nil { + if err := os.WriteFile(path, []byte(want), 0o600); err != nil { t.Fatal(err) } @@ -39,7 +39,7 @@ func TestRead_EmptyPath_Errors(t *testing.T) { func TestRead_OverSizeLimit_Errors(t *testing.T) { path := filepath.Join(t.TempDir(), "huge.html") - f, err := os.Create(path) + f, err := os.Create(path) //nolint:gosec // t.TempDir()-scoped test fixture path, not user input if err != nil { t.Fatal(err) } diff --git a/internal/adapters/filewatch/filewatch_test.go b/internal/adapters/filewatch/filewatch_test.go index b7b42443..b9875533 100644 --- a/internal/adapters/filewatch/filewatch_test.go +++ b/internal/adapters/filewatch/filewatch_test.go @@ -22,7 +22,7 @@ func TestWatch_FiresOnFileCreate(t *testing.T) { } defer func() { _ = b.Close() }() - if err := os.WriteFile(filepath.Join(dir, "new-file.txt"), []byte("hello"), 0o644); err != nil { + if err := os.WriteFile(filepath.Join(dir, "new-file.txt"), []byte("hello"), 0o600); err != nil { t.Fatalf("WriteFile() error: %v", err) } @@ -56,7 +56,7 @@ func TestWatch_CloseStopsFiring(t *testing.T) { t.Fatalf("Close() error: %v", err) } - if err := os.WriteFile(filepath.Join(dir, "after-close.txt"), []byte("hello"), 0o644); err != nil { + if err := os.WriteFile(filepath.Join(dir, "after-close.txt"), []byte("hello"), 0o600); err != nil { t.Fatalf("WriteFile() error: %v", err) } @@ -84,12 +84,12 @@ func TestWatch_PatternFilter(t *testing.T) { defer func() { _ = b.Close() }() // A non-matching file must NOT fire. - if err := os.WriteFile(filepath.Join(dir, "ignore.txt"), []byte("x"), 0o644); err != nil { + if err := os.WriteFile(filepath.Join(dir, "ignore.txt"), []byte("x"), 0o600); err != nil { t.Fatal(err) } // A matching file must fire, delivering its path. mdPath := filepath.Join(dir, "note.md") - if err := os.WriteFile(mdPath, []byte("x"), 0o644); err != nil { + if err := os.WriteFile(mdPath, []byte("x"), 0o600); err != nil { t.Fatal(err) } diff --git a/internal/adapters/hotkey/hotkey_server.go b/internal/adapters/hotkey/hotkey_server.go index ab4023f3..bd3e2711 100644 --- a/internal/adapters/hotkey/hotkey_server.go +++ b/internal/adapters/hotkey/hotkey_server.go @@ -15,7 +15,7 @@ var ErrUnsupportedInServerMode = errors.New("global hotkeys are not available in type Binding struct{} // Bind always fails in server mode; see ErrUnsupportedInServerMode. -func Bind(mods []string, key string) (*Binding, error) { +func Bind(_ []string, _ string) (*Binding, error) { return nil, ErrUnsupportedInServerMode } diff --git a/internal/adapters/httpconnector/httpconnector_test.go b/internal/adapters/httpconnector/httpconnector_test.go index b4bf096b..b362eaf5 100644 --- a/internal/adapters/httpconnector/httpconnector_test.go +++ b/internal/adapters/httpconnector/httpconnector_test.go @@ -9,7 +9,7 @@ import ( ) func TestExecute_GET_ReturnsBodyAndStatus(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(`{"ok":true}`)) })) @@ -101,7 +101,7 @@ func TestExecute_QueryMethod_SendsBody(t *testing.T) { // covers the "pass a non-retried HTTP-level response through as data" // contract composition/integration.go's status check depends on. func TestExecute_NonRetriedStatus_NoError(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusBadRequest) _, _ = w.Write([]byte("boom")) })) @@ -132,7 +132,7 @@ func TestExecute_RetriedStatusExhausted_Errors(t *testing.T) { }) var calls int - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { calls++ w.WriteHeader(http.StatusInternalServerError) _, _ = w.Write([]byte("boom")) diff --git a/internal/adapters/idletime/idletime_desktop.go b/internal/adapters/idletime/idletime_desktop.go index cbe73ae7..199553ee 100644 --- a/internal/adapters/idletime/idletime_desktop.go +++ b/internal/adapters/idletime/idletime_desktop.go @@ -3,6 +3,7 @@ package idletime import ( + "context" "errors" "fmt" "os/exec" @@ -11,6 +12,10 @@ import ( "time" ) +// ioregTimeout bounds the ioreg invocation below -- same fail-safe +// reasoning as clipboard's own cmdTimeout. +const ioregTimeout = 5 * time.Second + // Seconds shells out to `ioreg -c IOHIDSystem` -- the same // shell-out-to-a-real-OS-command pattern internal/adapters/clipboard // already establishes (osascript/pbcopy/pbpaste), zero cgo -- and reads @@ -23,7 +28,9 @@ import ( // IOHIDSystem's own idle-time property is plain IORegistry data any // process can read. func Seconds() (time.Duration, error) { - out, err := exec.Command("ioreg", "-c", "IOHIDSystem").Output() + ctx, cancel := context.WithTimeout(context.Background(), ioregTimeout) + defer cancel() + out, err := exec.CommandContext(ctx, "ioreg", "-c", "IOHIDSystem").Output() if err != nil { return 0, fmt.Errorf("idletime: ioreg: %w", err) } diff --git a/internal/adapters/launchatlogin/launchatlogin_desktop.go b/internal/adapters/launchatlogin/launchatlogin_desktop.go index 8c7b72c6..6295dfb7 100644 --- a/internal/adapters/launchatlogin/launchatlogin_desktop.go +++ b/internal/adapters/launchatlogin/launchatlogin_desktop.go @@ -3,12 +3,18 @@ package launchatlogin import ( + "context" "fmt" "os/exec" "path/filepath" "strings" + "time" ) +// osascriptTimeout bounds every osascript invocation below -- same +// fail-safe reasoning as clipboard's own cmdTimeout. +const osascriptTimeout = 5 * time.Second + // appBundlePath walks up from a running executable's path // (.../Foo.app/Contents/MacOS/Foo) to the .app bundle itself. Returns // ErrNotAppBundle if execPath doesn't have that shape. @@ -38,7 +44,12 @@ func Enable(execPath string) error { `tell application "System Events" to make login item at end with properties {path:%q, hidden:false, name:%q}`, bundlePath, appName(bundlePath), ) - if out, err := exec.Command("osascript", "-e", script).CombinedOutput(); err != nil { + ctx, cancel := context.WithTimeout(context.Background(), osascriptTimeout) + defer cancel() + // bundlePath/appName derive from execPath, which callers pass as + // Mill's own running executable path (os.Executable()), never + // arbitrary external/user input. + if out, err := exec.CommandContext(ctx, "osascript", "-e", script).CombinedOutput(); err != nil { //nolint:gosec // script is built from Mill's own executable path, not external input (see comment above) return fmt.Errorf("osascript enable login item failed: %w (%s)", err, strings.TrimSpace(string(out))) } return nil @@ -52,10 +63,13 @@ func Disable(execPath string) error { return err } script := fmt.Sprintf(`tell application "System Events" to delete login item %q`, appName(bundlePath)) + ctx, cancel := context.WithTimeout(context.Background(), osascriptTimeout) + defer cancel() // System Events errors if the named login item doesn't exist -- // that's the expected "already disabled" case, not a real failure, - // so it's deliberately not surfaced as one. - _ = exec.Command("osascript", "-e", script).Run() + // so it's deliberately not surfaced as one. script is built from + // Mill's own executable path (see Enable's own comment above). + _ = exec.CommandContext(ctx, "osascript", "-e", script).Run() //nolint:gosec // script is built from Mill's own executable path, not external input return nil } @@ -68,7 +82,9 @@ func IsEnabled(execPath string) (bool, error) { } name := appName(bundlePath) script := `tell application "System Events" to get the name of every login item` - out, err := exec.Command("osascript", "-e", script).Output() + ctx, cancel := context.WithTimeout(context.Background(), osascriptTimeout) + defer cancel() + out, err := exec.CommandContext(ctx, "osascript", "-e", script).Output() if err != nil { return false, fmt.Errorf("osascript list login items failed: %w", err) } diff --git a/internal/adapters/launchatlogin/launchatlogin_desktop_test.go b/internal/adapters/launchatlogin/launchatlogin_desktop_test.go index 5e9959c3..05adbe00 100644 --- a/internal/adapters/launchatlogin/launchatlogin_desktop_test.go +++ b/internal/adapters/launchatlogin/launchatlogin_desktop_test.go @@ -18,7 +18,7 @@ func TestAppBundlePath(t *testing.T) { } func TestAppBundlePath_DevBinary_ErrNotAppBundle(t *testing.T) { - _, err := appBundlePath("/Users/ali/code/mill/bin/mill.dev") + _, err := appBundlePath("/tmp/mill-dev-build/bin/mill.dev") if !errors.Is(err, ErrNotAppBundle) { t.Errorf("appBundlePath on a bare dev binary: err = %v, want ErrNotAppBundle", err) } diff --git a/internal/adapters/launchatlogin/launchatlogin_server.go b/internal/adapters/launchatlogin/launchatlogin_server.go index da35820c..b9be7e4a 100644 --- a/internal/adapters/launchatlogin/launchatlogin_server.go +++ b/internal/adapters/launchatlogin/launchatlogin_server.go @@ -3,16 +3,16 @@ package launchatlogin // Enable always fails in server mode; see ErrUnsupportedInServerMode. -func Enable(execPath string) error { +func Enable(_ string) error { return ErrUnsupportedInServerMode } // Disable always fails in server mode; see ErrUnsupportedInServerMode. -func Disable(execPath string) error { +func Disable(_ string) error { return ErrUnsupportedInServerMode } // IsEnabled always fails in server mode; see ErrUnsupportedInServerMode. -func IsEnabled(execPath string) (bool, error) { +func IsEnabled(_ string) (bool, error) { return false, ErrUnsupportedInServerMode } diff --git a/internal/adapters/mcpclient/mcpclient.go b/internal/adapters/mcpclient/mcpclient.go index bce86c8a..17615939 100644 --- a/internal/adapters/mcpclient/mcpclient.go +++ b/internal/adapters/mcpclient/mcpclient.go @@ -43,7 +43,14 @@ func ListTools(command string, args []string) ([]Tool, error) { ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() - transport := &mcp.CommandTransport{Command: exec.Command(command, args...)} + // command/args are the user's own MCP-server-connector configuration + // (Configure), the deliberate feature -- launching a user-chosen + // executable, the same class as shellenv's own login-shell launch. + // exec.Command never invokes a shell (argv is passed directly to + // execve), so there's no shell-metacharacter injection surface + // either; the risk here is "runs what the user configured," which is + // the intended behavior, not a vulnerability to close. + transport := &mcp.CommandTransport{Command: exec.CommandContext(ctx, command, args...)} //nolint:gosec // user-configured connector command, by design (see comment above) return listTools(ctx, transport) } @@ -56,7 +63,14 @@ func CallTool(command string, args []string, toolName string, arguments map[stri ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() - transport := &mcp.CommandTransport{Command: exec.Command(command, args...)} + // command/args are the user's own MCP-server-connector configuration + // (Configure), the deliberate feature -- launching a user-chosen + // executable, the same class as shellenv's own login-shell launch. + // exec.Command never invokes a shell (argv is passed directly to + // execve), so there's no shell-metacharacter injection surface + // either; the risk here is "runs what the user configured," which is + // the intended behavior, not a vulnerability to close. + transport := &mcp.CommandTransport{Command: exec.CommandContext(ctx, command, args...)} //nolint:gosec // user-configured connector command, by design (see comment above) return callTool(ctx, transport, toolName, arguments) } diff --git a/internal/adapters/mcpserving/mcpserving.go b/internal/adapters/mcpserving/mcpserving.go index 70a9ece8..30688e30 100644 --- a/internal/adapters/mcpserving/mcpserving.go +++ b/internal/adapters/mcpserving/mcpserving.go @@ -13,10 +13,18 @@ package mcpserving import ( "net/http" + "time" "github.com/modelcontextprotocol/go-sdk/mcp" ) +// readHeaderTimeout bounds how long the server waits to read a request's +// headers -- a real gosec finding (G112), not a false positive: even a +// loopback-only listener is reachable by any other local process, and an +// http.Server with no ReadHeaderTimeout is vulnerable to a slow-headers +// (Slowloris-style) connection-exhaustion attack from one such process. +const readHeaderTimeout = 5 * time.Second + // New constructs an MCP server with the given name/version identity. // Thin on purpose -- there's little to wrap here (the SDK's // *mcp.Server already has the right shape), this exists for the import @@ -36,7 +44,7 @@ func New(name, version string) *mcp.Server { // already returned. func Serve(addr string, server *mcp.Server) (*http.Server, <-chan error) { handler := mcp.NewStreamableHTTPHandler(func(*http.Request) *mcp.Server { return server }, nil) - httpServer := &http.Server{Addr: addr, Handler: handler} + httpServer := &http.Server{Addr: addr, Handler: handler, ReadHeaderTimeout: readHeaderTimeout} errCh := make(chan error, 1) go func() { if err := httpServer.ListenAndServe(); err != nil && err != http.ErrServerClosed { diff --git a/internal/adapters/notify/notify.go b/internal/adapters/notify/notify.go index 2cea3e1c..6a555688 100644 --- a/internal/adapters/notify/notify.go +++ b/internal/adapters/notify/notify.go @@ -18,7 +18,7 @@ const ( DenyActionID = "deny" // CategoryMCPWrite groups the two actions above under one reusable // category, registered once at startup (Start). - CategoryMCPWrite = "mill-mcp-write" + CategoryMCPWrite = "mill-mcp-write" //nolint:gosec // an OS-notification category name, not a credential (G101 false positive) // DefaultActionID mirrors notifications.DefaultActionIdentifier -- // the ActionIdentifier a Response carries when the user clicked the // notification body itself, not an action button. Hardcoded rather diff --git a/internal/adapters/notify/notify_server.go b/internal/adapters/notify/notify_server.go index a2f7b5a8..9d07df39 100644 --- a/internal/adapters/notify/notify_server.go +++ b/internal/adapters/notify/notify_server.go @@ -14,10 +14,10 @@ var ErrUnsupportedInServerMode = errors.New("OS notifications are not available func Start() error { return ErrUnsupportedInServerMode } // OnResponse is a no-op in server mode -- nothing ever calls callback. -func OnResponse(callback func(Response)) {} +func OnResponse(_ func(Response)) {} // SendActionable is a no-op in server mode. -func SendActionable(id, title, body string) error { return ErrUnsupportedInServerMode } +func SendActionable(_, _, _ string) error { return ErrUnsupportedInServerMode } // SendPlain is a no-op in server mode. -func SendPlain(id, title, body string) error { return ErrUnsupportedInServerMode } +func SendPlain(_, _, _ string) error { return ErrUnsupportedInServerMode } diff --git a/internal/adapters/procexec/procexec.go b/internal/adapters/procexec/procexec.go index 5974625a..de4e9184 100644 --- a/internal/adapters/procexec/procexec.go +++ b/internal/adapters/procexec/procexec.go @@ -170,7 +170,13 @@ func Start(spec Spec) (*Handle, error) { grace = defaultGrace } - cmd := exec.Command(spec.Argv[0], spec.Argv[1:]...) //nolint:gosec // Argv is caller-controlled per Spec's own doc; this package execs, it doesn't parse a shell string + // Deliberately exec.Command, not exec.CommandContext: this package's + // own doc above states its whole reason for existing -- it owns + // process supervision itself (graceful signal, then escalate to + // SIGKILL after GracePeriod, via kill.go's process-group kill), never + // a context's own immediate cancel-means-SIGKILL. Wiring a context in + // here would fight that design, not fix a gap. + cmd := exec.Command(spec.Argv[0], spec.Argv[1:]...) //nolint:gosec,noctx // Argv is caller-controlled per Spec's own doc, and lifecycle is this package's own job (see comment above) cmd.Dir = spec.Dir cmd.Env = spec.Env cmd.SysProcAttr = setpgidAttr() diff --git a/internal/adapters/settings/settings.go b/internal/adapters/settings/settings.go index 97a9f722..f18ee04d 100644 --- a/internal/adapters/settings/settings.go +++ b/internal/adapters/settings/settings.go @@ -33,7 +33,7 @@ type Store interface { // install it won't exist yet. A missing file is not an error (first run); // Load only fails on a genuinely corrupt/unreadable file. func New(filename string) (*kvstore.KVStoreService, error) { - if err := os.MkdirAll(filepath.Dir(filename), 0o755); err != nil { + if err := os.MkdirAll(filepath.Dir(filename), 0o750); err != nil { return nil, fmt.Errorf("creating settings directory: %w", err) } store := kvstore.NewWithConfig(&kvstore.Config{ diff --git a/internal/adapters/shellenv/shellenv.go b/internal/adapters/shellenv/shellenv.go index d042ae07..1907b827 100644 --- a/internal/adapters/shellenv/shellenv.go +++ b/internal/adapters/shellenv/shellenv.go @@ -7,6 +7,8 @@ package shellenv import ( + "context" + "errors" "fmt" "os" "os/exec" @@ -14,6 +16,11 @@ import ( "time" ) +// captureTimeout bounds CapturePath's shell invocation -- a broken shell +// profile can hang (a profile that prompts, an `exec` loop), and this +// must not hang its caller (the RPC) indefinitely. +const captureTimeout = 10 * time.Second + // CapturePath runs the user's login shell (from $SHELL, falling back // to macOS's default /bin/zsh) with -l -c so the login startup files // run exactly as a real terminal's would, and returns the resulting @@ -26,23 +33,20 @@ func CapturePath() (string, error) { if shell == "" { shell = "/bin/zsh" } - cmd := exec.Command(shell, "-l", "-c", `printf %s "$PATH"`) - // A broken shell profile can hang (a profile that prompts, an - // `exec` loop) -- bound it rather than hanging the RPC. - done := make(chan struct{}) - var out []byte - var err error - go func() { - out, err = cmd.Output() - close(done) - }() - select { - case <-done: - case <-time.After(10 * time.Second): - _ = cmd.Process.Kill() - return "", fmt.Errorf("shell %s did not produce a PATH within 10s -- a login profile may be hanging", shell) - } + ctx, cancel := context.WithTimeout(context.Background(), captureTimeout) + defer cancel() + // shell comes from the user's own $SHELL (or the macOS default) -- + // this process already runs with the user's own privileges, so + // there's no privilege boundary being crossed by running their own + // configured shell; exec.CommandContext never invokes a shell of its + // own (argv goes straight to execve), so there's no injection + // surface via the fixed "-l"/"-c"/printf arguments either. + cmd := exec.CommandContext(ctx, shell, "-l", "-c", `printf %s "$PATH"`) //nolint:gosec // runs the user's own $SHELL with the user's own privileges, by design + out, err := cmd.Output() if err != nil { + if errors.Is(ctx.Err(), context.DeadlineExceeded) { + return "", fmt.Errorf("shell %s did not produce a PATH within %s -- a login profile may be hanging", shell, captureTimeout) + } return "", fmt.Errorf("running %s -l failed: %w", shell, err) } path := strings.TrimSpace(string(out)) diff --git a/internal/domain/composition/authapikey.go b/internal/domain/composition/authapikey.go index 882083ba..935f47cd 100644 --- a/internal/domain/composition/authapikey.go +++ b/internal/domain/composition/authapikey.go @@ -9,7 +9,7 @@ import ( // AuthAPIKey sets X-Api-Key -- migrated verbatim from the original // AuthHeader switch (ADR-0015), byte-identical behavior. func init() { - RegisterAuthStrategy(httprequest.AuthAPIKey, func(rc ResolvedHTTPRequest, method, path string, headers map[string]string, query url.Values, body string) error { + RegisterAuthStrategy(httprequest.AuthAPIKey, func(rc ResolvedHTTPRequest, _, _ string, headers map[string]string, _ url.Values, _ string) error { headers["X-Api-Key"] = rc.Secret return nil }) diff --git a/internal/domain/composition/authbearer.go b/internal/domain/composition/authbearer.go index daa4126c..098910c4 100644 --- a/internal/domain/composition/authbearer.go +++ b/internal/domain/composition/authbearer.go @@ -10,7 +10,7 @@ import ( // from the original AuthHeader switch (ADR-0015), byte-identical // behavior. func init() { - RegisterAuthStrategy(httprequest.AuthBearer, func(rc ResolvedHTTPRequest, method, path string, headers map[string]string, query url.Values, body string) error { + RegisterAuthStrategy(httprequest.AuthBearer, func(rc ResolvedHTTPRequest, _, _ string, headers map[string]string, _ url.Values, _ string) error { headers["Authorization"] = "Bearer " + rc.Secret return nil }) diff --git a/internal/domain/composition/authhmac.go b/internal/domain/composition/authhmac.go index c91bec43..7b13b783 100644 --- a/internal/domain/composition/authhmac.go +++ b/internal/domain/composition/authhmac.go @@ -22,7 +22,7 @@ import ( // future work, §3.2's own "extend when a real request needs it" // principle applied here the same as everywhere else. func init() { - RegisterAuthStrategy(httprequest.AuthHMAC, func(rc ResolvedHTTPRequest, method, path string, headers map[string]string, query url.Values, body string) error { + RegisterAuthStrategy(httprequest.AuthHMAC, func(rc ResolvedHTTPRequest, method, path string, headers map[string]string, _ url.Values, body string) error { headerName := "X-Signature" if rc.Auth != nil && rc.Auth.HMAC != nil && rc.Auth.HMAC.HeaderName != "" { headerName = rc.Auth.HMAC.HeaderName diff --git a/internal/domain/composition/authmtls.go b/internal/domain/composition/authmtls.go index 3353d062..bfbc35f5 100644 --- a/internal/domain/composition/authmtls.go +++ b/internal/domain/composition/authmtls.go @@ -16,7 +16,7 @@ import ( // Certificates + software.sslmate.com/src/go-pkcs12 for P12 decoding, // per docs/SPEC.md §4.1's research) is real future work. func init() { - RegisterAuthStrategy(httprequest.AuthMTLS, func(rc ResolvedHTTPRequest, method, path string, headers map[string]string, query url.Values, body string) error { + RegisterAuthStrategy(httprequest.AuthMTLS, func(_ ResolvedHTTPRequest, _, _ string, _ map[string]string, _ url.Values, _ string) error { return fmt.Errorf("mTLS is not yet implemented (docs/adr/0015) -- deliberately deferred, not a bug") }) } diff --git a/internal/domain/composition/authnone.go b/internal/domain/composition/authnone.go index 95c09edd..95228a3f 100644 --- a/internal/domain/composition/authnone.go +++ b/internal/domain/composition/authnone.go @@ -9,7 +9,7 @@ import ( // AuthNone adds nothing to the request -- migrated from the original // AuthHeader switch's default case (ADR-0015), byte-identical behavior. func init() { - RegisterAuthStrategy(httprequest.AuthNone, func(rc ResolvedHTTPRequest, method, path string, headers map[string]string, query url.Values, body string) error { + RegisterAuthStrategy(httprequest.AuthNone, func(_ ResolvedHTTPRequest, _, _ string, _ map[string]string, _ url.Values, _ string) error { return nil }) } diff --git a/internal/domain/composition/authoauth1.go b/internal/domain/composition/authoauth1.go index ca357607..28562d59 100644 --- a/internal/domain/composition/authoauth1.go +++ b/internal/domain/composition/authoauth1.go @@ -82,7 +82,7 @@ func oauth1Nonce() string { // resulting credentials as an "Authorization: OAuth ..." header per // §3.5.1 (the header transport, the most common of RFC 5849's three). func init() { - RegisterAuthStrategy(httprequest.AuthOAuth1, func(rc ResolvedHTTPRequest, method, path string, headers map[string]string, query url.Values, body string) error { + RegisterAuthStrategy(httprequest.AuthOAuth1, func(rc ResolvedHTTPRequest, method, path string, headers map[string]string, query url.Values, _ string) error { var cfg httprequest.OAuth1Config if rc.Auth != nil && rc.Auth.OAuth1 != nil { cfg = *rc.Auth.OAuth1 diff --git a/internal/domain/composition/authoauth1vendor.go b/internal/domain/composition/authoauth1vendor.go index ba97cec0..f4535eda 100644 --- a/internal/domain/composition/authoauth1vendor.go +++ b/internal/domain/composition/authoauth1vendor.go @@ -20,7 +20,7 @@ import ( // new AuthType is a pure addition, zero changes to any other strategy // file, whether or not that type's own behavior is fully built yet. func init() { - RegisterAuthStrategy(httprequest.AuthOAuth1Vendor, func(rc ResolvedHTTPRequest, method, path string, headers map[string]string, query url.Values, body string) error { + RegisterAuthStrategy(httprequest.AuthOAuth1Vendor, func(_ ResolvedHTTPRequest, _, _ string, _ map[string]string, _ url.Values, _ string) error { return fmt.Errorf("the vendor-specific OAuth 1.0a variant is not yet implemented -- its exact signing convention was never confirmed (docs/adr/0015); use AuthOAuth1 (standard RFC 5849) if that fits, or file the real requirement") }) } diff --git a/internal/domain/composition/authqueryparam.go b/internal/domain/composition/authqueryparam.go index a7e01650..ef10a4f4 100644 --- a/internal/domain/composition/authqueryparam.go +++ b/internal/domain/composition/authqueryparam.go @@ -14,7 +14,7 @@ import ( // vendor needing a different query-param name is real future work, not // solved speculatively here). func init() { - RegisterAuthStrategy(httprequest.AuthQueryParam, func(rc ResolvedHTTPRequest, method, path string, headers map[string]string, query url.Values, body string) error { + RegisterAuthStrategy(httprequest.AuthQueryParam, func(rc ResolvedHTTPRequest, _, _ string, _ map[string]string, query url.Values, _ string) error { query.Set("apikey", rc.Secret) return nil }) diff --git a/internal/domain/composition/capturefile_test.go b/internal/domain/composition/capturefile_test.go index 00a5abc7..c4238539 100644 --- a/internal/domain/composition/capturefile_test.go +++ b/internal/domain/composition/capturefile_test.go @@ -13,7 +13,7 @@ import ( // §3.4). func TestCaptureFile_PayloadMode_ReadsPathFromPayload(t *testing.T) { path := filepath.Join(t.TempDir(), "page.html") - if err := os.WriteFile(path, []byte("payload-mode contents"), 0o644); err != nil { + if err := os.WriteFile(path, []byte("payload-mode contents"), 0o600); err != nil { t.Fatal(err) } @@ -40,7 +40,7 @@ func TestCaptureFile_PayloadMode_ReadsPathFromPayload(t *testing.T) { // instead. func TestCaptureFile_LiteralMode_ReadsConfiguredPath(t *testing.T) { path := filepath.Join(t.TempDir(), "page.html") - if err := os.WriteFile(path, []byte("literal-mode contents"), 0o644); err != nil { + if err := os.WriteFile(path, []byte("literal-mode contents"), 0o600); err != nil { t.Fatal(err) } diff --git a/internal/domain/composition/childworkflow.go b/internal/domain/composition/childworkflow.go index 504ededa..d91f01b2 100644 --- a/internal/domain/composition/childworkflow.go +++ b/internal/domain/composition/childworkflow.go @@ -23,7 +23,7 @@ import ( // (WithWorkflowID): re-invoking with the same key returns the child's // already-recorded result instead of re-running it. Empty means "a // fresh run every time," the same default every other run gets. -var runChildWorkflowFn = func(runCtx any, workflowID string, attrValues map[string]string, idempotencyKey string, pinnedVersion int) (string, error) { +var runChildWorkflowFn = func(_ any, workflowID string, _ map[string]string, _ string, _ int) (string, error) { return "", fmt.Errorf("no child-workflow runner registered (yet) for workflow %q", workflowID) } @@ -37,7 +37,7 @@ func SetChildWorkflowRunner(fn func(runCtx any, workflowID string, attrValues ma func init() { RegisterNodeType(NodeType{ ID: "child-workflow", Kind: KindProcess, - Label: "Run another workflow", + Label: "Run another workflow", // Effect is explicitly ClassNone (never left at the zero value) -- // docs/adr/0022: "Child workflows carry no class of their own // (none): the child's own steps are gated inside the child's own diff --git a/internal/domain/composition/codeexec.go b/internal/domain/composition/codeexec.go index a068690b..3b7cc7ac 100644 --- a/internal/domain/composition/codeexec.go +++ b/internal/domain/composition/codeexec.go @@ -82,7 +82,7 @@ func SetCodeRunner(fn func(procexec.Spec) (*procexec.Handle, error)) { // here (waitForApprovalFn's own park/resume doesn't need one since it // blocks synchronously, but a live process handle needs to be // reachable from OUTSIDE this call while it's still running). -var registerRunningProcessFn = func(runCtx any, nodeID string, h *procexec.Handle) func() { +var registerRunningProcessFn = func(_ any, _ string, _ *procexec.Handle) func() { return func() {} } diff --git a/internal/domain/composition/codeexec_test.go b/internal/domain/composition/codeexec_test.go index c74fc0cd..6277a951 100644 --- a/internal/domain/composition/codeexec_test.go +++ b/internal/domain/composition/codeexec_test.go @@ -18,10 +18,9 @@ func swapExecEnvLookupForTest(t *testing.T, fn func(id string) (ResolvedExecEnv, t.Cleanup(func() { lookupExecEnvFn = orig }) } -func testExecEnv(t *testing.T, extraEnv ...string) ResolvedExecEnv { +func testExecEnv(t *testing.T) ResolvedExecEnv { t.Helper() - env := append([]string{"PATH=/bin:/usr/bin"}, extraEnv...) - return ResolvedExecEnv{Shell: "sh", ProfileMode: "clean", Dir: t.TempDir(), Env: env} + return ResolvedExecEnv{Shell: "sh", ProfileMode: "clean", Dir: t.TempDir(), Env: []string{"PATH=/bin:/usr/bin"}} } func runCodeExecution(t *testing.T, node Node, payload string) (ExecContext, error) { diff --git a/internal/domain/composition/humanreview.go b/internal/domain/composition/humanreview.go index c6add74f..a4f9c32e 100644 --- a/internal/domain/composition/humanreview.go +++ b/internal/domain/composition/humanreview.go @@ -21,7 +21,7 @@ import ( // main.go to the same durable DBOS park the ambient gate uses // (executionservice_guardrail.go), so both patterns share one pending/ // approve/deny surface. Same injected-seam shape as runChildWorkflowFn. -var waitForApprovalFn = func(runCtx any, node Node, ec ExecContext, message string) (map[string]string, error) { +var waitForApprovalFn = func(_ any, _ Node, _ ExecContext, _ string) (map[string]string, error) { return nil, fmt.Errorf("no approval waiter registered -- this run has no interactive context to ask in") } diff --git a/internal/domain/composition/integrationexec_test.go b/internal/domain/composition/integrationexec_test.go index b1b14edc..160c1960 100644 --- a/internal/domain/composition/integrationexec_test.go +++ b/internal/domain/composition/integrationexec_test.go @@ -115,7 +115,7 @@ func TestExecuteWorkflow_IntegrationHTTP_QueryMethod_Accepted(t *testing.T) { } func TestExecuteWorkflow_IntegrationHTTP_UnknownHTTPRequest_Rejected(t *testing.T) { - withHTTPRequestLookup(t, func(id string) (ResolvedHTTPRequest, error) { + withHTTPRequestLookup(t, func(_ string) (ResolvedHTTPRequest, error) { return ResolvedHTTPRequest{}, errors.New("no such request") }) diff --git a/internal/domain/composition/seedproof_test.go b/internal/domain/composition/seedproof_test.go index d65ef7f5..9db5225b 100644 --- a/internal/domain/composition/seedproof_test.go +++ b/internal/domain/composition/seedproof_test.go @@ -212,9 +212,9 @@ var nodeTypeProofRegistry = map[string]seedProof{ // wasn't updated to match). func checkRegistry(t *testing.T, kind string, realIDs []string, registry map[string]seedProof) { t.Helper() - real := make(map[string]bool, len(realIDs)) + realSet := make(map[string]bool, len(realIDs)) for _, id := range realIDs { - real[id] = true + realSet[id] = true p, ok := registry[id] if !ok { t.Errorf("%s %q has no seedProof registry entry -- add one to seedproof_test.go naming its proof tests (proven(...)) or a ManualOnly reason", kind, id) @@ -225,7 +225,7 @@ func checkRegistry(t *testing.T, kind string, realIDs []string, registry map[str } } for id := range registry { - if !real[id] { + if !realSet[id] { t.Errorf("seedProof registry has an entry for %q, but no such %s currently exists -- remove the stale entry (or the artifact was renamed and the registry key needs updating)", id, kind) } } diff --git a/internal/domain/httprequest/builtin.go b/internal/domain/httprequest/builtin.go index 28ed5287..6e2f72c0 100644 --- a/internal/domain/httprequest/builtin.go +++ b/internal/domain/httprequest/builtin.go @@ -40,9 +40,9 @@ const ( // the COMPLETE endpoint URL and its single operation's path is "/", // which appends nothing at execution -- one URL, one place, matching // the request form's own single URL field. -func openAPISpecFor(title, path string) string { +func openAPISpecFor(title string) string { return `{"openapi":"3.0.3","info":{"title":"` + title + `","version":"1.0.0"},` + - `"paths":{"` + path + `":{"get":{"summary":"` + title + `","responses":{"200":{"description":"OK"}}}}}}` + `"paths":{"/":{"get":{"summary":"` + title + `","responses":{"200":{"description":"OK"}}}}}}` } // Two seeds carry a real *typed* schema (input parameters + typed @@ -100,7 +100,7 @@ func BuiltIn() []HTTPRequest { "validate the key's value (httpbin has no concept of a 'correct' key), so this is a " + "self-consistency check, not third-party-verified auth.", BaseURL: "https://httpbin.org/headers", AuthType: AuthAPIKey, Method: "GET", - OpenAPISpec: openAPISpecFor("httpbin headers echo", "/"), + OpenAPISpec: openAPISpecFor("httpbin headers echo"), BuiltIn: true, }, { @@ -121,7 +121,7 @@ func BuiltIn() []HTTPRequest { "the signed headers back so you can see them, but doesn't verify the signature -- " + "self-consistency check only, same caveat as the API-key example above.", BaseURL: "https://httpbin.org/headers", AuthType: AuthHMAC, Method: "GET", - OpenAPISpec: openAPISpecFor("httpbin headers echo (HMAC)", "/"), + OpenAPISpec: openAPISpecFor("httpbin headers echo (HMAC)"), BuiltIn: true, }, { @@ -133,7 +133,7 @@ func BuiltIn() []HTTPRequest { "successful\"} before this was seeded, not just self-consistent with Mill's own tests.", BaseURL: "https://postman-echo.com/oauth1", AuthType: AuthOAuth1, Method: "GET", Auth: &AuthConfig{OAuth1: &OAuth1Config{ConsumerKey: "RKCGzna7bv9YD57c"}}, - OpenAPISpec: openAPISpecFor("Postman Echo OAuth1", "/"), + OpenAPISpec: openAPISpecFor("Postman Echo OAuth1"), BuiltIn: true, }, { @@ -144,10 +144,10 @@ func BuiltIn() []HTTPRequest { "own repo will never carry a real client secret. Register a free Spotify developer " + "app and fill in the Client ID/Secret yourself to make this one actually run.", BaseURL: "https://api.spotify.com/v1/browse/new-releases", AuthType: AuthOAuth2, Method: "GET", - Auth: &AuthConfig{OAuth2: &OAuth2Config{ + Auth: &AuthConfig{OAuth2: &OAuth2Config{ //nolint:gosec // TokenURL below is a public token endpoint URL, not a credential (G101 false positive) -- this seed ships with no Client ID/Secret, see Description above GrantType: "client_credentials", TokenURL: "https://accounts.spotify.com/api/token", }}, - OpenAPISpec: openAPISpecFor("Spotify Web API (bring your own app)", "/"), + OpenAPISpec: openAPISpecFor("Spotify Web API (bring your own app)"), BuiltIn: true, }, { @@ -156,7 +156,7 @@ func BuiltIn() []HTTPRequest { "which echoes back the query it received -- same self-consistency-only caveat as the " + "header-based API-key example (httpbin doesn't validate the value).", BaseURL: "https://httpbin.org/get", AuthType: AuthQueryParam, Method: "GET", - OpenAPISpec: openAPISpecFor("httpbin query echo", "/"), + OpenAPISpec: openAPISpecFor("httpbin query echo"), BuiltIn: true, }, } diff --git a/internal/services/configuresvc/configureservice_builtin.go b/internal/services/configuresvc/configureservice_builtin.go index f0da371b..b9cb3450 100644 --- a/internal/services/configuresvc/configureservice_builtin.go +++ b/internal/services/configuresvc/configureservice_builtin.go @@ -44,7 +44,7 @@ var builtInSecrets = map[string]string{ // with this exact credential returned {"status":"pass","message": // "OAuth-1.0a signature verification was successful"} from the // server's own side, not just self-consistent with Mill's own tests. -const builtInOAuth1ConsumerSecret = `D+EdQ-gs$-%@2Nu7` +const builtInOAuth1ConsumerSecret = `D+EdQ-gs$-%@2Nu7` //nolint:gosec // Postman's own published, intentionally public test credential -- not a real secret (see doc comment above) // seedBuiltInSecrets writes each seeded example's demo secret into the // OS keychain -- called only from ConfigureService.restore() on a diff --git a/internal/services/configuresvc/configureservice_requestauth_test.go b/internal/services/configuresvc/configureservice_requestauth_test.go index 06cdd21f..a1de0de1 100644 --- a/internal/services/configuresvc/configureservice_requestauth_test.go +++ b/internal/services/configuresvc/configureservice_requestauth_test.go @@ -233,7 +233,7 @@ func TestCreateHTTPRequest_AuthConfig_PersistsAndSurvivesRestore(t *testing.T) { // (configureservice_test.go) for why. cfg.requests = nil - auth := &httprequest.AuthConfig{OAuth2: &httprequest.OAuth2Config{ + auth := &httprequest.AuthConfig{OAuth2: &httprequest.OAuth2Config{ //nolint:gosec // TokenURL below is a fixture URL, not a credential (G101 false positive) GrantType: "client_credentials", TokenURL: "https://auth.example.com/token", ClientID: "client-1", Scope: "read", }} req, err := cfg.CreateHTTPRequest("OAuth2 API", "https://example.com", "", "", httprequest.AuthOAuth2, nil, "", auth, nil, "") diff --git a/internal/services/executionsvc/executionservice_guardrail_test.go b/internal/services/executionsvc/executionservice_guardrail_test.go index 7081247c..4f791b35 100644 --- a/internal/services/executionsvc/executionservice_guardrail_test.go +++ b/internal/services/executionsvc/executionservice_guardrail_test.go @@ -28,7 +28,7 @@ func init() { }) } -func newGuardedHarness(t *testing.T) (*compositionsvc.CompositionService, *guardrailsvc.GuardrailService, *ExecutionService, string) { +func newGuardedHarness(t *testing.T) (*guardrailsvc.GuardrailService, *ExecutionService, string) { t.Helper() store := servicetest.NewFakeStore() comp := compositionsvc.NewCompositionService(store) @@ -47,7 +47,7 @@ func newGuardedHarness(t *testing.T) (*compositionsvc.CompositionService, *guard if err != nil { t.Fatalf("CreateWorkflow: %v", err) } - return comp, guard, exec, wf.ID + return guard, exec, wf.ID } func waitFor[T any](t *testing.T, what string, timeout time.Duration, poll func() (T, bool)) T { @@ -69,7 +69,7 @@ func waitFor[T any](t *testing.T, what string, timeout time.Duration, poll func( // hangs on a human), advertises exactly which step wants to run, and a // deny fails the run closed with the reason recorded. func TestGuardrail_ExternalStepParks_DenyFailsClosed(t *testing.T) { - _, _, exec, wfID := newGuardedHarness(t) + _, exec, wfID := newGuardedHarness(t) summary, err := exec.RunWorkflow(wfID, RunKindTest, nil) if err != nil { @@ -124,7 +124,7 @@ func TestGuardrail_ExternalStepParks_DenyFailsClosed(t *testing.T) { // The full ask flow, approved: the parked step executes after approval // and the run completes with the step's real output. func TestGuardrail_ExternalStepParks_ApproveExecutes(t *testing.T) { - _, _, exec, wfID := newGuardedHarness(t) + _, exec, wfID := newGuardedHarness(t) summary, err := exec.RunWorkflow(wfID, RunKindTest, nil) if err != nil { @@ -175,7 +175,7 @@ func TestGuardrail_ExternalStepParks_ApproveExecutes(t *testing.T) { // opt-in): the run completes synchronously, and the recorded verdict // names the rule that skipped it -- the "skipped" state made auditable. func TestGuardrail_AllowRuleSkipsApproval(t *testing.T) { - _, guard, exec, wfID := newGuardedHarness(t) + guard, exec, wfID := newGuardedHarness(t) rule, err := guard.CreateRule(guardrail.Rule{ Label: "echo is trusted", Effect: guardrail.EffectAllow, NodeTypeID: "test-external-echo", @@ -207,7 +207,7 @@ func TestGuardrail_AllowRuleSkipsApproval(t *testing.T) { // A deny rule fails the run immediately -- no park, no approval option. func TestGuardrail_DenyRuleFailsImmediately(t *testing.T) { - _, guard, exec, wfID := newGuardedHarness(t) + guard, exec, wfID := newGuardedHarness(t) if _, err := guard.CreateRule(guardrail.Rule{ Label: "no external calls", Effect: guardrail.EffectDeny, NodeTypeID: "test-external-echo", @@ -235,7 +235,7 @@ func TestGuardrail_DenyRuleFailsImmediately(t *testing.T) { // §8's locked testability requirement, checked against both the default // and a rule-driven outcome. func TestGuardrail_TestRulesMatchesLiveVerdict(t *testing.T) { - _, guard, _, wfID := newGuardedHarness(t) + guard, _, wfID := newGuardedHarness(t) res, err := guard.TestRules(wfID, "n1") if err != nil { diff --git a/internal/services/executionsvc/initialpayload_test.go b/internal/services/executionsvc/initialpayload_test.go index 0ea68dbe..9a770471 100644 --- a/internal/services/executionsvc/initialpayload_test.go +++ b/internal/services/executionsvc/initialpayload_test.go @@ -30,7 +30,7 @@ func TestRunWorkflowWithPayload_TestKind_FlowsIntoCaptureFile(t *testing.T) { } path := filepath.Join(t.TempDir(), "page.html") - if err := os.WriteFile(path, []byte("
hello payload
"), 0o644); err != nil { + if err := os.WriteFile(path, []byte("
hello payload
"), 0o600); err != nil { t.Fatal(err) } diff --git a/internal/services/mcpsvc/millmcpservice_approval.go b/internal/services/mcpsvc/millmcpservice_approval.go index 30e83e59..d8b0e409 100644 --- a/internal/services/mcpsvc/millmcpservice_approval.go +++ b/internal/services/mcpsvc/millmcpservice_approval.go @@ -27,7 +27,7 @@ import ( // still needs a human click. Defaults to REQUIRED when unset -- enabling // writes must not silently mean unattended writes (§8's fail-safe // default); relaxing to unattended is its own explicit opt-out. -const MCPWriteApprovalKey = "mcp-write-approval-required" +const MCPWriteApprovalKey = "mcp-write-approval-required" //nolint:gosec // a settings-store key name, not a credential (G101 false positive) // mcpPendingWritesKey persists every MCPWriteRecord (pending and, for // 24h, resolved) as one JSON blob -- same one-atomic-blob-per-key shape @@ -469,4 +469,3 @@ func (m *MillMCPService) loadWrites() { // that expires from then on. _ = m.sweepLocked(time.Now()) } - diff --git a/internal/services/mcpsvc/millmcpservice_payload_test.go b/internal/services/mcpsvc/millmcpservice_payload_test.go index 0b9a94c5..0107462b 100644 --- a/internal/services/mcpsvc/millmcpservice_payload_test.go +++ b/internal/services/mcpsvc/millmcpservice_payload_test.go @@ -48,7 +48,7 @@ func TestMCPRunWorkflow_PayloadFlowsIntoCaptureFile(t *testing.T) { } path := filepath.Join(t.TempDir(), "page.html") - if err := os.WriteFile(path, []byte("
payload over MCP works
"), 0o644); err != nil { + if err := os.WriteFile(path, []byte("
payload over MCP works
"), 0o600); err != nil { t.Fatal(err) } diff --git a/internal/services/mcpsvc/millmcpservice_tools.go b/internal/services/mcpsvc/millmcpservice_tools.go index 86c7b05b..09f86b58 100644 --- a/internal/services/mcpsvc/millmcpservice_tools.go +++ b/internal/services/mcpsvc/millmcpservice_tools.go @@ -25,7 +25,7 @@ import ( // MCPWriteEnabledKey stores the gate as the string "true"/"false" -- // matching every other settings key's string convention // (settingsservice.go). -const MCPWriteEnabledKey = "mcp-write-tools-enabled" +const MCPWriteEnabledKey = "mcp-write-tools-enabled" //nolint:gosec // a settings-store key name, not a credential (G101 false positive) func (m *MillMCPService) writeEnabled() bool { v, ok := m.store.Get(MCPWriteEnabledKey).(string) diff --git a/internal/services/triggersvc/filesystemwatch_seed_test.go b/internal/services/triggersvc/filesystemwatch_seed_test.go index 5042f2c8..29953478 100644 --- a/internal/services/triggersvc/filesystemwatch_seed_test.go +++ b/internal/services/triggersvc/filesystemwatch_seed_test.go @@ -86,7 +86,7 @@ func TestSeededDisabledFilesystemWatch_FiresRealWorkflowOnFileCreate(t *testing. t.Fatal("the re-pointed, enabled, published seed is not armed, want ArmedWorkflows() to report it live") } - if err := os.WriteFile(filepath.Join(watchDir, "new-file.txt"), []byte("hello"), 0o644); err != nil { + if err := os.WriteFile(filepath.Join(watchDir, "new-file.txt"), []byte("hello"), 0o600); err != nil { t.Fatalf("WriteFile: %v", err) } diff --git a/internal/services/triggersvc/savedpage_seed_test.go b/internal/services/triggersvc/savedpage_seed_test.go index 53927259..a67a9514 100644 --- a/internal/services/triggersvc/savedpage_seed_test.go +++ b/internal/services/triggersvc/savedpage_seed_test.go @@ -114,7 +114,7 @@ func TestSeededSavedPageToMarkdown_FiresRealWorkflowAndExtractsMainContent(t *te ` - if err := os.WriteFile(filepath.Join(watchDir, "saved-page.html"), []byte(fixtureHTML), 0o644); err != nil { + if err := os.WriteFile(filepath.Join(watchDir, "saved-page.html"), []byte(fixtureHTML), 0o600); err != nil { t.Fatalf("WriteFile: %v", err) }