diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index 05f9566..8096c64 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -82,12 +82,14 @@ jobs: - name: System dependencies # `nodejs` is not for us - it is what act_runner needs to execute the # JavaScript action in the next step. It has to be installed before the - # checkout, which is why this step comes first. + # checkout, which is why this step comes first. `curl` and `unzip` are + # for 1Password's action further down, which fetches its own CLI - the + # set proven by torben/gitea-mcp@ci-binford, binford-release.yaml. run: | set -eu apt-get update -qq apt-get install -y --no-install-recommends \ - python3 python3-venv git nodejs ripgrep ca-certificates + python3 python3-venv git nodejs ripgrep ca-certificates curl unzip rm -rf /var/lib/apt/lists/* - uses: actions/checkout@v7 @@ -145,6 +147,37 @@ jobs: .venv/bin/python -m pytest -q \ --cov --cov-report=term --cov-report=xml --cov-report=html + - name: Load a real secret reference from 1Password + # The CI half of the tiered settings (tools/chemenu/settings.py). Of the + # two ways a configuration reaches 1Password in CI - `op` itself with a + # service-account token, or an action resolving the `op://` references + # into variables first - this is the second, the shape this Gitea + # instance already runs (torben/gitea-mcp@ci-binford, + # binford-release.yaml). A `{"ref": "ci_smoke"}` then resolves through + # `WIKITOOL_SECRET_CI_SMOKE`. The token is a user-level secret whose + # service account sees only the CI-CD vault; the field read is a + # username, the least sensitive value that vault holds, and the action + # masks it in the log all the same. A pull request from a fork gets no + # secrets, so this step fails there - nobody but the operator pushes here. + uses: 1password/load-secrets-action@v2 + with: + export-env: true + env: + OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }} + WIKITOOL_SECRET_CI_SMOKE: op://CI-CD/gitea-package-token/username + + - name: Secret references resolve in CI + # `CHEMENU_LIVE_SECRET_REQUIRE=1` turns "no value loaded" into a failure: + # without it the test skips and stays green, which is the outcome this + # step exists to rule out - the same reason the CalDAV step below sets + # `CHEMENU_LIVE_REQUIRE`. + env: + CHEMENU_LIVE_SECRET_REQUIRE: "1" + run: | + set -eu + cd tools + .venv/bin/python -m pytest -q chemenu/tests/test_settings.py -k live_secret + - name: Start Radicale # Its own step because the script hands the server's address to the suite through # $GITHUB_ENV, which only reaches the steps after the one that wrote it. diff --git a/.gitignore b/.gitignore index ca319f7..e422e38 100644 --- a/.gitignore +++ b/.gitignore @@ -153,6 +153,12 @@ npm-debug.log* # preflight has not run, and `tools/wikitool` refuses to start (exit 42). /.wikitool-tools.json +# The checkout tier of the settings (tools/chemenu/settings.py): what is true +# only for this clone on this machine. Its committed counterpart, `wikitool.json`, +# holds the instance tier and must stay tracked - `docs verify` checks both. +# Secrets are in neither file; both hold only references to them. +/.wikitool-local.json + # Live-suite tracker profiles (Gitea #156): one file per tracker of the user's own that the # live suite may be pointed at (`CHEMENU_LIVE_PROFILE=`, # instructions/dev/tracker-testing.md). They carry the same credentials as the file above diff --git a/AGENTS.md b/AGENTS.md index a31ce8b..b8f8fe1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,6 +16,7 @@ maintained permanently; anything mechanical is done by `tools/wikitool`, never b - [File naming](#file-naming) - [Personalization](#personalization) - [Environment](#environment) +- [Settings](#settings) - [Routing](#routing) - [Gates](#gates) - [Tool error contract](#tool-error-contract) @@ -153,7 +154,7 @@ type, index, lint or provenance; `dist export` ships it verbatim except `docs/de verify` checks only that every file sits in one of its five subdirectories, and no other `tools/wikitool` command touches it. -Six explanation pages are reached from this file, each by link rather than automatically: +Seven explanation pages are reached from this file, each by link rather than automatically: [docs/explanation/pipeline-rationale.md](docs/explanation/pipeline-rationale.md) (why the pipeline has four stages), [docs/explanation/ownership-and-templates.md](docs/explanation/ownership-and-templates.md) (why @@ -165,10 +166,13 @@ reader rather than the owner), [docs/explanation/why-gates-are-code.md](docs/explanation/why-gates-are-code.md) (why the gates in [Gates](#gates) are code rather than instruction), [docs/explanation/version-model.md](docs/explanation/version-model.md) (why a version number -answers a compatibility question and a migration question separately), and +answers a compatibility question and a migration question separately), [docs/explanation/knowledge-and-commitment.md](docs/explanation/knowledge-and-commitment.md) (why commitments live in a task tracker rather than in `kb/`, and why the two are joined at read time -instead of synced). A seventh, `docs/explanation/model-and-effort-selection.md`, is deliberately +instead of synced), and +[docs/explanation/configuration-tiers.md](docs/explanation/configuration-tiers.md) (why settings +sit in three tiers, why a value has exactly one place, and what keeping secrets out of the tree +does not protect against). An eighth, `docs/explanation/model-and-effort-selection.md`, is deliberately not linked here but from `CLAUDE.md`: it decides something only that harness has to decide, and a link here would load it into the other three. @@ -210,6 +214,30 @@ It carries no authority: a remote or MCP server listed here does not authorize a (invariant 5) or open a gate, and does not source a `kb/` claim (invariant 3). It holds no credentials - it sits in plaintext in the working tree and in every agent's context. +## Settings + +`wikitool` reads its settings from two files at the root: `wikitool.json`, committed, for what +is the same in every clone of the instance, and `.wikitool-local.json`, gitignored, for what is +true only for this checkout. Secrets sit in neither - both hold only a reference to them, +`{"ref": ""}` or `{"ref": "op:////"}`. Each field declares which tier +it belongs to, and **a field that names a program to run or affects a gate is never in the +committed file**, not even as a reference: a pull from another clone must not change what +`wikitool` executes or where it pushes. The code refuses to declare such a field otherwise. + +- Write a setting with `tools/wikitool config set`, which puts it where its tier says. A field + that names a program or affects a gate is the user's to edit by hand - `config set` exits 42 + and says so. +- **A secret never passes through you.** Never ask for one in the chat, never put one on a + command line, and never read the secrets file or run `op read` to look at one. Ask the user to + put the value into 1Password, the secrets file or a `WIKITOOL_SECRET_` variable, then set + the reference. +- `tools/wikitool doctor` reports whether both files pass and whether each reference resolves - + never a value. + +Like `ENVIRONMENT.md`, neither file sources a `kb/` claim (invariant 3). What keeping secrets +out of the tree does and does not protect against: +[docs/explanation/configuration-tiers.md](docs/explanation/configuration-tiers.md). + ## Routing **The pipeline** - four stages, each with one job: diff --git a/CHANGES.md b/CHANGES.md index 2b175db..ea01250 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -14,16 +14,23 @@ place only: `instructions/dev/version-parts.md` § The candidate model. --- -## 9.0.1-beta.2 - 2026-10-10 - raw_cmd.py in ein Paket zerlegt: ein Modul je Unterkommando, Ausgabe unverändert +## 9.1.0-beta.1 - 2026-10-10 - Einstellungen in drei Stufen: wikitool.json (Instanz), .wikitool-local.json (Checkout), Secrets nur per Verweis - auch über die 1Password-CLI; neues Kommando config set **Author:** Torben Nehmer +**Medium impact** +- Einstellungen in drei Stufen: wikitool.json (Instanz), .wikitool-local.json (Checkout), Secrets nur per Verweis - auch über die 1Password-CLI; neues Kommando config set + **Low impact** - Ketten von Paketen: stack-dev schreibt ## Kette, stack-close nennt das nächste Glied - raw_cmd.py in ein Paket zerlegt: ein Modul je Unterkommando, Ausgabe unverändert +### Einstellungen in drei Stufen, Secrets nur per Verweis (#207) + +Neue Einstellungen liegen in `wikitool.json` (eingecheckt, gleich in jedem Klon) oder `.wikitool-local.json` (gitignoriert, nur dieser Checkout); jedes Feld deklariert seine Stufe, ein Wert in der falschen Datei ist ein Fehler, keine Überschreibung. Ein Feld, das ein Programm benennt oder ein Gate betrifft, ist nie `instance`. Secrets stehen nur als Verweis in den Dateien: `{"ref": ""}` über `WIKITOOL_SECRET_` oder eine Secret-Datei im Nutzerverzeichnis, `{"ref": "op://..."}` über die 1Password-CLI, die der Preflight als optionales Werkzeug erfasst; gesperrtes 1Password ergibt exit 42. `config set` schreibt dorthin, wo die Stufe es verlangt, und verweigert Secret-Werte; `doctor` zeigt eine Zeile pro Stufe, `docs verify` prüft `wikitool.json` auf Klartext-Secrets. Noch liest kein Feature seine Einstellungen so. + --- ## 9.0.0 - 2026-10-09 - Docs sorted into four areas, generated references, raw derive for real documents diff --git a/INSTALL.md b/INSTALL.md index f8e0955..de0d2e0 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -29,6 +29,10 @@ preflight names the install command for your system. The list is generated from - **ripgrep (rg)** +Optional: the **1Password CLI (`op`)**, if you want secrets and private values in the settings to +come from 1Password ([use-1password-for-settings.md](docs/how-to/use-1password-for-settings.md)). +The preflight records it when it finds it and never asks for it. + You also need: - **An agent harness**: Claude Code, GitHub Copilot (in VS Code or as a CLI), Codex CLI or diff --git a/VERSION b/VERSION index 3e54d33..ae212fa 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -9.0.1-beta.2 +9.1.0-beta.1 diff --git a/docs/explanation/README.md b/docs/explanation/README.md index e8894f3..75c2133 100644 --- a/docs/explanation/README.md +++ b/docs/explanation/README.md @@ -18,5 +18,7 @@ lives in `AGENTS.md` or a `CONTRACT.md`, and a page here only gives its reasons. and a migration question separately - [knowledge-and-commitment.md](knowledge-and-commitment.md) - why commitments live in a task tracker rather than in `kb/` +- [configuration-tiers.md](configuration-tiers.md) - why settings sit in three tiers, and what + keeping secrets out of the tree does not protect against - [model-and-effort-selection.md](model-and-effort-selection.md) - which model and effort suit which kind of session, for the harness that has to choose diff --git a/docs/explanation/configuration-tiers.md b/docs/explanation/configuration-tiers.md new file mode 100644 index 0000000..7fda80e --- /dev/null +++ b/docs/explanation/configuration-tiers.md @@ -0,0 +1,85 @@ +# Configuration tiers + +Settings are split into three tiers: what belongs to the instance (`wikitool.json`, committed), +what belongs to one checkout (`.wikitool-local.json`, gitignored), and secrets, which sit +outside the working tree and are reached only by reference. The files, their fields and the +command that writes them are in [the configuration reference](../reference/configuration.md#tiered-settings); +the rules an agent follows are in [AGENTS.md § Settings](../../AGENTS.md#settings). This page is +about why it is built this way, and what it does not do. + +## Three kinds of data that used to share files + +Before the tiers, every feature kept its settings in a gitignored file of its own. Three kinds of +data ended up mixed in them: + +- **What belongs to the instance.** The weekly review's thresholds are a decision about the + corpus, and every clone of it should make the same one. Gitignoring them meant each clone had + to be told again. +- **What belongs to one checkout.** A tool path, the push URLs a checkout may publish to, the + consent to record telemetry on this disk. Committing any of these would hand every other clone + an answer that is wrong for it rather than missing. +- **Secrets.** A tracker token sat in plain text in the working tree, protected only by + `.gitignore` - one wrong pattern away from a commit, and in plain view of every agent that read + the file in the course of ordinary work. + +The tiers give each kind one place, declared per field, so the question "where does this go" is +answered once, by whoever declares the field, rather than each time by whoever writes a value. + +## One place per value, not an override + +A familiar design lets the local file override the committed one. It is deliberately not this +one: a value in the wrong file is an error. With two possible sources for one value, every +reader first has to work out which one wins, and the answer differs between clones - a reviewer +looking at `wikitool.json` would no longer know what the instance actually runs with. One place +per value keeps the committed file a true statement about every clone. + +## What runs, and where it pushes, never travels with a pull + +A field that names a program to run, or that arms or loosens a gate, is never in the committed +file - not even as a reference, since a resolved reference is still a value someone else chose. +Otherwise a `git pull` from another clone could change what `wikitool` executes on this machine, +or where `publish` sends this checkout's content. The rule is checked when a field is declared, +in code, so no reader has to remember it; the reasoning is the same as for the gates +([why-gates-are-code.md](why-gates-are-code.md)). The same fields are also not written by +`config set`: like the Publish-Remote Gate's allowlist, they change by the user's own edit. + +## References instead of values + +A secret is only ever a reference, and a private value - a host name, an internal URL - may be +one. A reference names where the value lives: a name, which resolves through a +`WIKITOOL_SECRET_` variable or a secrets file in the user's own configuration directory, or +an `op://` URI, which resolves through the 1Password CLI. + +That is what makes the committed file portable. An instance that is public commits +`wikitool.json` publicly, so a private value can never sit there as text; as an `op://` +reference it can, and every machine signed in to the same 1Password account resolves it to the +same value. In CI, `op` runs with a service-account token and resolves the same `op://` +references. Alternatively, 1Password's own action resolves values into variables before +`wikitool` starts, which serves references by name. This repository's CI does the second. + +A reference is resolved only when a command asks for that one value, never when the files are +read, and the value stays in that process's memory. `doctor` reports whether each reference +resolves, never what it resolves to. + +## What this does not protect against + +Keeping secrets out of the tree protects against three things: a secret being committed, a +secret sitting in a file an agent reads while doing ordinary work, and a secret appearing in +`wikitool`'s output. + +It does not protect against an agent with shell access that sets out to read one. If 1Password +is unlocked in the terminal, the agent can run `op read` itself. It can read the secrets file. In +general, whatever the `wikitool` process can read in a user account, an agent running in that same +account can read too. The stack does not claim otherwise. + +What limits the exposure without preventing it: + +- a 1Password vault of its own for this instance's entries, so an unlocked `op` reaches nothing + else; +- the approval prompt of the 1Password desktop app, which asks a human before each new process + reads a value; +- in CI, a service account that sees only that vault. + +The rule in [AGENTS.md § Settings](../../AGENTS.md#settings) - a secret never passes through the +agent - is an instruction, not a barrier. The tiers make following it the easy path. They do not +make breaking it impossible. diff --git a/docs/how-to/README.md b/docs/how-to/README.md index 20545ff..e1ab38d 100644 --- a/docs/how-to/README.md +++ b/docs/how-to/README.md @@ -25,5 +25,7 @@ Running the instance: - [Set up the MCP read server](set-up-the-mcp-server.md) - for a consumer that is not a terminal on this machine - [Trace and score agent sessions](trace-and-score-sessions.md) +- [Use 1Password for settings](use-1password-for-settings.md) - secrets and private values out + of the tree, on the desktop and in CI Installing a new instance is [INSTALL.md](../../INSTALL.md) at the repository root. diff --git a/docs/how-to/use-1password-for-settings.md b/docs/how-to/use-1password-for-settings.md new file mode 100644 index 0000000..ca81e91 --- /dev/null +++ b/docs/how-to/use-1password-for-settings.md @@ -0,0 +1,56 @@ +# Use 1Password for settings + +Keep secrets and private values (host names, internal URLs) out of the working tree by storing +them in 1Password and leaving only an `op://` reference in the settings. The committed +`wikitool.json` then works on every machine signed in to the same 1Password account, and in CI. +What the files and references are: [configuration reference](../reference/configuration.md#tiered-settings). +Why, and what this does not protect against: [configuration-tiers.md](../explanation/configuration-tiers.md). + +## On a desktop + +1. **Install the 1Password CLI** (`op`) and turn on its integration with the 1Password desktop + app, so `op` asks the app instead of keeping a session of its own. 1Password's own + documentation describes both steps for each platform. +2. **Run the preflight again** - `tools/preflight.sh`, or in PowerShell 7 + `pwsh -NoProfile -ExecutionPolicy Bypass -File tools/preflight.ps1`. It records `op` in + `.wikitool-tools.json` and shows it as `OK`; without `op` the line reads `ABSENT`, which is no + problem until a reference needs it. +3. **Make a vault of its own for this instance**, and put each secret or private value into an + item there. A vault of its own limits what an unlocked `op` can reach. +4. **Copy each value's reference** (`op:////`) - the 1Password app offers it + on the field - and tell the agent which setting it belongs to: + + > Set `
.` to the reference `op:////`. + + The agent runs `tools/wikitool config set
. --ref op:////`, + which writes the reference into `wikitool.json`. Never paste the value itself into the chat: + the agent should never see it. +5. **Check it:** `tools/wikitool doctor`. The `settings-secrets` line says how many references + resolve. When the desktop app asks to approve access, that is the prompt doing its job - a + locked app makes the line a `WARN`, and a command that needs the value stops with exit 42 until + you unlock it. + +## Without 1Password + +A reference by name works the same way without `op`: + +- set the reference: `tools/wikitool config set
. --ref `; +- put the value into `WIKITOOL_SECRET_` (the name upper-cased) in the environment, or into + the secrets file `~/.config/chemenu//secrets.json` as `{"": ""}`, + readable by you only (`chmod 600`). The `instance_id` is in `wikitool.json`. + +## In CI + +There are two ways, and which one fits depends on the form of the references: + +- **`op` itself, with a service-account token** - for settings that hold `op://` references. + Install `op` in the job, run the preflight so it records it, and set `OP_SERVICE_ACCOUNT_TOKEN`; + the references then resolve exactly as on a desktop. +- **1Password's action resolves the values first** - for settings that hold references by name. + `1password/load-secrets-action` with a service-account token reads an `op://` reference into a + variable before `wikitool` starts; name the variable `WIKITOOL_SECRET_` and use + `{"ref": ""}` in the settings. This repository's own CI does this, in + `.gitea/workflows/ci.yml`. An `op://` reference in the settings is not resolved this way: it + still needs `op`. + +Give the service account access to the instance's own vault only. diff --git a/docs/reference/commands/README.md b/docs/reference/commands/README.md index 0d0285c..84127c3 100644 --- a/docs/reference/commands/README.md +++ b/docs/reference/commands/README.md @@ -21,6 +21,7 @@ collected in [exit codes and gates](../exit-codes.md). - [Telemetry](#telemetry) - [Distribution and versioning](#distribution-and-versioning) - [Content migrations](#content-migrations) +- [Configuration](#configuration) - [Instance health](#instance-health) @@ -157,6 +158,12 @@ Full records: [content-migrations.md](content-migrations.md) - [`migrate done`](content-migrations.md#migrate-done) - Record one migration as applied, advancing `kb_version` in `.wikitool-kb.json`. - [`migrate baseline`](content-migrations.md#migrate-baseline) - Declare `kb_version` once, for an instance predating `.wikitool-kb.json`. +## Configuration + +Full records: [configuration.md](configuration.md) + +- [`config set`](configuration.md#config-set) - Set one value of the tiered settings, in the file its tier names. + ## Instance health Full records: [instance-health.md](instance-health.md) diff --git a/docs/reference/commands/configuration.md b/docs/reference/commands/configuration.md new file mode 100644 index 0000000..9dccbd6 --- /dev/null +++ b/docs/reference/commands/configuration.md @@ -0,0 +1,67 @@ +# Configuration + +Writing the tiered settings in `wikitool.json` and `.wikitool-local.json`. + + +## `config set` + +Set one value of the tiered settings, in the file its tier names. + +**Synopsis** + +- `wikitool config set
. ` +- `wikitool config set
. --ref ` + +**Options** + +```text +Arguments: + key
. [required] + value The plain value; a string as given, anything else as JSON. + +Options: + --ref A reference instead of a value: or + op:////. + -h, --help Show this message and exit. +``` + +**Examples** + +```bash +tools/wikitool config set tracker.stale_days 14 +tools/wikitool config set tracker.token --ref tracker_token +tools/wikitool config set tracker.url --ref op://Chemenu/tracker/url +``` + +**Behaviour** + +- Writes a plain value of a field of tier `instance` into `wikitool.json`, and one of tier `checkout` - a private field's included - into `.wikitool-local.json`. +- With `--ref`, writes a reference instead of a value into `wikitool.json`: `` resolves through `WIKITOOL_SECRET_`, then the secrets file in the user's configuration directory; `op:////` resolves through the 1Password CLI. Only a secret or private field takes one. +- Removes the same key from the other file, so a value never has two places, and says so. +- Creates `wikitool.json` with a fresh `instance_id` the first time it writes anything; the secrets file is found by that id. +- Checks both files as they would stand after the write against every registered section, and writes nothing when that fails. +- Prints the key and the file it wrote, never the value. Resolves no reference. + +**Properties** + +- Changes files: yes +- Idempotent: yes +- Atomic: Each file is replaced in one rename; both are checked before either is written +- Counts toward the session call budget: yes +- Can reach the network: no + +**Exit codes** + +- `0` - success +- `1` - The key names no registered section or field, or is not `
.` +- `1` - A plain value given for a field of tier `secret` +- `1` - `--ref` given for a field that takes a plain value, or a malformed reference +- `1` - The value is not of the field's type, or the files would not pass their check afterwards +- `42` - The field names a program to run or affects a gate + +**See also** + +- `wikitool doctor` - whether both files pass and every reference resolves +- `docs/reference/configuration.md` - the files, the tiers and the references +- `docs/how-to/use-1password-for-settings.md` - setting 1Password up as the source + diff --git a/docs/reference/commands/instance-health.md b/docs/reference/commands/instance-health.md index fee6830..df3f5da 100644 --- a/docs/reference/commands/instance-health.md +++ b/docs/reference/commands/instance-health.md @@ -39,6 +39,7 @@ tools/wikitool doctor --json - Task tracker: `.wikitool-tasks.json` present, absent or malformed - absent is `OK` (no tracker configured), malformed is a `FAIL`. When `WIKITOOL_TASKS_CONFIG` is set, that file is read instead and the finding names it; a set variable that names no file is a `FAIL`, never `OK`. - For a configured `superproductivity` provider, the configured `access` path's own state: `access: "api"` reports whether its local REST API answers `GET /health` with a ready renderer right now, `access: "snapshot"` whether a backup file is ready. The other access path is never attempted, and neither state is ever a `FAIL`. - For a configured `caldav` provider, whether the server is reachable and Basic auth succeeds - never a `FAIL`; only a broken config block is. +- Settings (`settings-instance`, `settings-checkout`, `settings-secrets`): one line per tier. `wikitool.json` and `.wikitool-local.json` each present or absent - absent is `OK` - and a `FAIL` naming file, key and the right tier when a value sits in the wrong file, a secret or a committed private field holds a plain value, or a value fails its field; a section this version does not read is a `WARN`. For the references: how many resolve, never a value - an unresolvable one is a `FAIL`, one 1Password will not hand out until a human unlocks it a `WARN`. - Session id source: `OK` for `WIKITOOL_SESSION_ID` or a registered harness variable, `WARN` only for the bare parent-pid fallback. - Telemetry: on or off and why - installation-form default, `.wikitool-telemetry.json`, or `WIKI_TRACE` - and the current session's count and byte total against both caps; never a `FAIL`. A `WARN` when `.wikitool-telemetry.json` is not read as written - unreadable JSON, not an object, `enabled` not a JSON boolean, a cap not a positive integer, or an unknown key - naming the field; telemetry itself drops or reinterprets such a value without a word, so this is where it shows. - Exits 1 only on a `FAIL`; a missing remote, session id or `VERSION` is a `WARN`, not a fault. @@ -61,7 +62,7 @@ tools/wikitool doctor --json - `instructions/setup-instance.md` - the setup steps most findings point back to - `instructions/preflight.md` - what `tool-paths` and `install-dir` point back to -- `docs/reference/configuration.md` - the per-checkout configuration files +- `docs/reference/configuration.md` - the per-checkout configuration files and the tiered settings - `docs/explanation/telemetry-and-evaluation.md` - telemetry state and caps - `instructions/session-setup.md` - setting `WIKITOOL_SESSION_ID` diff --git a/docs/reference/commands/types-instructions-and-docs.md b/docs/reference/commands/types-instructions-and-docs.md index 1d23f3b..d5f7694 100644 --- a/docs/reference/commands/types-instructions-and-docs.md +++ b/docs/reference/commands/types-instructions-and-docs.md @@ -283,7 +283,8 @@ tools/wikitool docs verify - Collections: every directory under `kb/` has a `COLLECTION.md` and no directory outside it does; every collection declares `profile:` and a `required_by_stack:` that agrees with the stack's own list. - Types: every type the stack lists (currently `source` and `project`) has a type-spec of that name whose schema requires the field the stack list names (`raw_files:`/`state:`); every file under `types/` declaring `type: types/type-spec.md` validates against `types/type-spec.schema.yaml`; every subtype template `types/..md` (any `` but `guidance`) sits beside a type-spec declaring `subtype_field:`, names a value that field's schema enum allows, and carries no frontmatter; no pre-migration `type: entity` blocks are left in the contracts. - `kb/CONVENTIONS.md`, if it exists at all, names all three tool-owned section headings; every stage contract is present. -- The `.gitignore` canaries clear in both directions: nothing ignored under `raw/`/`kb/`, `incoming/` ignored, everything ignored under `reports/` and the published skill directories. +- The `.gitignore` canaries clear in both directions: nothing ignored under `raw/`/`kb/`, `incoming/` ignored, everything ignored under `reports/` and the published skill directories, `.wikitool-local.json` ignored and `wikitool.json` tracked. +- A `wikitool.json` at the root holds every value in the place its tier allows - no secret and no private field as plain text, no `checkout` field at all except as a reference - and a well-formed `schema` and `instance_id`. The local file is not read. - No `.md`/`.template` file `dist export` would ship cites an issue number. A `` region is exempt: the check reads the export plan's text, from which it is already gone. - Every reference file `docs toc` covers carries the current table-of-contents region for its own headings - missing and stale are one check. - Every relative markdown link in one of those reference files resolves to an existing file. A target's `#anchor` suffix is stripped first, and code fences and inline code spans are masked before scanning, so link syntax shown as an example is not mistaken for a real reference. @@ -312,6 +313,7 @@ tools/wikitool docs verify - `1` - The `` region of `tools/CONTRACT.md` is stale - `1` - A type-spec's own frontmatter fails its schema - `1` - A subtype template `types/..md` has no type-spec with a `subtype_field:` beside it, names a value outside that field's enum, or carries frontmatter +- `1` - `wikitool.json` holds a secret or private value as plain text, or a value its tier puts elsewhere - `1` - A shipped `.md`/`.template` cites an issue number - `1` - A region of the human reference under `docs/reference/` is stale - `1` - A page of the human reference is missing or lacks its marker pair, or a page under `docs/reference/commands/` belongs to no group diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index c611fd2..228e858 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -1,7 +1,7 @@ # Configuration -The environment variables `wikitool` reads, and the per-checkout files an operator may create at the -root of an instance. None of them is needed for an instance to work: an instance installed from a +The environment variables `wikitool` reads, the per-checkout files an operator may create at the +root of an instance, and the two tiered settings files. None of them is needed for an instance to work: an instance installed from a release runs against the origin's public release feed with no configuration at all. The shape of each file - its fields, what an absent or a malformed file means, an example - is @@ -24,6 +24,7 @@ something you run. - [Super Productivity](#super-productivity) - [CalDAV](#caldav) - [MCP submissions](#mcp-submissions) +- [Tiered settings](#tiered-settings) ## Environment variables @@ -35,6 +36,7 @@ something you run. | `WIKITOOL_UPDATE_URL` | The release feed `version check`, `version notes` and `dist upgrade --latest` ask | The value in `.wikitool-release.json`, else the origin's feed | | `WIKITOOL_UPDATE_TOKEN` | A Gitea token for the release feed, sent only to downloads on the feed's own host | None. Not needed against the origin, only against a feed in a private repository | | `WIKITOOL_TASKS_CONFIG` | Path to a tracker configuration that `task`, `review` and `doctor` read instead of `.wikitool-tasks.json`, for running one checkout against several trackers in turn | `.wikitool-tasks.json` at the root. A path to a file that does not exist is an error, never "no tracker configured" | +| `WIKITOOL_SECRET_` | The value behind a settings reference `{"ref": ""}`, for CI and containers - `` is the name upper-cased | The secrets file in the user's configuration directory ([below](#tiered-settings)) | | `CHEMENU_ROOT` | Which corpus the package works on, for a caller outside the checkout itself | The checkout the package sits in. `tools/wikitool` behaves the same without it | | `WIKI_TRACE` | `0`/`1` turns telemetry off or on, beating both the default and `.wikitool-telemetry.json` | The installation-form default ([below](#telemetry)) | | `WIKI_TRACE_DIR` | Writes traces somewhere other than `reports/telemetry/` | `reports/telemetry/` | @@ -84,7 +86,10 @@ both caps. What the caps do and what never reaches a trace: ## Tool paths **The preflight writes this file, not you.** `.wikitool-tools.json` at the root holds the absolute -paths of Python, git and ripgrep, as the preflight found them on this machine. `wikitool` starts git +paths of Python, git and ripgrep, as the preflight found them on this machine - and of the +1Password CLI `op` when it is installed. `op` is optional: the preflight records it when it finds +it, reports `ABSENT` when it does not, and never asks for it. Only an `op://` reference in the +[tiered settings](#tiered-settings) needs it. `wikitool` starts git and rg from there instead of from `PATH`. It is per checkout and gitignored, because a path on one machine says nothing about the next. @@ -330,3 +335,61 @@ Example: Size and quota are your deployment's decision, not defaults worth copying blindly. + +## Tiered settings + +Two files at the root hold the settings of the features that read them this way, and every field +declares the tier it belongs to: + +| Tier | File | Holds | +|------|------|-------| +| `instance` | `wikitool.json`, committed | What belongs to the corpus and is the same in every clone | +| `checkout` | `.wikitool-local.json`, gitignored | What is true only for this clone on this machine | +| `secret` | neither | Tokens and passwords - both files hold only a reference | + +Each file is a JSON object of sections, one per feature, beside two keys of its own: + +```json +{ + "schema": 1, + "instance_id": "6f1c0e52-3d1b-4b8e-9a51-2f0e7d1c9b44", + "
": {"": ""} +} +``` + +`instance_id` sits in `wikitool.json` only. The first `wikitool config set` creates the file and +the id; the id names the directory of the secrets file below. No feature reads its settings this +way yet, so this version declares no section: `config set` names the sections it knows, and +`doctor` reports a section it does not read as a `WARN`. + +**One place per value.** A value in the file its tier does not name is an error naming the file, +the key and the right tier - never an override. The same holds for a key set in both files. + +**References.** A field of tier `secret` takes only a reference; a field marked private - a host +name, an internal URL - takes a plain value in `.wikitool-local.json` or a reference in either +file. No other field takes one. + +| Reference | Resolves through | +|-----------|------------------| +| `{"ref": ""}` - lower-case letters, digits and single `_` | `WIKITOOL_SECRET_`, else the key `` in the secrets file | +| `{"ref": "op:////"}` | `op read`, the 1Password CLI the preflight recorded | + +The secrets file is `~/.config/chemenu//secrets.json` (`$XDG_CONFIG_HOME` instead of +`~/.config` when it is set; `%APPDATA%\chemenu\\secrets.json` on Windows), a JSON object of +names to strings. Outside Windows it must be readable by its owner only (`chmod 600`); a file +others can read is refused. + +A reference is resolved when a command asks for that value, never when the files are read. A name +nobody provides, an `op` that is not installed, or an item 1Password does not have ends the command +with exit 1; a 1Password that is locked, signed out or waiting for an approval ends it with exit 42, +for a human to unlock. Setting 1Password up: [use-1password-for-settings.md](../how-to/use-1password-for-settings.md). + +**Never in the committed file:** a field that names a program to run or affects a gate - not even +as a reference. `config set` does not write such a field either; the user edits +`.wikitool-local.json` by hand, and `doctor` checks it afterwards. + +`wikitool doctor` reports one line per tier - `settings-instance`, `settings-checkout`, +`settings-secrets` - and for the references only whether they resolve. `docs verify` holds a +committed `wikitool.json` to the placement rules above, a plain secret or private value above +all. Neither file ships with `dist export`. Why the tiers exist, and what keeping secrets out of +the tree does not protect against: [configuration-tiers.md](../explanation/configuration-tiers.md). diff --git a/docs/reference/exit-codes.md b/docs/reference/exit-codes.md index 45b8dd4..c37f18f 100644 --- a/docs/reference/exit-codes.md +++ b/docs/reference/exit-codes.md @@ -38,4 +38,5 @@ What each exit code means, and how a caller reacts to it, is [AGENTS.md § Tool - [`publish`](commands/git.md#publish): Rebase-Review Gate: `/` moved and both sides changed the same file; the output lists the upstream commits, the overlapping files and their diff - [`publish`](commands/git.md#publish): Publish-Remote Gate: `.wikitool-remotes.json` exists and the push URL of `--remote` is not listed in it, or `--remote` resolves to no push URL - [`export guidelines`](commands/guideline-export.md#export-guidelines): Guideline Push Gate: `--push` without `--confirm`, or with a token that does not match the current set of targets +- [`config set`](commands/configuration.md#config-set): The field names a program to run or affects a gate diff --git a/instructions/dev/doc-pull-through.md b/instructions/dev/doc-pull-through.md index 8fd8b0c..1803acc 100644 --- a/instructions/dev/doc-pull-through.md +++ b/instructions/dev/doc-pull-through.md @@ -37,10 +37,10 @@ touched; a row that does not apply needs no action. | A stage's authoring rules (`raw/`, `kb/`, `types/`, `reports/`, `work/`, `tools/`, `instructions/`) | The touched `/CONTRACT.md` | | A rule, gate, or invariant `AGENTS.md` itself states | The relevant `AGENTS.md` section (Invariants, Gates, File naming, Routing, ...) | | A workflow, stage, or command a human operates by hand | Whichever of `README.md`, `tools/README.md`, `INSTALL.md` or the pages under `docs/tutorials/`, `docs/how-to/` and `docs/reference/` names it - AGENTS.md § File naming says which document is for which reader. A new page goes into the area its kind of documentation belongs to; that area's `README.md` says which | - | The reasoning behind a gate, boundary, or design decision | The `docs/explanation/` page that carries it, if one exists (AGENTS.md § File naming lists all six reached from AGENTS.md itself, plus a seventh reached only from CLAUDE.md). **A decision with no page yet is the gap worth closing**: reasoning that lives only in a Gitea issue never ships - `dist export` carries `docs/explanation/` and no issue tracker, so a distributed instance gets the mechanism without the why | + | The reasoning behind a gate, boundary, or design decision | The `docs/explanation/` page that carries it, if one exists (AGENTS.md § File naming lists all seven reached from AGENTS.md itself, plus an eighth reached only from CLAUDE.md). **A decision with no page yet is the gap worth closing**: reasoning that lives only in a Gitea issue never ships - `dist export` carries `docs/explanation/` and no issue tracker, so a distributed instance gets the mechanism without the why | | A task-tracker adapter (`tools/chemenu/tasks/`), its recorded fixtures, or the live suite | [instructions/dev/tracker-testing.md](tracker-testing.md), and `MANIFEST.json` beside the fixtures when they were re-recorded | | A skill's own step sequence or catalogue | The skill's `SKILL.md` source under `instructions//` or `instructions/dev//`; for a shipped skill (not under `instructions/dev/`) whose name or `description` changed, or one added or removed, `wikitool docs reference --apply` for the skill catalogue `docs/reference/skills.md` | - | A per-checkout configuration file an instance owns (`.wikitool-tasks.json`, `.wikitool-telemetry.json`, `.wikitool-remotes.json`, `.wikitool-upload.json`) | Its shape - a field, a default, what is required or tolerated - is its record in `tools/chemenu/config_shapes.py`, which the loader reads; change it there, then `wikitool docs reference --apply` for its region in [docs/reference/configuration.md](../../docs/reference/configuration.md). What a setting *does* is the hand-written prose around that region. Also the [setup-instance.md](../setup-instance.md) decision point that offers the file during setup, and `doctor`'s own row in [tools/CONTRACT.md](../../tools/CONTRACT.md), since `doctor` is what reports the file's state | + | A per-checkout configuration file an instance owns (`.wikitool-tasks.json`, `.wikitool-telemetry.json`, `.wikitool-remotes.json`, `.wikitool-upload.json`), or a section of the tiered settings (`wikitool.json`, `.wikitool-local.json`; `config_shapes.SETTINGS_SECTIONS`) | Its shape - a field, a default, what is required or tolerated - is its record in `tools/chemenu/config_shapes.py`, which the loader reads; change it there, then `wikitool docs reference --apply` for its region in [docs/reference/configuration.md](../../docs/reference/configuration.md). What a setting *does* is the hand-written prose around that region. Also the [setup-instance.md](../setup-instance.md) decision point that offers the file during setup, and `doctor`'s own row in [tools/CONTRACT.md](../../tools/CONTRACT.md), since `doctor` is what reports the file's state | | An installation instruction - [preflight.md](../preflight.md), [setup-instance.md](../setup-instance.md), [bootstrap.md](../bootstrap.md), [upgrade-instance.md](../upgrade-instance.md) - or `tools/prerequisites.txt` | [INSTALL.md](../../INSTALL.md), the human guide to the same procedure - for `upgrade-instance.md`, [docs/how-to/update-an-instance.md](../../docs/how-to/update-an-instance.md) instead. Read it against the instruction: what to prepare, the sentence for the agent, what the agent asks, where it stops and why. `docs verify` checks only the two enumerable overlaps - the prerequisites lists, which `wikitool docs prerequisites --apply` regenerates from the manifest, and the setup questions: a question the agent asks the user carries `` where it is asked in `setup-instance.md`, and `INSTALL.md` § "What the agent asks you" names it with the same marker. Every other sentence is this session's to compare. `INSTALL.md` does not retell the steps, so a change to their order or wording alone moves nothing there | | [dev-setup.md](dev-setup.md), the release workflow (`version bump`/`version release`/`publish`/CI), or another developer-facing process | [docs/dev/development.md](../../docs/dev/development.md), read against it the same way - nothing checks this pair at all. A page under `docs/dev/` is never shipped, so it may link into `instructions/dev/` and cite issue numbers | | A new page type the stack requires, or a new collection | Its type-spec and `COLLECTION.md` (both as the `.template` an instance adopts), the collection table in [kb/CONTRACT.md](../../kb/CONTRACT.md), and **both adoption paths**: [setup-instance.md](../setup-instance.md) for a fresh instance and [upgrade-instance.md](../upgrade-instance.md) for an existing one, where an unadopted template is what `docs verify` refuses. A new or changed `root: kb` type-spec or its schema - description, `layout:`, subtype enum, required fields - also needs `wikitool docs reference --apply` for `docs/reference/page-types.md` | diff --git a/instructions/dev/testing-conventions.md b/instructions/dev/testing-conventions.md index d2e704c..b7b0866 100644 --- a/instructions/dev/testing-conventions.md +++ b/instructions/dev/testing-conventions.md @@ -41,6 +41,7 @@ Do not re-do any of this per test; it is done for you, per test, via `monkeypatc | `GIT_CONFIG_GLOBAL`, `GIT_CONFIG_SYSTEM` | `/dev/null` - git's own way to say "no such file" | | `GIT_DIR`, `GIT_WORK_TREE`, `GIT_AUTHOR_*`, `GIT_COMMITTER_*`, `EMAIL` | unset | | `WIKI_AUTHOR`, `WIKI_TRACE`, `WIKI_TRACE_CONTENT`, `WIKI_TRACE_MAX_CONTENT`, `WIKITOOL_SESSION_ID`, `WIKITOOL_UPDATE_URL`, `WIKITOOL_UPDATE_TOKEN` | unset | +| every `WIKITOOL_SECRET_*` - a family, cleared by prefix | unset | `WIKI_TRACE_DIR` is the one variable that stays *set*: the separate `isolated_trace_dir` fixture redirects it into `tmp_path`. Tracing is never disabled suite-wide, because two @@ -130,7 +131,8 @@ Whenever you add or change a test under `tools/chemenu/tests/`. fixture creates that machine. 4. **Adding a new environment variable to the tool?** Add it to `_WIKITOOL_ENV` in - `conftest.py` in the same change (`WIKITOOL_TASKS_CONFIG` is one). A variable the tool reads and the fixture does not clear + `conftest.py` in the same change (`WIKITOOL_TASKS_CONFIG` is one); a family read by prefix gets + a prefix loop beside it, as `WIKITOOL_SECRET_*` has. A variable the tool reads and the fixture does not clear is the exact hole this whole file is about, reopened. 5. **Writing a fixture that builds a tree?** Repoint `config.ROOT` at it and call diff --git a/instructions/preflight.md b/instructions/preflight.md index f922604..004aad2 100644 --- a/instructions/preflight.md +++ b/instructions/preflight.md @@ -19,7 +19,9 @@ Windows. The two answer the same questions from the same list and write the same (PowerShell only) that the execution policy and the files' Mark of the Web let `tools/wikitool.ps1` start; - records the absolute path of each tool in `.wikitool-tools.json`, which `wikitool` then starts - them from instead of trusting whatever `PATH` a session inherited; + them from instead of trusting whatever `PATH` a session inherited - and of an optional tool, + such as the 1Password CLI `op`, when it finds one: an optional tool missing is reported as + `ABSENT` and is never a reason to stop, nor to install anything; - creates `tools/.venv` from the recorded Python and installs `tools/requirements.txt` into it. diff --git a/tools/CONTRACT.md b/tools/CONTRACT.md index 8969962..4a56dea 100644 --- a/tools/CONTRACT.md +++ b/tools/CONTRACT.md @@ -48,6 +48,7 @@ ON FAILURE and NEVER, which address an agent. - [Telemetry](#telemetry) - [Distribution and versioning](#distribution-and-versioning) - [Content migrations](#content-migrations) + - [Configuration](#configuration) - [Instance health](#instance-health) - [Design notes](#design-notes) - [Tests](#tests) @@ -165,6 +166,7 @@ migrate status read idempotent budget:exempt exit:0,1 migrate verify read idempotent budget:exempt exit:0,1 Compare `kb/` against a git revision on the invariants a content migration must not change. migrate done write non-idempotent budget:counted exit:0,1 Record one migration as applied, advancing `kb_version` in `.wikitool-kb.json`. migrate baseline write idempotent budget:counted exit:0,1 Declare `kb_version` once, for an instance predating `.wikitool-kb.json`. +config set write idempotent budget:counted exit:0,1,42 Set one value of the tiered settings, in the file its tier names. doctor read idempotent budget:exempt exit:0,1 Check that this instance is correctly configured. ``` @@ -2579,6 +2581,7 @@ Check the docs that mirror the code. - 1 The `` region of `tools/CONTRACT.md` is stale - 1 A type-spec's own frontmatter fails its schema - 1 A subtype template `types/..md` has no type-spec with a `subtype_field:` beside it, names a value outside that field's enum, or carries frontmatter +- 1 `wikitool.json` holds a secret or private value as plain text, or a value its tier puts elsewhere - 1 A shipped `.md`/`.template` cites an issue number - 1 A region of the human reference under `docs/reference/` is stale - 1 A page of the human reference is missing or lacks its marker pair, or a page under `docs/reference/commands/` belongs to no group @@ -2598,6 +2601,7 @@ Check the docs that mirror the code. - The `` region of `tools/CONTRACT.md` is stale -> Run `docs contract --apply`, then re-run - A type-spec's own frontmatter fails its schema -> Fix the field, or add a matching line to `types/type-spec.schema.yaml` if the field is legitimately new - A subtype template `types/..md` has no type-spec with a `subtype_field:` beside it, names a value outside that field's enum, or carries frontmatter -> Rename the file to the type and value it was meant for, delete it, or remove its frontmatter block +- `wikitool.json` holds a secret or private value as plain text, or a value its tier puts elsewhere -> Show the user the message: a plain value there may already be in the history. Move it as the message says - a secret into 1Password, the secrets file or a variable, set by the user, with only the reference left behind - then re-run - A shipped `.md`/`.template` cites an issue number -> Say what was decided instead of pointing at where, or move the pointer behind a `` block - A region of the human reference under `docs/reference/` is stale -> Run `docs reference --apply`, then re-run - A page of the human reference is missing or lacks its marker pair, or a page under `docs/reference/commands/` belongs to no group -> Create the page with a title, a one-sentence head and the marker pair the message names (or add the pair), then run `docs reference --apply`; `git mv` or `git rm` a page left over from a renamed or removed group @@ -2620,7 +2624,8 @@ Check the docs that mirror the code. - Collections: every directory under `kb/` has a `COLLECTION.md` and no directory outside it does; every collection declares `profile:` and a `required_by_stack:` that agrees with the stack's own list. - Types: every type the stack lists (currently `source` and `project`) has a type-spec of that name whose schema requires the field the stack list names (`raw_files:`/`state:`); every file under `types/` declaring `type: types/type-spec.md` validates against `types/type-spec.schema.yaml`; every subtype template `types/..md` (any `` but `guidance`) sits beside a type-spec declaring `subtype_field:`, names a value that field's schema enum allows, and carries no frontmatter; no pre-migration `type: entity` blocks are left in the contracts. - `kb/CONVENTIONS.md`, if it exists at all, names all three tool-owned section headings; every stage contract is present. -- The `.gitignore` canaries clear in both directions: nothing ignored under `raw/`/`kb/`, `incoming/` ignored, everything ignored under `reports/` and the published skill directories. +- The `.gitignore` canaries clear in both directions: nothing ignored under `raw/`/`kb/`, `incoming/` ignored, everything ignored under `reports/` and the published skill directories, `.wikitool-local.json` ignored and `wikitool.json` tracked. +- A `wikitool.json` at the root holds every value in the place its tier allows - no secret and no private field as plain text, no `checkout` field at all except as a reference - and a well-formed `schema` and `instance_id`. The local file is not read. - No `.md`/`.template` file `dist export` would ship cites an issue number. A `` region is exempt: the check reads the export plan's text, from which it is already gone. - Every reference file `docs toc` covers carries the current table-of-contents region for its own headings - missing and stale are one check. - Every relative markdown link in one of those reference files resolves to an existing file. A target's `#anchor` suffix is stripped first, and code fences and inline code spans are masked before scanning, so link syntax shown as an example is not mistaken for a real reference. @@ -3658,6 +3663,68 @@ Declare `kb_version` once, for an instance predating `.wikitool-kb.json`. - `wikitool migrate done` - advances the version after a migration - `wikitool migrate status` - what is owed from the declared version +### Configuration + +#### `config set` + +Set one value of the tiered settings, in the file its tier names. + +**SYNOPSIS** + +- `wikitool config set
. ` +- `wikitool config set
. --ref ` + +**PROPERTIES** + +- effect: write +- idempotent: yes +- atomic: Each file is replaced in one rename; both are checked before either is written +- budget: counted +- network: no + +**EXAMPLES** + +- `tools/wikitool config set tracker.stale_days 14` +- `tools/wikitool config set tracker.token --ref tracker_token` +- `tools/wikitool config set tracker.url --ref op://Chemenu/tracker/url` + +**EXIT STATUS** + +- 0 success +- 1 The key names no registered section or field, or is not `
.` +- 1 A plain value given for a field of tier `secret` +- 1 `--ref` given for a field that takes a plain value, or a malformed reference +- 1 The value is not of the field's type, or the files would not pass their check afterwards +- 42 The field names a program to run or affects a gate + +**ON FAILURE** + +- The key names no registered section or field, or is not `
.` -> Use a key from the list the message prints; `docs/reference/configuration.md` describes the sections +- A plain value given for a field of tier `secret` -> Do not retry with the value. Ask the user to put it into `WIKITOOL_SECRET_`, the secrets file or 1Password, then set the field with `--ref` +- `--ref` given for a field that takes a plain value, or a malformed reference -> Give the value itself, or fix the reference to `` (lower case, digits, `_`) or `op:////` +- The value is not of the field's type, or the files would not pass their check afterwards -> Fix the value or the problem the message names in the file it names, then re-run once +- The field names a program to run or affects a gate -> Show the user the output verbatim and stop. Only the user sets this field, by editing `.wikitool-local.json` themselves + +**NEVER** + +- Never pass a secret's value on the command line - not to this command and not to any other. +- Never read the secrets file or run `op read` yourself to see a value. + +**NOTES** + +- Writes a plain value of a field of tier `instance` into `wikitool.json`, and one of tier `checkout` - a private field's included - into `.wikitool-local.json`. +- With `--ref`, writes a reference instead of a value into `wikitool.json`: `` resolves through `WIKITOOL_SECRET_`, then the secrets file in the user's configuration directory; `op:////` resolves through the 1Password CLI. Only a secret or private field takes one. +- Removes the same key from the other file, so a value never has two places, and says so. +- Creates `wikitool.json` with a fresh `instance_id` the first time it writes anything; the secrets file is found by that id. +- Checks both files as they would stand after the write against every registered section, and writes nothing when that fails. +- Prints the key and the file it wrote, never the value. Resolves no reference. + +**SEE ALSO** + +- `wikitool doctor` - whether both files pass and every reference resolves +- `docs/reference/configuration.md` - the files, the tiers and the references +- `docs/how-to/use-1password-for-settings.md` - setting 1Password up as the source + ### Instance health #### `doctor` @@ -3704,6 +3771,7 @@ Check that this instance is correctly configured. - Task tracker: `.wikitool-tasks.json` present, absent or malformed - absent is `OK` (no tracker configured), malformed is a `FAIL`. When `WIKITOOL_TASKS_CONFIG` is set, that file is read instead and the finding names it; a set variable that names no file is a `FAIL`, never `OK`. - For a configured `superproductivity` provider, the configured `access` path's own state: `access: "api"` reports whether its local REST API answers `GET /health` with a ready renderer right now, `access: "snapshot"` whether a backup file is ready. The other access path is never attempted, and neither state is ever a `FAIL`. - For a configured `caldav` provider, whether the server is reachable and Basic auth succeeds - never a `FAIL`; only a broken config block is. +- Settings (`settings-instance`, `settings-checkout`, `settings-secrets`): one line per tier. `wikitool.json` and `.wikitool-local.json` each present or absent - absent is `OK` - and a `FAIL` naming file, key and the right tier when a value sits in the wrong file, a secret or a committed private field holds a plain value, or a value fails its field; a section this version does not read is a `WARN`. For the references: how many resolve, never a value - an unresolvable one is a `FAIL`, one 1Password will not hand out until a human unlocks it a `WARN`. - Session id source: `OK` for `WIKITOOL_SESSION_ID` or a registered harness variable, `WARN` only for the bare parent-pid fallback. - Telemetry: on or off and why - installation-form default, `.wikitool-telemetry.json`, or `WIKI_TRACE` - and the current session's count and byte total against both caps; never a `FAIL`. A `WARN` when `.wikitool-telemetry.json` is not read as written - unreadable JSON, not an object, `enabled` not a JSON boolean, a cap not a positive integer, or an unknown key - naming the field; telemetry itself drops or reinterprets such a value without a word, so this is where it shows. - Exits 1 only on a `FAIL`; a missing remote, session id or `VERSION` is a `WARN`, not a fault. @@ -3713,7 +3781,7 @@ Check that this instance is correctly configured. - `instructions/setup-instance.md` - the setup steps most findings point back to - `instructions/preflight.md` - what `tool-paths` and `install-dir` point back to -- `docs/reference/configuration.md` - the per-checkout configuration files +- `docs/reference/configuration.md` - the per-checkout configuration files and the tiered settings - `docs/explanation/telemetry-and-evaluation.md` - telemetry state and caps - `instructions/session-setup.md` - setting `WIKITOOL_SESSION_ID` diff --git a/tools/README.md b/tools/README.md index 055f56b..0ac0d33 100644 --- a/tools/README.md +++ b/tools/README.md @@ -54,6 +54,9 @@ the recorded path - or, with no file at all (the test suite, a bare `python -m chemenu.cli`), falls back to the bare name. A file that is present but names a path that has gone raises `ToolPathError`, which the CLI turns into an `ERROR` line pointing at the preflight rather than a traceback. +An optional tool - the 1Password CLI `op`, recorded only when the preflight +found it - is asked for through `toolpaths.optional()`, which answers None +when the file does not name it; the caller says what that costs. The package runs natively on Windows as well, which CI never does. So the rules that keep it portable are held by reading the source, in the origin repository's `tests/test_portability.py`: a path that @@ -89,10 +92,11 @@ tools/ cli.py Typer app: registers every command, runs the budget gate, renders `-h`/`--help` from cli_contract cli_contract.py one data record per command (name, synopsis, properties, exit status) - the source `-h`, the index, CONTRACT.md's generated region and the human reference under docs/reference/ render from config.py repo layout: root resolution and every path under it - config_shapes.py one record per hand-written .wikitool-*.json (telemetry, remotes, upload, tasks): what its loader accepts, its defaults and `Expected:` text, and what docs/reference/configuration.md renders; stdlib-only, since the telemetry policy reads it + config_shapes.py one record per hand-written .wikitool-*.json (telemetry, remotes, upload, tasks): what its loader accepts, its defaults and `Expected:` text, and what docs/reference/configuration.md renders; also the sections of the tiered settings, each field with its tier; stdlib-only, since the telemetry policy reads it + settings.py the tiered settings: wikitool.json (instance), .wikitool-local.json (checkout), secrets by reference - placement checked per field, references resolved on access through WIKITOOL_SECRET_, the secrets file or `op read` api.py the in-process entry point - point Chemenu at a corpus and read it errors.py ChemenuError / ValidationError / BackendError - toolpaths.py where git and rg are started from: .wikitool-tools.json, bare name only without the file + toolpaths.py where git and rg are started from: .wikitool-tools.json, bare name only without the file; an optional tool such as `op` is None when not recorded filelock.py an exclusive lock on an open file, flock on POSIX and msvcrt on Windows - the only module that imports either prerequisites.py prerequisites.txt read from Python, plus the platform and long-path questions `doctor` asks install_doc.py INSTALL.md held to the instructions: its prerequisites lists generated from prerequisites.txt, its setup questions matched to setup-instance.md's markers diff --git a/tools/chemenu/cli.py b/tools/chemenu/cli.py index fa553a9..6b0f12a 100644 --- a/tools/chemenu/cli.py +++ b/tools/chemenu/cli.py @@ -27,6 +27,7 @@ try: from chemenu.commands import ( _util, cite_cmd, + config_cmd, dist_cmd, doctor, docs_verify, @@ -180,6 +181,7 @@ app.add_typer(version_cmd.app, name="version") app.add_typer(migrate_cmd.app, name="migrate") app.add_typer(task_cmd.app, name="task") app.add_typer(export_cmd.app, name="export") +app.add_typer(config_cmd.app, name="config") app.command("new")(new_page.new_page_command) app.command("touch")(touch_module.touch_command) app.command("rename")(page_ops.rename_command) diff --git a/tools/chemenu/cli_contract.py b/tools/chemenu/cli_contract.py index 20fcdc9..9d9c0bd 100644 --- a/tools/chemenu/cli_contract.py +++ b/tools/chemenu/cli_contract.py @@ -315,6 +315,9 @@ GROUPS: tuple[tuple[str, tuple[str, ...]], ...] = ( ("Content migrations", ( "migrate list", "migrate status", "migrate verify", "migrate done", "migrate baseline", )), + ("Configuration", ( + "config set", + )), ("Instance health", ( "doctor", )), diff --git a/tools/chemenu/commands/_util.py b/tools/chemenu/commands/_util.py index d557e0a..91ced21 100644 --- a/tools/chemenu/commands/_util.py +++ b/tools/chemenu/commands/_util.py @@ -95,6 +95,33 @@ def needs_clearance(msg: str) -> None: raise typer.Exit(code=EXIT_NEEDS_CLEARANCE) +def load_settings(): + """The tiered settings (`chemenu.settings.load`) for a command, or the + command's exit 1 naming file, key and tier.""" + from chemenu import settings + from chemenu.errors import ValidationError + + try: + return settings.load() + except ValidationError as exc: + fail(str(exc)) + + +def setting_value(section, name: str) -> Any: + """One settings value for a command, a reference resolved: exit 1 when it + cannot be found, exit 42 when 1Password needs a human to unlock it first. + Neither message carries the value.""" + from chemenu import settings + from chemenu.errors import ValidationError + + try: + return section[name] + except settings.SecretSourceLocked as exc: + needs_clearance(str(exc)) + except ValidationError as exc: + fail(str(exc)) + + # A comma preceded by a backslash is a literal comma, not a separator. _UNESCAPED_COMMA = re.compile(r"(?`, the secrets file or 1Password, by the user. And a +field that names a program to run or affects a gate: like the Publish-Remote +Gate's allowlist, that one is changed by the user's own edit, never by an agent. +""" +from __future__ import annotations + +import json +from typing import Any, Optional + +import typer + +from chemenu import cli_contract, config, settings +from chemenu.config_shapes import INSTANCE, SECRET, Field +from chemenu.commands._util import fail, needs_clearance, success +from chemenu.errors import ValidationError + +app = typer.Typer(help="Write the tiered settings in wikitool.json and .wikitool-local.json.") + + +def _plain_value(spec: Field, raw: str) -> Any: + """`raw` as the JSON value `spec` takes: a string as given, anything else + parsed as JSON (`true`, `14`, `["a"]`).""" + if spec.json_type in (None, "string"): + return raw + try: + return json.loads(raw) + except ValueError: + raise ValidationError(f"{spec.name} takes a JSON {spec.json_type}, and the value given is not JSON") from None + + +@app.command("set") +@cli_contract.record(cli_contract.CommandRecord( + path="config set", + summary="Set one value of the tiered settings, in the file its tier names.", + synopsis=( + cli_contract.Variant(usage="config set
. "), + cli_contract.Variant(usage="config set
. --ref "), + ), + properties=cli_contract.Properties( + effect=cli_contract.Effect.WRITE, + idempotent=cli_contract.Idempotent.YES, + atomic="Each file is replaced in one rename; both are checked before either is written", + budget=cli_contract.Budget.COUNTED, + ), + notes=( + "Writes a plain value of a field of tier `instance` into `wikitool.json`, and one of tier " + "`checkout` - a private field's included - into `.wikitool-local.json`.", + "With `--ref`, writes a reference instead of a value into `wikitool.json`: `` " + "resolves through `WIKITOOL_SECRET_`, then the secrets file in the user's " + "configuration directory; `op:////` resolves through the 1Password " + "CLI. Only a secret or private field takes one.", + "Removes the same key from the other file, so a value never has two places, and says so.", + "Creates `wikitool.json` with a fresh `instance_id` the first time it writes anything; " + "the secrets file is found by that id.", + "Checks both files as they would stand after the write against every registered section, " + "and writes nothing when that fails.", + "Prints the key and the file it wrote, never the value. Resolves no reference.", + ), + failures=( + cli_contract.Failure( + cause="The key names no registered section or field, or is not `
.`", + reaction="Use a key from the list the message prints; `docs/reference/configuration.md` " + "describes the sections", + ), + cli_contract.Failure( + cause="A plain value given for a field of tier `secret`", + reaction="Do not retry with the value. Ask the user to put it into " + "`WIKITOOL_SECRET_`, the secrets file or 1Password, then set the field with " + "`--ref`", + ), + cli_contract.Failure( + cause="`--ref` given for a field that takes a plain value, or a malformed reference", + reaction="Give the value itself, or fix the reference to `` (lower case, digits, " + "`_`) or `op:////`", + ), + cli_contract.Failure( + cause="The value is not of the field's type, or the files would not pass their check " + "afterwards", + reaction="Fix the value or the problem the message names in the file it names, then " + "re-run once", + ), + cli_contract.Failure( + cause="The field names a program to run or affects a gate", + reaction="Show the user the output verbatim and stop. Only the user sets this field, " + "by editing `.wikitool-local.json` themselves", + code=42, + ), + ), + examples=( + "tools/wikitool config set tracker.stale_days 14", + "tools/wikitool config set tracker.token --ref tracker_token", + "tools/wikitool config set tracker.url --ref op://Chemenu/tracker/url", + ), + never=( + "Never pass a secret's value on the command line - not to this command and not to any other.", + "Never read the secrets file or run `op read` yourself to see a value.", + ), + see_also=( + "`wikitool doctor` - whether both files pass and every reference resolves", + "`docs/reference/configuration.md` - the files, the tiers and the references", + "`docs/how-to/use-1password-for-settings.md` - setting 1Password up as the source", + ), +)) +def set_command( + key: str = typer.Argument(..., help="
."), + value: Optional[str] = typer.Argument(None, help="The plain value; a string as given, anything else as JSON."), + ref: Optional[str] = typer.Option(None, "--ref", help="A reference instead of a value: or op:////."), +): + """Set one value of the tiered settings, in the file its tier names.""" + if (value is None) == (ref is None): + fail("Give either a value or --ref, not both and not neither") + try: + shape, spec = settings.find_field(key) + except ValidationError as exc: + fail(str(exc)) + if spec.user_only: + needs_clearance( + f"{key} names a program to run or affects a gate, so only the user sets it - by " + f"editing {config.LOCAL_SETTINGS_FILENAME} at the root of this checkout themselves. " + "`wikitool doctor` checks the file afterwards." + ) + + if ref is not None: + if not spec.takes_ref: + fail(f"{key} takes a plain value, not a reference - only a secret or a private field takes one") + stored: Any = {settings.REF_KEY: ref} + try: + settings.parse_ref(stored) + except ValidationError as exc: + fail(f"{key}: {exc}") + in_instance = True + else: + if spec.tier == SECRET: + fail(f"{key} is tier {SECRET!r}: its value never passes through `config set`, where " + "whoever runs the command would see it. Put it into " + f"{config.ENV_SECRET_PREFIX}, the secrets file or 1Password, then set " + f"{key} with --ref.") + try: + stored = _plain_value(spec, value) + except ValidationError as exc: + fail(f"{key}: {exc}") + in_instance = spec.tier == INSTANCE + + instance_file, local_file = settings.instance_path(), settings.local_path() + try: + instance_data = settings.read_file(instance_file) + local_data = settings.read_file(local_file) + except ValidationError as exc: + fail(str(exc)) + instance_data = dict(instance_data or {}) + local_data = dict(local_data or {}) + instance_data.setdefault("schema", settings.SCHEMA) + created_id = settings.INSTANCE_ID_KEY not in instance_data + instance_data.setdefault(settings.INSTANCE_ID_KEY, settings.new_instance_id()) + + target, other = (instance_data, local_data) if in_instance else (local_data, instance_data) + target.setdefault("schema", settings.SCHEMA) + target_section = dict(target.get(shape.key) or {}) + target_section[spec.name] = stored + target[shape.key] = target_section + removed = False + other_section = other.get(shape.key) + if isinstance(other_section, dict) and spec.name in other_section: + other_section = {k: v for k, v in other_section.items() if k != spec.name} + if other_section: + other[shape.key] = other_section + else: + del other[shape.key] + removed = True + + write_local = not in_instance or removed or local_file.exists() + try: + settings.check_files(instance_data, local_data if (write_local or local_data) else None, + complete=False) + except ValidationError as exc: + fail(f"Nothing written - the settings would not pass their check:\n{exc}") + + settings.write_file(instance_file, instance_data) + if write_local: + settings.write_file(local_file, local_data) + + where = config.INSTANCE_SETTINGS_FILENAME if in_instance else config.LOCAL_SETTINGS_FILENAME + other_name = config.LOCAL_SETTINGS_FILENAME if in_instance else config.INSTANCE_SETTINGS_FILENAME + what = "reference" if ref is not None else "value" + message = f"{key}: {what} set in {where}" + if removed: + message += f"; removed from {other_name}" + if created_id: + message += f" ({config.INSTANCE_SETTINGS_FILENAME} created with a new {settings.INSTANCE_ID_KEY})" + success(message) diff --git a/tools/chemenu/commands/docs_verify.py b/tools/chemenu/commands/docs_verify.py index 8b3e9bb..5e2d778 100644 --- a/tools/chemenu/commands/docs_verify.py +++ b/tools/chemenu/commands/docs_verify.py @@ -176,6 +176,11 @@ REQUIRED_IGNORE_CANARIES = ( # `raw capture`'s git cache (Gitea #177) - fetched repositories, recomputable, # and in the way of `publish`'s `git add -A` like the coverage output above. "tools/.wikitool_capture/0123abcd/HEAD", + # The checkout tier of the settings (`chemenu.settings`): true for one clone + # on one machine, so a committed copy would hand every other clone a value + # that is wrong for it. Its counterpart `wikitool.json` is in + # REQUIRED_TRACKED_PATHS below. + ".wikitool-local.json", ) REQUIRED_TRACKED_PATHS = ( "reports/CONTRACT.md", @@ -196,6 +201,10 @@ REQUIRED_TRACKED_PATHS = ( # the directory form would silently drop this file again; this canary is # what makes that regression fail loudly instead. "incoming/.gitkeep", + # The instance tier of the settings: the one settings file meant to be + # committed, so every clone of the instance reads the same values. A careless + # `*.json` or `wikitool*` pattern would quietly make it per-checkout. + "wikitool.json", ) CLI_README = config.ROOT / "tools" / "CONTRACT.md" @@ -1116,13 +1125,30 @@ def check_ignored_content() -> list[str]: wrongly_ignored = _check_ignore(REQUIRED_TRACKED_PATHS) if wrongly_ignored is not None: issues += [ - f"`{path}` is ignored - it must survive the reports/ ignore rule" + f"`{path}` is ignored - it must stay tracked, whatever the ignore rules around it exclude" for path in REQUIRED_TRACKED_PATHS if path in wrongly_ignored ] return issues +def check_instance_settings() -> list[str]: + """A committed `wikitool.json` holds every value where its tier allows: + above all, no secret and no private field as plain text. The file is public wherever the + instance is, so this is the last stop before a value is published - the + local file is not this check's business, it is never committed.""" + from chemenu import settings + from chemenu.errors import ValidationError + + try: + data = settings.read_file(settings.instance_path()) + except ValidationError as exc: + return [str(exc)] + if data is None: + return [] + return settings.committed_file_problems(data) + + def check_version_changelog() -> list[str]: """`VERSION` must parse, and the newest versioned `CHANGES.md` entry must name it. @@ -1334,7 +1360,10 @@ def check_breaking_change_for_boundary() -> list[str]: "every stage contract is present.", "The `.gitignore` canaries clear in both directions: nothing ignored under " "`raw/`/`kb/`, `incoming/` ignored, everything ignored under `reports/` and the " - "published skill directories.", + "published skill directories, `.wikitool-local.json` ignored and `wikitool.json` tracked.", + "A `wikitool.json` at the root holds every value in the place its tier allows - no secret " + "and no private field as plain text, no `checkout` field at all except as a reference - " + "and a well-formed `schema` and `instance_id`. The local file is not read.", "No `.md`/`.template` file `dist export` would ship cites an issue number. A " "`` region is exempt: the check reads the export plan's " "text, from which it is already gone.", @@ -1405,6 +1434,13 @@ def check_breaking_change_for_boundary() -> list[str]: reaction="Rename the file to the type and value it was meant for, delete it, or " "remove its frontmatter block", ), + cli_contract.Failure( + cause="`wikitool.json` holds a secret or private value as plain text, or a value its " + "tier puts elsewhere", + reaction="Show the user the message: a plain value there may already be in the " + "history. Move it as the message says - a secret into 1Password, the secrets file or " + "a variable, set by the user, with only the reference left behind - then re-run", + ), cli_contract.Failure( cause="A shipped `.md`/`.template` cites an issue number", reaction="Say what was decided instead of pointing at where, or move the pointer " @@ -1487,6 +1523,7 @@ def verify(): + check_subtype_templates() + check_legacy_type_blocks() + check_ignored_content() + + check_instance_settings() + check_version_changelog() + check_migration_for_boundary() + check_migration_line_names_documents() diff --git a/tools/chemenu/commands/doctor.py b/tools/chemenu/commands/doctor.py index f4f6954..68afb21 100644 --- a/tools/chemenu/commands/doctor.py +++ b/tools/chemenu/commands/doctor.py @@ -658,6 +658,104 @@ def check_tasks_provider() -> Check: return Check("tasks-provider", "OK", f"provider '{cfg.provider}' configured{via}") +def _settings_file_check(name: str, path, in_instance: bool) -> tuple[Check, Optional[dict], bool]: + """One settings file on its own: absent, unreadable, or read. The third + value says whether the file is usable for the joint check.""" + from chemenu import settings + + try: + data = settings.read_file(path) + except Exception as exc: # ValidationError, and anything else a broken file raises + return Check(name, "FAIL", str(exc), f"Fix or delete {path.name}"), None, False + if data is None: + what = ("no instance settings" if in_instance else "no settings for this checkout") + return Check(name, "OK", f"{path.name} absent - {what}"), None, True + return Check(name, "OK", f"{path.name} present"), data, True + + +def check_settings() -> list[Check]: + """The tiered settings, one line per tier: `wikitool.json` (instance), + `.wikitool-local.json` (checkout), and the references both carry (secret). + + A reference is reported as resolvable or not, never with its value. One + 1Password cannot open without a human - locked, signed out, waiting for an + approval - is a `WARN`: nothing is wrong with the configuration, and a + `FAIL` would make `doctor` red whenever the app is locked.""" + from chemenu import settings + from chemenu.errors import ValidationError + + instance_check, instance_data, instance_ok = _settings_file_check( + "settings-instance", settings.instance_path(), True) + local_check, local_data, local_ok = _settings_file_check( + "settings-checkout", settings.local_path(), False) + if not (instance_ok and local_ok): + return [instance_check, local_check, + Check("settings-secrets", "WARN", "not checked - a settings file is unreadable", + "See the settings line above")] + if instance_data is None and local_data is None: + return [instance_check, local_check, Check("settings-secrets", "OK", "no references")] + + try: + found = settings.check_files(instance_data, local_data) + except ValidationError as exc: + # A problem names the local file when it is that file's; everything + # else - the committed file's own, a value set in both, a required + # field nobody set - is reported on the instance line. + local_prefix = f"{config.LOCAL_SETTINGS_FILENAME}: " + problems = str(exc).splitlines() + for check, own in ((local_check, [p for p in problems if p.startswith(local_prefix)]), + (instance_check, [p for p in problems if not p.startswith(local_prefix)])): + if own: + check.status, check.detail = "FAIL", "; ".join(own) + check.fix = ("Fix the file as the message says - `wikitool config set` writes a " + "value where its tier puts it") + return [instance_check, local_check, + Check("settings-secrets", "WARN", "not checked - the settings files do not pass", + "See the settings lines above")] + + for check, data, file_name in ((instance_check, instance_data, config.INSTANCE_SETTINGS_FILENAME), + (local_check, local_data, config.LOCAL_SETTINGS_FILENAME)): + if data is None: + continue + known = sorted(k for k in data if k not in ("schema", settings.INSTANCE_ID_KEY) + and k not in found.unknown.get(file_name, [])) + check.detail = f"{file_name} present, sections: {', '.join(known) or 'none'}" + if file_name == config.INSTANCE_SETTINGS_FILENAME and found.instance_id: + check.detail += f"; instance id {found.instance_id}" + unknown = found.unknown.get(file_name) + if unknown: + check.status = "WARN" + check.detail += f"; not read by this version: {', '.join(unknown)}" + check.fix = "A newer stack may write these; otherwise remove them" + + refs = list(found.refs()) + if not refs: + return [instance_check, local_check, Check("settings-secrets", "OK", "no references")] + resolvable, unresolved, locked = [], [], [] + for key, ref in refs: + try: + settings.resolve(ref, key=key, instance_id=found.instance_id) + except settings.SecretSourceLocked: + locked.append(key) + except ValidationError as exc: + unresolved.append(str(exc)) + else: + resolvable.append(key) + summary = f"{len(resolvable)} of {len(refs)} reference(s) resolvable" + if unresolved: + return [instance_check, local_check, Check( + "settings-secrets", "FAIL", summary + "; not resolvable: " + "; ".join(unresolved), + "Provide each value where its message says - by the user, never by `config set`", + )] + if locked: + return [instance_check, local_check, Check( + "settings-secrets", "WARN", + summary + f"; 1Password would not answer for: {', '.join(locked)}", + "Unlock the 1Password app or run `op signin`, then run doctor again", + )] + return [instance_check, local_check, Check("settings-secrets", "OK", summary)] + + def check_session_id() -> Check: """Three-valued, not two: an explicit `WIKITOOL_SESSION_ID` and a recognised harness variable (see `chemenu.session.HARNESS_ENV_VARS`) both @@ -778,6 +876,7 @@ def run_doctor() -> list[Check]: check_publish_remotes(), check_upload_intake(), check_tasks_provider(), + *check_settings(), check_generated_files(), check_session_id(), check_telemetry(), @@ -835,6 +934,13 @@ def run_doctor() -> list[Check]: "path is never attempted, and neither state is ever a `FAIL`.", "For a configured `caldav` provider, whether the server is reachable and Basic auth " "succeeds - never a `FAIL`; only a broken config block is.", + "Settings (`settings-instance`, `settings-checkout`, `settings-secrets`): one line per " + "tier. `wikitool.json` and `.wikitool-local.json` each present or absent - absent is " + "`OK` - and a `FAIL` naming file, key and the right tier when a value sits in the wrong " + "file, a secret or a committed private field holds a plain value, or a value fails its " + "field; a section this version does not read is a `WARN`. For the references: how many " + "resolve, never a value - an unresolvable one is a `FAIL`, one 1Password will not hand " + "out until a human unlocks it a `WARN`.", "Session id source: `OK` for `WIKITOOL_SESSION_ID` or a registered harness variable, " "`WARN` only for the bare parent-pid fallback.", "Telemetry: on or off and why - installation-form default, `.wikitool-telemetry.json`, " @@ -859,7 +965,8 @@ def run_doctor() -> list[Check]: see_also=( "`instructions/setup-instance.md` - the setup steps most findings point back to", "`instructions/preflight.md` - what `tool-paths` and `install-dir` point back to", - "`docs/reference/configuration.md` - the per-checkout configuration files", + "`docs/reference/configuration.md` - the per-checkout configuration files and the tiered " + "settings", "`docs/explanation/telemetry-and-evaluation.md` - telemetry state and caps", "`instructions/session-setup.md` - setting `WIKITOOL_SESSION_ID`", ), diff --git a/tools/chemenu/config.py b/tools/chemenu/config.py index 84f6c4f..2a425a0 100644 --- a/tools/chemenu/config.py +++ b/tools/chemenu/config.py @@ -274,6 +274,43 @@ TASKS_CONFIG_FILENAME = ".wikitool-tasks.json" ENV_TASKS_CONFIG = "WIKITOOL_TASKS_CONFIG" +# The tiered settings files (`chemenu.settings`). Every field of a settings +# section belongs to exactly one tier, declared on its `config_shapes.Field`: +# `instance` values live in `wikitool.json`, which is committed and the same in +# every clone of the instance; `checkout` values live in `.wikitool-local.json`, +# which is gitignored and true only for this clone on this machine; `secret` +# values live in neither - both files hold only a reference to them. +# +# `wikitool.json` has no leading dot because it is meant to be seen and +# committed. `dist export` does not ship it - it belongs to the instance, and the +# first `wikitool config set` creates it - so nothing in a distribution names it +# but this constant. +INSTANCE_SETTINGS_FILENAME = "wikitool.json" +LOCAL_SETTINGS_FILENAME = ".wikitool-local.json" + +# A `{"ref": ""}` reference resolves through `WIKITOOL_SECRET_` +# first, then through the secrets file below. A family of variables rather +# than one, so `_WIKITOOL_ENV` in tools/chemenu/tests/conftest.py clears it by +# prefix. +ENV_SECRET_PREFIX = "WIKITOOL_SECRET_" + + +def secrets_file(instance_id: str) -> Path: + """Where a `{"ref": ""}` reference looks once no variable answers it: + a file in the user's own configuration directory, outside every working + tree, keyed by the instance id `wikitool.json` carries so two instances on + one machine keep their secrets apart. + + `%APPDATA%` on Windows; `$XDG_CONFIG_HOME`, else `~/.config`, everywhere + else.""" + if os.name == "nt" and os.environ.get("APPDATA"): + base = Path(os.environ["APPDATA"]) + else: + xdg = os.environ.get("XDG_CONFIG_HOME", "").strip() + base = Path(xdg) if xdg else Path.home() / ".config" + return base / "chemenu" / instance_id / "secrets.json" + + def tasks_config_override() -> "Path | None": """The file `$WIKITOOL_TASKS_CONFIG` names, or `None` when it is unset or blank.""" value = os.environ.get(ENV_TASKS_CONFIG, "").strip() diff --git a/tools/chemenu/config_shapes.py b/tools/chemenu/config_shapes.py index d8b1b14..451e7d4 100644 --- a/tools/chemenu/config_shapes.py +++ b/tools/chemenu/config_shapes.py @@ -20,6 +20,12 @@ has, which is a different kind of change than writing the shape down. hook handler reads it on every tool call. `deviations()` is the strict reading `doctor` uses to report what that silence would otherwise hide. +The same records describe the sections of the two tiered settings files, +`wikitool.json` and `.wikitool-local.json` (`SETTINGS_SECTIONS`, read by +`chemenu.settings`). There every field also declares the tier it belongs to, +and the rule that a field naming a program or affecting a gate is never +`instance` is checked when the `Field` is declared (`Field.__post_init__`). + The three machine-written files beside them - `.wikitool-release.json`, `.wikitool-kb.json`, `.wikitool-tools.json` - are deliberately not here: nobody writes them by hand, so they have no shape for an operator to look up. @@ -39,6 +45,14 @@ from chemenu import config IGNORE = "ignore" REJECT = "reject" +# The three tiers a settings field can belong to (`chemenu.settings`). A field +# of one of the four per-feature files above carries none: those files predate +# the tiers and are read by their own loaders. +INSTANCE = "instance" +CHECKOUT = "checkout" +SECRET = "secret" +TIERS = (INSTANCE, CHECKOUT, SECRET) + _ABSENT = object() @@ -56,7 +70,17 @@ class Field: applies to the value it finds - it may accept more than `json_type`, which is the leniency `deviations()` reports - and raises `TypeError`/`ValueError` on a value it cannot use. `check` then judges the coerced value and returns - a problem, or None. A field with `fields` is a nested object.""" + a problem, or None. A field with `fields` is a nested object. + + The last four attributes matter only to a settings section + (`chemenu.settings`). `tier` says which file the field's value lives in. + `private` marks private infrastructure - a host name, an internal URL - that + may sit in the committed `wikitool.json` only as a reference, never as + plain text. `names_command` and `gate` mark a field that names a program to + run or that loosens or arms a gate. Such a field is never `instance`: a + `git pull` from another clone must not change what `wikitool` executes or + where it pushes. That rule is enforced here, when the field is declared, + rather than by whoever reads it.""" name: str type: str @@ -72,6 +96,41 @@ class Field: required_why: str = "" lenient: str = "" fields: tuple["Field", ...] = () + tier: str = "" + private: bool = False + names_command: bool = False + gate: bool = False + + def __post_init__(self) -> None: + where = f"config_shapes: field {self.name!r}" + if self.tier and self.tier not in TIERS: + raise ValueError(f"{where}: tier must be one of {', '.join(TIERS)}, not {self.tier!r}") + if not self.tier and (self.private or self.names_command or self.gate): + raise ValueError(f"{where}: `private`, `names_command` and `gate` need a tier") + if self.tier == INSTANCE and self.user_only: + raise ValueError( + f"{where}: a field that names a program to run or affects a gate is never " + f"tier {INSTANCE!r} - a pull from another clone would change what runs or " + f"where it pushes; declare it {CHECKOUT!r}" + ) + if self.private and self.tier != CHECKOUT: + raise ValueError( + f"{where}: a private field is tier {CHECKOUT!r} - its plain value never goes " + "into the committed file, only a reference to it may" + ) + if self.tier and self.fields: + raise ValueError(f"{where}: a settings field is a single value, not a nested object") + + @property + def user_only(self) -> bool: + """Set only by the user's own edit, never through `wikitool config set`.""" + return self.names_command or self.gate + + @property + def takes_ref(self) -> bool: + """Whether a `{"ref": ...}` may stand in for the value - required for a + secret, optional for a private field, refused everywhere else.""" + return self.tier == SECRET or self.private @dataclass(frozen=True) @@ -393,6 +452,13 @@ TASKS = Shape( SHAPES = (TELEMETRY, REMOTES, UPLOAD, TASKS) +# The sections of the tiered settings files, `wikitool.json` and +# `.wikitool-local.json` (`chemenu.settings`). Each is a `Shape` whose `key` is +# the section's key in those files and whose every field declares its `tier`. +# Empty until the first feature reads its settings this way; `chemenu.settings` +# reads the tuple at call time, so a test can stand in a section of its own. +SETTINGS_SECTIONS: tuple[Shape, ...] = () + # --------------------------------------------------------------------------- # Checking a value against a record diff --git a/tools/chemenu/prerequisites.py b/tools/chemenu/prerequisites.py index bc99cc3..1f72b65 100644 --- a/tools/chemenu/prerequisites.py +++ b/tools/chemenu/prerequisites.py @@ -47,10 +47,19 @@ class Tool: why: str +@dataclass(frozen=True) +class OptionalTool: + """A tool the preflight records when it finds it and never asks for.""" + name: str + label: str + why: str + + @dataclass(frozen=True) class Manifest: limits: dict[str, int] tools: tuple[Tool, ...] + optional: tuple[OptionalTool, ...] = () def tools_for(self, platform: str) -> tuple[Tool, ...]: return tuple(t for t in self.tools if t.platforms in ("all", platform)) @@ -64,6 +73,7 @@ def load_manifest(path: Optional[Path] = None) -> Manifest: path = manifest_path() if path is None else path limits: dict[str, int] = {} tools: list[Tool] = [] + optional: list[OptionalTool] = [] for line in path.read_text(encoding="utf-8").splitlines(): if not line.strip() or line.startswith("#"): continue @@ -73,7 +83,9 @@ def load_manifest(path: Optional[Path] = None) -> Manifest: elif fields[0] == "tool": minimum = None if fields[2] == "-" else fields[2] tools.append(Tool(fields[1], minimum, fields[3], fields[4], fields[5])) - return Manifest(limits, tuple(tools)) + elif fields[0] == "optional": + optional.append(OptionalTool(fields[1], fields[2], fields[3])) + return Manifest(limits, tuple(tools), tuple(optional)) def platform() -> str: diff --git a/tools/chemenu/settings.py b/tools/chemenu/settings.py new file mode 100644 index 0000000..113cdbc --- /dev/null +++ b/tools/chemenu/settings.py @@ -0,0 +1,527 @@ +"""The tiered settings: one committed file, one local file, and secrets outside the tree. + +| Tier | File | Holds | +|---|---|---| +| `instance` | `wikitool.json`, committed | what belongs to the corpus and is the same in every clone | +| `checkout` | `.wikitool-local.json`, gitignored | what is true only for this clone on this machine | +| `secret` | neither | tokens and passwords; both files hold only a reference to them | + +Every field of a section belongs to exactly one tier, declared on its +`config_shapes.Field`. A value in the wrong file is an error, never an override: +with two sources for one value, every reader would first have to work out which +one wins, and the answer would differ between clones. + +**References instead of values.** A secret field takes only a reference, and a +private one (`Field.private`: host names, internal URLs) may take one: + +- `{"ref": ""}` - `WIKITOOL_SECRET_` first, for CI and containers, + then the secrets file `config.secrets_file()` in the user's own + configuration directory, which must be readable by its owner only; +- `{"ref": "op:////"}` - `op read`, the 1Password CLI the + preflight recorded in `.wikitool-tools.json`. + +A reference is resolved when a reader asks for that one value, never while the +files are read and checked, and the value stays in this process's memory - it +is never cached on disk and never printed. A private field's plain value may sit +in the local file only: an instance that is public commits `wikitool.json` +publicly. As a reference it may sit in either, which is what makes the committed +file portable across every machine with the same 1Password account. + +The sections themselves are `config_shapes.SETTINGS_SECTIONS`, read at call +time so a test can stand in a section of its own. Unknown top-level keys are +reported (`unknown`), not refused: a section a newer stack writes must not stop +an older one from reading the sections it knows. + +**What this does not protect against**, stated where the mechanism is: an +agent with a shell, in the same user account, can run `op read` or read the +secrets file itself. Keeping secrets out of the tree keeps them out of commits, +out of files an agent reads in the course of ordinary work, and out of +`wikitool` output - not out of reach. docs/explanation/configuration-tiers.md +has the rest. +""" +from __future__ import annotations + +import json +import os +import re +import subprocess +import tempfile +import uuid +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any, Iterator, Optional + +from chemenu import config, config_shapes, toolpaths +from chemenu.config_shapes import CHECKOUT, INSTANCE, SECRET, Field, Shape +from chemenu.errors import ChemenuError, ValidationError + +SCHEMA = 1 +INSTANCE_ID_KEY = "instance_id" +REF_KEY = "ref" +OP_SCHEME = "op://" +OP_TIMEOUT_SECONDS = 60 + +# Lower case, so `WIKITOOL_SECRET_` is a plain upper-casing of it and two +# names cannot collapse onto one variable. +_NAME_RE = re.compile(r"^[a-z0-9]+(?:_[a-z0-9]+)*$") +# A directory name under the user's configuration directory: no separator, no +# leading dot, so it can never climb out of `chemenu/`. +_INSTANCE_ID_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_-]{0,63}$") +# What `op read` says when the reference itself is wrong, as opposed to +# 1Password being out of reach. Everything else is read as "a human has to +# unlock, sign in or approve" - the conservative side of an unknown message. +_OP_NOT_FOUND = re.compile( + r"isn't (?:an item|a vault|a field)|does not have a field|no item found|could not find item", + re.IGNORECASE, +) + + +class SecretSourceLocked(ChemenuError): + """1Password did not hand out a value because a human has to act first - + unlock it, sign in, or approve the request. The CLI renders this as exit 42: + not an error in the input, and nothing an agent can fix by retrying.""" + + +@dataclass(frozen=True) +class Ref: + """A reference in place of a value: a name, or an `op://` URI.""" + + target: str + + @property + def is_op(self) -> bool: + return self.target.startswith(OP_SCHEME) + + @property + def env_var(self) -> str: + return config.ENV_SECRET_PREFIX + self.target.upper() + + def __str__(self) -> str: + return json.dumps({REF_KEY: self.target}) + + +def is_ref(value: Any) -> bool: + """Whether `value` is shaped as a reference at all - an object carrying + `ref`. Whether it is a *valid* one is `ref_problem`'s question.""" + return isinstance(value, dict) and REF_KEY in value + + +def ref_problem(value: Any) -> Optional[str]: + """Why `value` is not a valid reference, or None when it is one.""" + if not isinstance(value, dict) or set(value) != {REF_KEY}: + return f'must be a reference of the form {{"{REF_KEY}": ""}}, with no other key' + target = value[REF_KEY] + if not isinstance(target, str): + return "has a reference that is not a string" + if target.startswith(OP_SCHEME): + path = target[len(OP_SCHEME):].split("?", 1)[0] + parts = path.split("/") + if len(parts) < 3 or not all(part.strip() for part in parts): + return (f"has a 1Password reference that is not {OP_SCHEME}// " + f"(or {OP_SCHEME}//
/)") + return None + if not _NAME_RE.match(target): + return (f"has a reference name that is neither {OP_SCHEME}... nor lower-case letters, " + "digits and single underscores") + return None + + +def parse_ref(value: Any) -> Ref: + """`value` as a `Ref`; raises `ValidationError` naming what is wrong.""" + problem = ref_problem(value) + if problem: + raise ValidationError(f"the reference {problem}") + return Ref(value[REF_KEY]) + + +# --------------------------------------------------------------------------- +# The sections +# --------------------------------------------------------------------------- + + +def section_problems(shape: Shape) -> list[str]: + """What keeps `shape` from being a settings section: every field tiered, + no discriminator and no sub-sections - a field's tier has to be readable off + the field alone.""" + where = f"settings section {shape.key!r}" + problems = [f"{where}: field {spec.name!r} has no tier" for spec in shape.fields if not spec.tier] + if shape.discriminator or shape.sections: + problems.append(f"{where}: a settings section takes neither variants nor sub-sections") + if shape.key in (INSTANCE_ID_KEY, "schema"): + problems.append(f"{where}: {shape.key!r} is a key of the file itself") + return problems + + +def sections() -> dict[str, Shape]: + """The registered settings sections, by key. Raises `ValueError` on a + section that is not one - a programming error, not a configuration one.""" + found: dict[str, Shape] = {} + for shape in config_shapes.SETTINGS_SECTIONS: + problems = section_problems(shape) + if problems: + raise ValueError("; ".join(problems)) + if shape.key in found: + raise ValueError(f"settings section {shape.key!r} is registered twice") + found[shape.key] = shape + return found + + +def find_field(key: str, known: Optional[dict[str, Shape]] = None) -> tuple[Shape, Field]: + """The section and field a `
.` key names; raises + `ValidationError` naming what exists instead.""" + known = sections() if known is None else known + section_name, dot, field_name = key.partition(".") + if not dot or not section_name or not field_name: + raise ValidationError(f"{key!r} is not a settings key - write it as
.") + shape = known.get(section_name) + if shape is None: + offered = ", ".join(sorted(known)) or "none is registered in this version" + raise ValidationError(f"no settings section {section_name!r} (sections: {offered})") + for spec in shape.fields: + if spec.name == field_name: + return shape, spec + names = ", ".join(spec.name for spec in shape.fields) + raise ValidationError(f"section {section_name!r} has no field {field_name!r} (fields: {names})") + + +# --------------------------------------------------------------------------- +# Reading and checking +# --------------------------------------------------------------------------- + + +def instance_path(root: "Path | str | None" = None) -> Path: + return Path(config.ROOT if root is None else root) / config.INSTANCE_SETTINGS_FILENAME + + +def local_path(root: "Path | str | None" = None) -> Path: + return Path(config.ROOT if root is None else root) / config.LOCAL_SETTINGS_FILENAME + + +def read_file(path: Path) -> Optional[dict]: + """The parsed file, or None when it does not exist. Raises + `ValidationError` for one that is not a JSON object.""" + try: + text = path.read_text(encoding="utf-8") + except FileNotFoundError: + return None + except OSError as exc: + raise ValidationError(f"{path.name} cannot be read ({exc})") from exc + try: + data = json.loads(text) + except ValueError as exc: + raise ValidationError(f"{path.name} is not valid JSON ({exc})") from exc + if not isinstance(data, dict): + raise ValidationError(f"{path.name} must contain a JSON object") + return data + + +def _file_label(in_instance: bool) -> str: + return config.INSTANCE_SETTINGS_FILENAME if in_instance else config.LOCAL_SETTINGS_FILENAME + + +def placement_problem(spec: Field, value: Any, in_instance: bool, key: str) -> Optional[str]: + """Why `value` may not stand for `spec` in this file, or None. Never + quotes the value: a plain secret is exactly what must not be printed.""" + here = _file_label(in_instance) + if is_ref(value): + problem = ref_problem(value) + if problem: + return f"{here}: {key} {problem}" + if not spec.takes_ref: + return (f"{here}: {key} takes a plain value, not a reference - only a secret or a " + "private field takes one") + if in_instance and spec.user_only: + return (f"{here}: {key} names a program to run or affects a gate, so it is never in " + f"{config.INSTANCE_SETTINGS_FILENAME}, not even as a reference - it belongs to " + f"tier {CHECKOUT!r}, in {config.LOCAL_SETTINGS_FILENAME}") + return None + if spec.tier == SECRET: + return (f"{here}: {key} is tier {SECRET!r} and holds a plain value - a secret is only ever " + f'a reference, {{"{REF_KEY}": ""}} or {{"{REF_KEY}": "{OP_SCHEME}..."}}; ' + f"the value itself goes into {config.ENV_SECRET_PREFIX}, the secrets file " + "or 1Password") + if spec.private and in_instance: + return (f"{here}: {key} is private and holds a plain value in the committed file - move " + f"it to {config.LOCAL_SETTINGS_FILENAME} (tier {CHECKOUT!r}), or replace it with a " + "reference") + if (spec.tier == INSTANCE) != in_instance: + return (f"{here}: {key} belongs to tier {spec.tier!r} - move it to " + f"{_file_label(spec.tier == INSTANCE)}") + return None + + +def _envelope_problems(data: Optional[dict], in_instance: bool) -> list[str]: + if data is None: + return [] + here = _file_label(in_instance) + problems = [] + if "schema" in data and data["schema"] != SCHEMA: + problems.append(f"{here}: schema must be {SCHEMA}") + if INSTANCE_ID_KEY in data: + value = data[INSTANCE_ID_KEY] + if not in_instance: + problems.append(f"{here}: {INSTANCE_ID_KEY} belongs to tier {INSTANCE!r} - move it to " + f"{config.INSTANCE_SETTINGS_FILENAME}") + elif not isinstance(value, str) or not _INSTANCE_ID_RE.match(value): + problems.append(f"{here}: {INSTANCE_ID_KEY} must be letters, digits, '-' and '_', " + "starting with a letter or digit") + return problems + + +@dataclass +class Section: + """One section as both files give it: plain values (coerced, defaults + filled in) and references, resolved only on access.""" + + shape: Shape + values: dict = field(default_factory=dict) + refs: dict = field(default_factory=dict) + given: set = field(default_factory=set) + instance_id: Optional[str] = None + + @property + def configured(self) -> bool: + return bool(self.given) + + def __getitem__(self, name: str) -> Any: + if name in self.refs: + return resolve(self.refs[name], key=f"{self.shape.key}.{name}", instance_id=self.instance_id) + return self.values[name] + + def get(self, name: str, default: Any = None) -> Any: + if name in self.refs or name in self.values: + return self[name] + return default + + +@dataclass +class Settings: + instance_present: bool = False + local_present: bool = False + instance_id: Optional[str] = None + by_section: dict = field(default_factory=dict) + unknown: dict = field(default_factory=dict) + + def section(self, name: str) -> Section: + """The section `name`, empty (not `configured`) when no file sets it.""" + found = self.by_section.get(name) + if found is not None: + return found + shape = sections()[name] + empty = config_shapes.check(shape, {}).values + return Section(shape, values=empty, instance_id=self.instance_id) + + def refs(self) -> Iterator[tuple[str, Ref]]: + """Every reference in either file, as `(
., ref)`.""" + for name, section in sorted(self.by_section.items()): + for field_name, ref in sorted(section.refs.items()): + yield f"{name}.{field_name}", ref + + +def check_files( + instance_data: Optional[dict], local_data: Optional[dict], + known: Optional[dict[str, Shape]] = None, *, complete: bool = True, +) -> Settings: + """Both files' contents checked against the registered sections. Raises + `ValidationError` listing every problem, one per line; resolves nothing. + + `complete=False` is the check for a write in progress (`config set`): a + required field nobody has set yet is not a problem there, or the first + value of a section could never be written.""" + known = sections() if known is None else known + problems = _envelope_problems(instance_data, True) + _envelope_problems(local_data, False) + settings = Settings(instance_present=instance_data is not None, local_present=local_data is not None) + if instance_data is not None and isinstance(instance_data.get(INSTANCE_ID_KEY), str): + settings.instance_id = instance_data[INSTANCE_ID_KEY] + + merged: dict[str, dict[str, tuple[Any, bool]]] = {} + for data, in_instance in ((instance_data, True), (local_data, False)): + if data is None: + continue + here = _file_label(in_instance) + for name, body in data.items(): + if name in ("schema", INSTANCE_ID_KEY): + continue + shape = known.get(name) + if shape is None: + settings.unknown.setdefault(here, []).append(name) + continue + if not isinstance(body, dict): + problems.append(f"{here}: {name} must be an object") + continue + fields = {spec.name: spec for spec in shape.fields} + for field_name, value in body.items(): + key = f"{name}.{field_name}" + spec = fields.get(field_name) + if spec is None: + problems.append(f"{here}: {key} is not a field of {name!r} " + f"(fields: {', '.join(fields)})") + continue + problem = placement_problem(spec, value, in_instance, key) + if problem: + problems.append(problem) + continue + if field_name in merged.setdefault(name, {}): + problems.append(f"{key} is set in both {config.INSTANCE_SETTINGS_FILENAME} and " + f"{config.LOCAL_SETTINGS_FILENAME} - a value has exactly one place") + continue + merged[name][field_name] = (value, in_instance) + + for name, entries in merged.items(): + shape = known[name] + plain = {k: v for k, (v, _) in entries.items() if not is_ref(v)} + refs = {k: Ref(v[REF_KEY]) for k, (v, _) in entries.items() if is_ref(v)} + checked = config_shapes.check(shape, plain) + for problem in checked.problems: + if problem.field in refs: + continue # "missing" only because a reference stands in for it + if not complete and problem.field not in entries: + continue # not set yet - the write in progress may be the first of several + origin = entries.get(problem.field) + where = f"{_file_label(origin[1])}: " if origin else "" + problems.append(f"{where}{name}.{problem.message}") + settings.by_section[name] = Section( + shape, values=checked.values, refs=refs, + given=set(checked.given) | set(refs), instance_id=settings.instance_id, + ) + + if problems: + raise ValidationError("\n".join(problems)) + return settings + + +def committed_file_problems(instance_data: dict, known: Optional[dict[str, Shape]] = None) -> list[str]: + """What is wrong with `wikitool.json` on its own: its envelope, and every + value in a place its tier forbids - a plain secret or private value above + all. Unlike `check_files`, it does not ask for a required field: that one + may live in a local file this check never sees.""" + known = sections() if known is None else known + problems = _envelope_problems(instance_data, True) + for name, body in instance_data.items(): + shape = known.get(name) + if shape is None or not isinstance(body, dict): + continue + fields = {spec.name: spec for spec in shape.fields} + for field_name, value in body.items(): + spec = fields.get(field_name) + problem = (placement_problem(spec, value, True, f"{name}.{field_name}") if spec else + f"{config.INSTANCE_SETTINGS_FILENAME}: {name}.{field_name} is not a field of {name!r}") + if problem: + problems.append(problem) + return problems + + +def load(root: "Path | str | None" = None) -> Settings: + """Both files under `root` (default `config.ROOT`), read and checked.""" + return check_files(read_file(instance_path(root)), read_file(local_path(root))) + + +# --------------------------------------------------------------------------- +# Resolving a reference +# --------------------------------------------------------------------------- + + +def _op_missing(key: str) -> str: + return (f"{key}: the reference needs the 1Password CLI (`op`), and this checkout has none " + f"recorded - install it, then {toolpaths.PREFLIGHT}, which records it") + + +def _locked(key: str, why: str) -> str: + return (f"{key}: 1Password {why} - it is locked, not signed in, or waiting for approval. " + "Unlock the 1Password app or run `op signin` (in CI: set OP_SERVICE_ACCOUNT_TOKEN), " + "then run this command again.") + + +def read_secrets_file(path: Path) -> Optional[dict]: + """The secrets file, or None when it does not exist. Raises + `ValidationError` when it is readable by anyone but its owner, or not a + JSON object of strings - without quoting any of it.""" + try: + mode = path.stat().st_mode + except FileNotFoundError: + return None + if os.name != "nt" and mode & 0o077: + raise ValidationError(f"{path} is readable by other users - restrict it: chmod 600 {path}") + try: + data = json.loads(path.read_text(encoding="utf-8")) + except (OSError, ValueError) as exc: + raise ValidationError(f"{path} cannot be read as JSON ({type(exc).__name__})") from None + if not isinstance(data, dict) or not all(isinstance(v, str) for v in data.values()): + raise ValidationError(f"{path} must be a JSON object of names to strings") + return data + + +def _resolve_named(ref: Ref, key: str, instance_id: Optional[str]) -> str: + value = os.environ.get(ref.env_var, "") + if value: + return value + if not instance_id: + raise ValidationError( + f"{key}: {ref} cannot be resolved - {ref.env_var} is not set, and " + f"{config.INSTANCE_SETTINGS_FILENAME} has no {INSTANCE_ID_KEY} to find the secrets " + "file by (the first `wikitool config set` writes one)" + ) + path = config.secrets_file(instance_id) + data = read_secrets_file(path) or {} + value = data.get(ref.target, "") + if value: + return value + raise ValidationError( + f"{key}: {ref} cannot be resolved - set {ref.env_var}, or add \"{ref.target}\" to {path}" + ) + + +def _resolve_op(ref: Ref, key: str) -> str: + program = toolpaths.optional("op") + if program is None: + raise ValidationError(_op_missing(key)) + try: + result = subprocess.run( + [program, "read", "--no-newline", ref.target], + capture_output=True, text=True, encoding="utf-8", errors="replace", + stdin=subprocess.DEVNULL, timeout=OP_TIMEOUT_SECONDS, check=False, + ) + except FileNotFoundError: + raise ValidationError(_op_missing(key)) from None + except subprocess.TimeoutExpired: + raise SecretSourceLocked(_locked(key, f"did not answer within {OP_TIMEOUT_SECONDS} s")) from None + except OSError as exc: + raise ValidationError(f"{key}: `op` could not be started ({exc}) - {toolpaths.PREFLIGHT}") from None + if result.returncode == 0: + return result.stdout + if _OP_NOT_FOUND.search(result.stderr or ""): + raise ValidationError( + f"{key}: 1Password has no value at {ref.target} - check the vault, item and field names" + ) + raise SecretSourceLocked(_locked(key, "did not hand out the value")) + + +def resolve(ref: Ref, *, key: str, instance_id: Optional[str]) -> str: + """The value behind `ref`, for the setting `key`. Raises `ValidationError` + when it cannot be found and `SecretSourceLocked` when a human has to unlock + 1Password first. Neither message carries the value.""" + if ref.is_op: + return _resolve_op(ref, key) + return _resolve_named(ref, key, instance_id) + + +# --------------------------------------------------------------------------- +# Writing (`wikitool config set`) +# --------------------------------------------------------------------------- + + +def new_instance_id() -> str: + return str(uuid.uuid4()) + + +def write_file(path: Path, data: dict) -> None: + """Replace `path` with `data` in one rename, so a reader never sees half a file.""" + path.parent.mkdir(parents=True, exist_ok=True) + fd, tmp = tempfile.mkstemp(prefix=f".{path.name}.", suffix=".tmp", dir=str(path.parent)) + try: + with os.fdopen(fd, "w", encoding="utf-8", newline="\n") as handle: + handle.write(json.dumps(data, indent=2, ensure_ascii=False) + "\n") + os.replace(tmp, path) + except BaseException: + Path(tmp).unlink(missing_ok=True) + raise diff --git a/tools/chemenu/tests/conftest.py b/tools/chemenu/tests/conftest.py index 1ee92b5..1e501ae 100644 --- a/tools/chemenu/tests/conftest.py +++ b/tools/chemenu/tests/conftest.py @@ -160,6 +160,10 @@ def hermetic_environment(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): for name in (*_WIKITOOL_ENV, *_GIT_ENV): monkeypatch.delenv(name, raising=False) + # A family rather than a name: `{"ref": ""}` reads + # `WIKITOOL_SECRET_` for any name, so every one is cleared. + for name in [n for n in os.environ if n.startswith(config.ENV_SECRET_PREFIX)]: + monkeypatch.delenv(name, raising=False) # `.wikitool-tools.json` beside this checkout records where the developer's # git and rg live. A test must not depend on whether the preflight has run diff --git a/tools/chemenu/tests/test_dist_cmd.py b/tools/chemenu/tests/test_dist_cmd.py index 0726c37..99a9596 100644 --- a/tools/chemenu/tests/test_dist_cmd.py +++ b/tools/chemenu/tests/test_dist_cmd.py @@ -202,6 +202,19 @@ def test_plan_ships_no_retired_root_docs(repo): assert "README.md" in plan +def test_plan_ships_neither_settings_file(repo): + """`wikitool.json` belongs to the instance that wrote it - its id, its + references - and `.wikitool-local.json` to one checkout. A distribution + carries neither; the first `config set` in the new instance creates its own.""" + (repo / config.INSTANCE_SETTINGS_FILENAME).write_text( + '{"schema": 1, "instance_id": "origin-only"}\n', encoding="utf-8") + (repo / config.LOCAL_SETTINGS_FILENAME).write_text('{"schema": 1}\n', encoding="utf-8") + plan = dist_cmd.build_plan() + assert config.INSTANCE_SETTINGS_FILENAME not in plan + assert config.LOCAL_SETTINGS_FILENAME not in plan + assert "README.md" in plan + + def test_plan_never_includes_instructions_dev(repo): """instructions/dev/ - flat dev-only instructions and the nested skill that switches a session into tool-development mode - is pruned diff --git a/tools/chemenu/tests/test_preflight.py b/tools/chemenu/tests/test_preflight.py index 9e6417f..ad1deea 100644 --- a/tools/chemenu/tests/test_preflight.py +++ b/tools/chemenu/tests/test_preflight.py @@ -185,6 +185,40 @@ def test_complete_path_records_absolute_paths_and_sets_up_the_venv(machine, shel assert "Preflight passed" in result.stdout +@pytest.mark.parametrize("shell", SHELLS) +def test_an_optional_op_is_recorded_when_found(machine, shell): + machine.standard() + machine.stub("op", "#!/bin/sh\necho '2.30.0'\n") + result = machine.run(shell=shell) + assert result.returncode == 0, result.stdout + result.stderr + assert machine.recorded()["tools"]["op"] == str(machine.stubs / "op") + assert any(line.split()[:3] == ["OK", "op", "2.30"] for line in result.stdout.splitlines()) + + +@pytest.mark.parametrize("shell", SHELLS) +def test_an_absent_optional_op_is_no_problem_and_not_recorded(machine, shell): + machine.standard() + result = machine.run(shell=shell) + assert result.returncode == 0, result.stdout + result.stderr + assert "op" not in machine.recorded()["tools"] + assert any(line.split()[:2] == ["ABSENT", "op"] for line in result.stdout.splitlines()) + + +@pytest.mark.parametrize("shell", SHELLS) +def test_set_names_an_optional_tool(machine, shell): + machine.standard() + elsewhere = machine.stub("op", "#!/bin/sh\necho '2.30.0'\n", where=machine.base / "elsewhere") + result = machine.run("--set", f"op={elsewhere}", shell=shell) + assert result.returncode == 0, result.stdout + result.stderr + assert machine.recorded()["tools"]["op"] == str(elsewhere) + + +def test_the_manifest_names_op_as_optional_only(): + manifest = prerequisites.load_manifest() + assert [tool.name for tool in manifest.optional] == ["op"] + assert "op" not in {tool.name for tool in manifest.tools} + + @pytest.mark.parametrize("shell", SHELLS) def test_second_run_changes_nothing(machine, shell): machine.standard() diff --git a/tools/chemenu/tests/test_preflight_pwsh.py b/tools/chemenu/tests/test_preflight_pwsh.py index 93e6c54..30528bf 100644 --- a/tools/chemenu/tests/test_preflight_pwsh.py +++ b/tools/chemenu/tests/test_preflight_pwsh.py @@ -95,6 +95,24 @@ def test_both_preflights_record_the_same_file(machine): assert machine.tools_file.read_bytes() == from_pwsh +@pytest.mark.skipif(not SHELLS, reason="no POSIX shell on this machine") +def test_both_preflights_record_an_optional_op_the_same_way(machine): + machine.stub("op", "#!/bin/sh\necho '2.30.0'\n") + assert _preflight(machine).returncode == 0 + from_pwsh = machine.tools_file.read_bytes() + assert json.loads(from_pwsh)["tools"]["op"] == str(machine.stubs / "op") + machine.tools_file.unlink() + assert machine.run(shell=SHELLS[0]).returncode == 0 + assert machine.tools_file.read_bytes() == from_pwsh + + +def test_an_absent_optional_op_is_no_problem(machine): + result = _preflight(machine) + assert result.returncode == 0, result.stdout + result.stderr + assert "op" not in machine.recorded()["tools"] + assert any(line.split()[:2] == ["ABSENT", "op"] for line in result.stdout.splitlines()) + + def test_help_prints_the_usage_and_exits_0(machine): result = _preflight(machine, "--help") assert result.returncode == 0 diff --git a/tools/chemenu/tests/test_settings.py b/tools/chemenu/tests/test_settings.py new file mode 100644 index 0000000..ad3f738 --- /dev/null +++ b/tools/chemenu/tests/test_settings.py @@ -0,0 +1,574 @@ +"""The tiered settings (`chemenu.settings`), `wikitool config set`, and what +`doctor` and `docs verify` report about them. + +No real section ships yet, so every test stands in `TRACKER`, a section with one +field of each kind: an instance value, a private checkout value, a secret, a +field that names a program, and a plain checkout value. 1Password is a fake `op` +script written into the test's own directory - never a real account; the one +exception is the CI smoke test at the bottom, which only runs where CI has +loaded a real `op://` value into the environment. +""" +from __future__ import annotations + +import json +import os +import shlex +import stat +import subprocess +from pathlib import Path + +import pytest +import typer +from typer.testing import CliRunner + +from chemenu import config, config_shapes, settings, toolpaths +from chemenu.cli import app +from chemenu.commands import _util, docs_verify, doctor +from chemenu.config_shapes import CHECKOUT, INSTANCE, SECRET, Field, Shape +from chemenu.errors import ValidationError + +runner = CliRunner() + +# Captured at import, before the autouse fixture clears every +# `WIKITOOL_SECRET_*` for the test run - see the CI smoke test at the bottom. +_LIVE_VALUE = os.environ.get("WIKITOOL_SECRET_CI_SMOKE", "") +_LIVE_REQUIRED = os.environ.get("CHEMENU_LIVE_SECRET_REQUIRE", "") == "1" + +needs_posix = pytest.mark.skipif(os.name == "nt", reason="the fake `op` is a POSIX shell script") + +SECRET_VALUE = "s3cr3t-never-printed" +PRIVATE_VALUE = "https://tracker.internal.example" + + +def _positive(value: int): + return None if value > 0 else "must be positive" + + +TRACKER = Shape( + key="tracker", title="Test tracker", purpose="A section with one field of each kind.", + fields=( + Field("stale_days", "positive integer", "Instance threshold.", tier=INSTANCE, + json_type="integer", coerce=int, check=_positive, default=14), + Field("url", "string", "Private infrastructure.", tier=CHECKOUT, private=True, + json_type="string", required=True), + Field("token", "string", "A secret.", tier=SECRET, json_type="string"), + Field("program", "string", "Names a program, privately.", tier=CHECKOUT, private=True, + names_command=True, json_type="string"), + Field("label", "string", "Plain checkout value.", tier=CHECKOUT, json_type="string"), + ), + examples=({},), + unknown_keys=config_shapes.REJECT, +) + + +@pytest.fixture +def root(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + tree = tmp_path / "instance" + tree.mkdir() + monkeypatch.setattr(config, "ROOT", tree) + monkeypatch.setattr(config_shapes, "SETTINGS_SECTIONS", (TRACKER,)) + return tree + + +def _write(root: Path, instance: dict | None = None, local: dict | None = None) -> None: + if instance is not None: + (root / config.INSTANCE_SETTINGS_FILENAME).write_text(json.dumps(instance), encoding="utf-8") + if local is not None: + (root / config.LOCAL_SETTINGS_FILENAME).write_text(json.dumps(local), encoding="utf-8") + + +def _out(result) -> str: + """The command's output with Rich's line wrapping undone.""" + return " ".join(result.output.split()) + + +def _squashed(result) -> str: + """The output with every whitespace removed - for asserting a value is + absent, where a value Rich wrapped mid-word must still be found.""" + return "".join(result.output.split()) + + +def _read(path: Path) -> dict: + return json.loads(path.read_text(encoding="utf-8")) + + +def _fake_op(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, *, values: dict | None = None, + locked: bool = False, hang: bool = False) -> Path: + """An `op` that answers `op read --no-newline ` from `values`, says + "not signed in" when `locked`, or never answers when `hang` - recorded in a + `.wikitool-tools.json` of the test's own, the way the preflight records it.""" + lines = ["#!/bin/sh", + '[ "$1" = read ] && [ "$2" = --no-newline ] || { echo "[ERROR] bad call: $*" >&2; exit 2; }'] + if hang: + lines.append("sleep 10") + if locked: + lines += ['echo "[ERROR] 2026/10/10 12:00:00 You are not currently signed in." >&2', "exit 1"] + lines.append('case "$3" in') + for ref, value in (values or {}).items(): + lines.append(f" {shlex.quote(ref)}) printf '%s' {shlex.quote(value)}; exit 0 ;;") + lines += ["esac", + """echo "[ERROR] 2026/10/10 12:00:00 could not read secret '$3': \\"item\\" isn't an item in the \\"Vault\\" vault" >&2""", + "exit 1"] + script = tmp_path / "bin" / "op" + script.parent.mkdir(exist_ok=True) + script.write_text("\n".join(lines) + "\n", encoding="utf-8") + script.chmod(0o755) + tools = tmp_path / toolpaths.FILE_NAME + tools.write_text(json.dumps({"schema": 1, "complete": True, "tools": {"op": str(script)}}), encoding="utf-8") + monkeypatch.setattr(toolpaths, "tools_file", lambda: tools) + return script + + +def _command_app(body) -> typer.Typer: + """A one-command app standing in for a future consumer of the settings, + which reads them through the same two helpers.""" + test_app = typer.Typer() + + @test_app.command() + def run(): + body() + + @test_app.callback() + def _callback(): + pass + + return test_app + + +def _read_token(): + typer.echo(f"length {len(_util.setting_value(_util.load_settings().section('tracker'), 'token'))}") + + +# --- declaring a field ---------------------------------------------------------- + + +@pytest.mark.parametrize("flag", ["names_command", "gate"]) +def test_a_field_that_names_a_program_or_a_gate_cannot_be_declared_instance(flag): + with pytest.raises(ValueError, match="never tier 'instance'"): + Field("x", "string", "x", tier=INSTANCE, **{flag: True}) + assert Field("x", "string", "x", tier=CHECKOUT, **{flag: True}).user_only + + +def test_a_private_field_is_tier_checkout(): + with pytest.raises(ValueError, match="private field is tier 'checkout'"): + Field("x", "string", "x", tier=INSTANCE, private=True) + + +def test_the_settings_flags_need_a_tier(): + with pytest.raises(ValueError, match="need a tier"): + Field("x", "string", "x", private=True) + + +def test_an_unknown_tier_is_refused(): + with pytest.raises(ValueError, match="tier must be one of"): + Field("x", "string", "x", tier="global") + + +def test_every_shipped_settings_section_is_one(): + for shape in config_shapes.SETTINGS_SECTIONS: + assert settings.section_problems(shape) == [] + # The four per-feature files predate the tiers and declare none. + for shape in config_shapes.SHAPES: + assert all(not spec.tier for spec in shape.fields) + + +# --- reading: the right file ---------------------------------------------------- + + +def test_no_files_read_as_nothing_configured(root): + found = settings.load() + assert not found.instance_present and not found.local_present + assert not found.section("tracker").configured + assert found.section("tracker")["stale_days"] == 14 + + +def test_values_in_their_own_files_are_read(root): + _write(root, {"schema": 1, "instance_id": "abc", "tracker": {"stale_days": 21}}, + {"tracker": {"url": PRIVATE_VALUE, "label": "mine"}}) + section = settings.load().section("tracker") + assert section["stale_days"] == 21 + assert section["url"] == PRIVATE_VALUE + assert section["label"] == "mine" + + +@pytest.mark.parametrize("instance,local,file_name,key,tier", [ + (None, {"tracker": {"stale_days": 7, "url": "x"}}, ".wikitool-local.json", "tracker.stale_days", "instance"), + ({"tracker": {"label": "mine"}}, {"tracker": {"url": "x"}}, "wikitool.json", "tracker.label", "checkout"), +]) +def test_a_value_in_the_wrong_tier_fails_naming_file_key_and_tier(root, instance, local, file_name, key, tier): + _write(root, instance, local) + with pytest.raises(ValidationError) as caught: + settings.load() + assert f"{file_name}: {key} belongs to tier '{tier}'" in str(caught.value) + + result = runner.invoke(_command_app(lambda: _util.load_settings()), ["run"]) + assert result.exit_code == 1 + assert f"{file_name}: {key} belongs to tier '{tier}'" in _out(result) + + +def test_a_value_set_in_both_files_is_an_error_not_an_override(root): + _write(root, {"tracker": {"url": {"ref": "tracker_url"}}}, {"tracker": {"url": "x"}}) + with pytest.raises(ValidationError, match="set in both"): + settings.load() + + +def test_a_program_field_is_never_in_the_committed_file_even_as_a_reference(root): + _write(root, {"tracker": {"program": {"ref": "op://V/i/f"}}}, {"tracker": {"url": "x"}}) + with pytest.raises(ValidationError, match="not even as a reference"): + settings.load() + + +def test_a_reference_where_a_plain_value_belongs_is_refused(root): + _write(root, {"tracker": {"stale_days": {"ref": "days"}}}, {"tracker": {"url": "x"}}) + with pytest.raises(ValidationError, match="takes a plain value, not a reference"): + settings.load() + + +def test_a_missing_required_field_is_reported_unless_a_reference_stands_in(root): + _write(root, {"tracker": {"stale_days": 3}}) + with pytest.raises(ValidationError, match="tracker.url is missing"): + settings.load() + _write(root, {"tracker": {"stale_days": 3, "url": {"ref": "tracker_url"}}}) + assert settings.load().section("tracker").refs["url"] == settings.Ref("tracker_url") + + +def test_an_unknown_section_is_reported_not_refused(root): + _write(root, {"newer": {"x": 1}}) + assert settings.load().unknown == {"wikitool.json": ["newer"]} + + +@pytest.mark.parametrize("ref", [{"ref": "Upper"}, {"ref": "op://only/two"}, {"ref": 3}, + {"ref": "x", "extra": 1}]) +def test_a_malformed_reference_is_refused(root, ref): + _write(root, {"tracker": {"token": ref}}, {"tracker": {"url": "x"}}) + with pytest.raises(ValidationError): + settings.load() + + +# --- reading: no plain secret, no plain private value in the committed file ----- + + +@pytest.mark.parametrize("instance,local", [ + ({"tracker": {"token": SECRET_VALUE}}, {"tracker": {"url": "x"}}), + (None, {"tracker": {"url": "x", "token": SECRET_VALUE}}), + ({"tracker": {"url": SECRET_VALUE}}, None), +]) +def test_a_plain_secret_or_committed_private_value_fails_without_showing_it(root, instance, local): + _write(root, instance, local) + with pytest.raises(ValidationError) as caught: + settings.load() + assert SECRET_VALUE not in str(caught.value) + + result = runner.invoke(_command_app(lambda: _util.load_settings()), ["run"]) + assert result.exit_code == 1 + assert SECRET_VALUE not in _squashed(result) + + checks = doctor.check_settings() + assert any(c.status == "FAIL" for c in checks) + assert SECRET_VALUE not in json.dumps([c.__dict__ for c in checks]) + + +def test_docs_verify_flags_a_plain_secret_in_the_committed_file_only(root): + _write(root, {"tracker": {"token": SECRET_VALUE, "url": SECRET_VALUE}}) + problems = docs_verify.check_instance_settings() + assert len(problems) == 2 + assert SECRET_VALUE not in "\n".join(problems) + # The local file is never committed, and a required field may live there. + _write(root, {"tracker": {"token": {"ref": "tracker_token"}}}, {"tracker": {"token": SECRET_VALUE}}) + assert docs_verify.check_instance_settings() == [] + + +def test_docs_verify_accepts_an_absent_committed_file(root): + assert docs_verify.check_instance_settings() == [] + + +# --- resolving a named reference ------------------------------------------------ + + +def test_a_named_reference_resolves_from_its_variable(root, monkeypatch): + _write(root, {"instance_id": "abc", "tracker": {"token": {"ref": "tracker_token"}}}, + {"tracker": {"url": "x"}}) + monkeypatch.setenv("WIKITOOL_SECRET_TRACKER_TOKEN", SECRET_VALUE) + assert settings.load().section("tracker")["token"] == SECRET_VALUE + + +def test_without_the_variable_it_resolves_from_the_secrets_file_in_the_user_s_home(root, hermetic_environment): + _write(root, {"instance_id": "abc", "tracker": {"token": {"ref": "tracker_token"}}}, + {"tracker": {"url": "x"}}) + secrets_file = config.secrets_file("abc") + assert secrets_file == hermetic_environment / ".config" / "chemenu" / "abc" / "secrets.json" + secrets_file.parent.mkdir(parents=True) + secrets_file.write_text(json.dumps({"tracker_token": SECRET_VALUE}), encoding="utf-8") + secrets_file.chmod(0o600) + assert settings.load().section("tracker")["token"] == SECRET_VALUE + + +@needs_posix +def test_a_secrets_file_others_can_read_is_refused(root, hermetic_environment): + _write(root, {"instance_id": "abc", "tracker": {"token": {"ref": "tracker_token"}}}, + {"tracker": {"url": "x"}}) + secrets_file = config.secrets_file("abc") + secrets_file.parent.mkdir(parents=True) + secrets_file.write_text(json.dumps({"tracker_token": SECRET_VALUE}), encoding="utf-8") + secrets_file.chmod(0o644) + with pytest.raises(ValidationError, match="chmod 600"): + settings.load().section("tracker")["token"] + + +def test_an_unresolvable_name_says_where_to_put_the_value(root): + _write(root, {"instance_id": "abc", "tracker": {"token": {"ref": "tracker_token"}}}, + {"tracker": {"url": "x"}}) + with pytest.raises(ValidationError, match="WIKITOOL_SECRET_TRACKER_TOKEN"): + settings.load().section("tracker")["token"] + result = runner.invoke(_command_app(_read_token), ["run"]) + assert result.exit_code == 1 + + +def test_a_reference_is_resolved_on_access_not_on_load(root): + _write(root, {"instance_id": "abc", "tracker": {"token": {"ref": "tracker_token"}}}, + {"tracker": {"url": "x"}}) + section = settings.load().section("tracker") # no error: nothing resolved yet + assert section["url"] == "x" + + +# --- resolving an op:// reference ----------------------------------------------- + + +@needs_posix +def test_an_op_reference_resolves_through_op(root, tmp_path, monkeypatch): + _fake_op(tmp_path, monkeypatch, values={"op://Vault/tracker/url": PRIVATE_VALUE, + "op://Vault/tracker/token": SECRET_VALUE}) + _write(root, {"tracker": {"url": {"ref": "op://Vault/tracker/url"}, + "token": {"ref": "op://Vault/tracker/token"}}}) + section = settings.load().section("tracker") + assert section["url"] == PRIVATE_VALUE + assert section["token"] == SECRET_VALUE + + +@needs_posix +def test_a_locked_op_is_exit_42(root, tmp_path, monkeypatch): + _fake_op(tmp_path, monkeypatch, locked=True) + _write(root, {"tracker": {"token": {"ref": "op://Vault/tracker/token"}}}, {"tracker": {"url": "x"}}) + with pytest.raises(settings.SecretSourceLocked): + settings.load().section("tracker")["token"] + result = runner.invoke(_command_app(_read_token), ["run"]) + assert result.exit_code == 42 + assert "NEEDS USER CLEARANCE" in _out(result) + assert "op://Vault" not in _squashed(result) + + +@needs_posix +def test_an_op_that_does_not_answer_in_time_is_exit_42(root, tmp_path, monkeypatch): + _fake_op(tmp_path, monkeypatch, hang=True) + monkeypatch.setattr(settings, "OP_TIMEOUT_SECONDS", 1) + _write(root, {"tracker": {"token": {"ref": "op://Vault/tracker/token"}}}, {"tracker": {"url": "x"}}) + result = runner.invoke(_command_app(_read_token), ["run"]) + assert result.exit_code == 42 + + +@needs_posix +def test_a_missing_item_is_exit_1_naming_the_reference(root, tmp_path, monkeypatch): + _fake_op(tmp_path, monkeypatch, values={}) + _write(root, {"tracker": {"token": {"ref": "op://Vault/tracker/token"}}}, {"tracker": {"url": "x"}}) + result = runner.invoke(_command_app(_read_token), ["run"]) + assert result.exit_code == 1 + assert "op://Vault/tracker/token" in _out(result) + + +def test_a_missing_op_is_exit_1_pointing_at_the_preflight(root, tmp_path, monkeypatch): + tools = tmp_path / toolpaths.FILE_NAME + tools.write_text(json.dumps({"schema": 1, "complete": True, "tools": {}}), encoding="utf-8") + monkeypatch.setattr(toolpaths, "tools_file", lambda: tools) + _write(root, {"tracker": {"token": {"ref": "op://Vault/tracker/token"}}}, {"tracker": {"url": "x"}}) + result = runner.invoke(_command_app(_read_token), ["run"]) + assert result.exit_code == 1 + assert "preflight" in _out(result) + + +def test_without_a_tools_file_an_absent_op_is_exit_1_too(root, monkeypatch): + monkeypatch.setenv("PATH", "") + _write(root, {"tracker": {"token": {"ref": "op://Vault/tracker/token"}}}, {"tracker": {"url": "x"}}) + result = runner.invoke(_command_app(_read_token), ["run"]) + assert result.exit_code == 1 + assert "1Password CLI" in _out(result) + + +# --- doctor --------------------------------------------------------------------- + + +def _by_name(checks) -> dict: + return {c.name: c for c in checks} + + +def test_doctor_reports_each_tier_absent_as_ok(root): + checks = _by_name(doctor.check_settings()) + assert [c.status for c in checks.values()] == ["OK", "OK", "OK"] + assert "absent" in checks["settings-instance"].detail + assert "absent" in checks["settings-checkout"].detail + assert checks["settings-secrets"].detail == "no references" + + +def test_doctor_reports_a_malformed_file_as_fail(root): + (root / config.INSTANCE_SETTINGS_FILENAME).write_text("{not json", encoding="utf-8") + checks = _by_name(doctor.check_settings()) + assert checks["settings-instance"].status == "FAIL" + assert checks["settings-checkout"].status == "OK" + + +@needs_posix +def test_doctor_reports_references_as_resolvable_or_not_never_their_value(root, tmp_path, monkeypatch): + _fake_op(tmp_path, monkeypatch, values={"op://Vault/tracker/url": PRIVATE_VALUE}) + _write(root, {"instance_id": "abc", "tracker": {"url": {"ref": "op://Vault/tracker/url"}, + "token": {"ref": "tracker_token"}}}) + monkeypatch.setenv("WIKITOOL_SECRET_TRACKER_TOKEN", SECRET_VALUE) + checks = _by_name(doctor.check_settings()) + assert checks["settings-secrets"].status == "OK" + assert checks["settings-secrets"].detail == "2 of 2 reference(s) resolvable" + assert "instance id abc" in checks["settings-instance"].detail + + monkeypatch.delenv("WIKITOOL_SECRET_TRACKER_TOKEN") + checks = _by_name(doctor.check_settings()) + assert checks["settings-secrets"].status == "FAIL" + assert "1 of 2" in checks["settings-secrets"].detail + dumped = json.dumps([c.__dict__ for c in checks.values()]) + assert SECRET_VALUE not in dumped and PRIVATE_VALUE not in dumped + + +@needs_posix +def test_doctor_warns_rather_than_fails_on_a_locked_op(root, tmp_path, monkeypatch): + _fake_op(tmp_path, monkeypatch, locked=True) + _write(root, {"tracker": {"url": {"ref": "op://Vault/tracker/url"}}}) + checks = _by_name(doctor.check_settings()) + assert checks["settings-secrets"].status == "WARN" + assert "tracker.url" in checks["settings-secrets"].detail + + +def test_doctor_warns_on_a_section_this_version_does_not_read(root): + _write(root, {"newer": {}}) + checks = _by_name(doctor.check_settings()) + assert checks["settings-instance"].status == "WARN" + + +# --- config set ----------------------------------------------------------------- + + +def test_config_set_writes_an_instance_value_and_creates_the_file(root): + result = runner.invoke(app, ["config", "set", "tracker.stale_days", "21"]) + assert result.exit_code == 0, result.output + data = _read(root / config.INSTANCE_SETTINGS_FILENAME) + assert data["tracker"] == {"stale_days": 21} + assert data["schema"] == 1 + assert data["instance_id"] + assert "created" in _out(result) + assert not (root / config.LOCAL_SETTINGS_FILENAME).exists() + + instance_id = data["instance_id"] + result = runner.invoke(app, ["config", "set", "tracker.stale_days", "30"]) + assert result.exit_code == 0, result.output + assert _read(root / config.INSTANCE_SETTINGS_FILENAME)["instance_id"] == instance_id + + +def test_config_set_refuses_a_plain_secret_without_echoing_it(root): + result = runner.invoke(app, ["config", "set", "tracker.token", SECRET_VALUE]) + assert result.exit_code == 1 + assert SECRET_VALUE not in _squashed(result) + assert not (root / config.INSTANCE_SETTINGS_FILENAME).exists() + + +def test_config_set_writes_a_secret_reference_into_the_committed_file(root): + result = runner.invoke(app, ["config", "set", "tracker.token", "--ref", "tracker_token"]) + assert result.exit_code == 0, result.output + assert _read(root / config.INSTANCE_SETTINGS_FILENAME)["tracker"]["token"] == {"ref": "tracker_token"} + + +def test_config_set_puts_a_private_value_locally_and_a_reference_in_its_place(root): + result = runner.invoke(app, ["config", "set", "tracker.url", PRIVATE_VALUE]) + assert result.exit_code == 0, result.output + assert _read(root / config.LOCAL_SETTINGS_FILENAME)["tracker"]["url"] == PRIVATE_VALUE + assert PRIVATE_VALUE not in _squashed(result) + assert "tracker" not in _read(root / config.INSTANCE_SETTINGS_FILENAME) + + result = runner.invoke(app, ["config", "set", "tracker.url", "--ref", "op://Vault/tracker/url"]) + assert result.exit_code == 0, result.output + assert "removed from .wikitool-local.json" in _out(result) + assert _read(root / config.INSTANCE_SETTINGS_FILENAME)["tracker"]["url"] == {"ref": "op://Vault/tracker/url"} + assert "tracker" not in _read(root / config.LOCAL_SETTINGS_FILENAME) + + +def test_config_set_leaves_a_program_field_to_the_user(root): + result = runner.invoke(app, ["config", "set", "tracker.program", "/usr/bin/true"]) + assert result.exit_code == 42 + assert ".wikitool-local.json" in _out(result) + assert not (root / config.LOCAL_SETTINGS_FILENAME).exists() + + +@pytest.mark.parametrize("args", [ + ["tracker.nope", "1"], + ["nope.field", "1"], + ["nodot", "1"], + ["tracker.stale_days", "--ref", "days"], + ["tracker.token", "--ref", "Not-A-Name"], + ["tracker.stale_days", "many"], + ["tracker.stale_days", "0"], + ["tracker.stale_days"], +]) +def test_config_set_refuses_and_writes_nothing(root, args): + result = runner.invoke(app, ["config", "set", *args]) + assert result.exit_code == 1, result.output + assert not (root / config.INSTANCE_SETTINGS_FILENAME).exists() + assert not (root / config.LOCAL_SETTINGS_FILENAME).exists() + + +def test_config_set_refuses_on_top_of_files_that_do_not_pass(root): + _write(root, {"tracker": {"token": SECRET_VALUE}}) + before = (root / config.INSTANCE_SETTINGS_FILENAME).read_text(encoding="utf-8") + result = runner.invoke(app, ["config", "set", "tracker.stale_days", "3"]) + assert result.exit_code == 1 + assert SECRET_VALUE not in _squashed(result) + assert (root / config.INSTANCE_SETTINGS_FILENAME).read_text(encoding="utf-8") == before + + +def test_config_set_with_no_section_registered_names_none(root, monkeypatch): + monkeypatch.setattr(config_shapes, "SETTINGS_SECTIONS", ()) + result = runner.invoke(app, ["config", "set", "tracker.stale_days", "3"]) + assert result.exit_code == 1 + assert "none is registered" in _out(result) + + +# --- the two files in git ------------------------------------------------------- + + +def test_the_repository_ignores_the_local_file_and_tracks_the_committed_one(): + root = config._PACKAGE_ROOT + probe = subprocess.run( + ["git", "-C", str(root), "check-ignore", "--no-index", "-q", config.LOCAL_SETTINGS_FILENAME], + capture_output=True, check=False, + ) + if probe.returncode not in (0, 1): + pytest.skip("no git checkout to ask") + assert probe.returncode == 0 + tracked = subprocess.run( + ["git", "-C", str(root), "check-ignore", "--no-index", "-q", config.INSTANCE_SETTINGS_FILENAME], + capture_output=True, check=False, + ) + assert tracked.returncode == 1 + + +# --- CI: a real op:// value ------------------------------------------------------- + + +def test_live_secret_reference_resolves_from_ci(root, monkeypatch): + """CI loads `op://CI-CD/...` through 1Password's own action into + `WIKITOOL_SECRET_CI_SMOKE` (.gitea/workflows/ci.yml) and runs this test with + `CHEMENU_LIVE_SECRET_REQUIRE=1`, which turns "no value" into a failure. The + value is never printed - only that a reference resolved to something.""" + if not _LIVE_VALUE: + if _LIVE_REQUIRED: + pytest.fail("CHEMENU_LIVE_SECRET_REQUIRE=1 but WIKITOOL_SECRET_CI_SMOKE is empty - " + "the 1Password step did not load it") + pytest.skip("no WIKITOOL_SECRET_CI_SMOKE - set only in CI, by the 1Password step") + monkeypatch.setenv("WIKITOOL_SECRET_CI_SMOKE", _LIVE_VALUE) + _write(root, {"instance_id": "ci", "tracker": {"token": {"ref": "ci_smoke"}}}, {"tracker": {"url": "x"}}) + assert settings.load().section("tracker")["token"] == _LIVE_VALUE + checks = _by_name(doctor.check_settings()) + assert checks["settings-secrets"].detail == "1 of 1 reference(s) resolvable" diff --git a/tools/chemenu/toolpaths.py b/tools/chemenu/toolpaths.py index 651e504..87dfd2b 100644 --- a/tools/chemenu/toolpaths.py +++ b/tools/chemenu/toolpaths.py @@ -81,6 +81,22 @@ def resolve(name: str) -> str: return path +def optional(name: str) -> Optional[str]: + """The program to start for an optional tool - one the preflight records + when it finds it and never asks for (`optional|` in + `tools/prerequisites.txt`). Its recorded path, `name` itself when no file + has been written, and None when the file does not name it or names a path + that has gone: for an optional tool both mean "not installed here", and the + caller says what that costs.""" + data = load() + if data is None: + return name + path = data["tools"].get(name) + if not isinstance(path, str) or not path or not os.path.isfile(path): + return None + return path + + def git() -> str: return resolve("git") diff --git a/tools/preflight.ps1 b/tools/preflight.ps1 index 934a404..c98bd1d 100644 --- a/tools/preflight.ps1 +++ b/tools/preflight.ps1 @@ -537,6 +537,20 @@ foreach ($line in $ManifestLines) { } } +# Recorded when found, never asked for - see the manifest's own header. +$Optional = @() +foreach ($line in $ManifestLines) { + $parts = $line.Split('|') + if ($parts[0] -ne 'optional') { + continue + } + if ($parts[1] -notmatch '^[a-z0-9]+$') { + Write-ErrorLine "preflight: bad tool name '$($parts[1])' in $Manifest" + exit 1 + } + $Optional += $parts[1] +} + function Get-InstallHint { param([string]$Tool) $choco = Get-ManifestField 'tool' $Tool 7 @@ -696,8 +710,8 @@ function Get-SetValue { # --- --set: every named path has to work, or nothing is written ------------------- foreach ($name in $Sets.Keys) { - if ($Tools -notcontains $name) { - Write-ErrorLine "preflight: --set names '$name', which is not a tool this platform needs: $($Tools -join ' ')" + if ($Tools -notcontains $name -and $Optional -notcontains $name) { + Write-ErrorLine "preflight: --set names '$name', which is not a tool this platform needs: $($Tools -join ' ') (optional: $($Optional -join ' '))" exit 1 } } @@ -848,6 +862,38 @@ foreach ($tool in $Tools) { $Found[$tool] = $path } +# --- optional tools: recorded when found, never a problem when not --------------- + +foreach ($tool in $Optional) { + $label = Get-ManifestField 'optional' $tool 3 + $given = Get-SetValue $tool + $path = '' + if ($given) { + if (Test-Usable $given) { + $path = $given + } else { + Add-Problem "The path given for $label does not work: $given" (Get-ManifestField 'optional' $tool 4) ` + "Check the path - it has to be the $tool executable itself.`nThen run: $Self --set $tool=" + $script:BadSet = $true + continue + } + } else { + $previous = Get-RecordedPath $tool + if ($previous -and (Test-Usable $previous)) { + $path = $previous + } else { + $path = [string](Find-OnPath $tool | Select-Object -First 1) + } + } + if (-not $path) { + Add-Report 'ABSENT' $tool '-' 'optional, not installed' + continue + } + $version = Get-VersionOf ([string](Invoke-Native -Path $path -Arguments @('--version'))) + Add-Report 'OK' $tool $(if ($version) { $version } else { '-' }) $path + $Found[$tool] = $path +} + # --- install folder length (Windows, long paths off) ------------------------------ if ($Platform -eq 'windows' -and -not (Test-LongPathsEnabled)) { @@ -945,7 +991,7 @@ function Write-ToolsFile { $flag = if ($Complete) { 'true' } else { 'false' } $text = "{`n `"schema`": 1,`n `"complete`": $flag,`n `"tools`": {" $separator = '' - foreach ($tool in $Tools) { + foreach ($tool in @($Tools) + @($Optional)) { if ($Found.ContainsKey($tool)) { $text += "$separator`n `"$tool`": `"$(ConvertTo-JsonString $Found[$tool])`"" $separator = ',' diff --git a/tools/preflight.sh b/tools/preflight.sh index 991b758..88ec9c2 100755 --- a/tools/preflight.sh +++ b/tools/preflight.sh @@ -456,6 +456,14 @@ while IFS='|' read -r kind name minimum platforms rest; do fi done < "$MANIFEST" +# Recorded when found, never asked for - see the manifest's own header. +OPTIONAL="" +while IFS='|' read -r kind name rest; do + [ "$kind" = optional ] || continue + case "$name" in *[!a-z0-9]*|'') echo "preflight: bad tool name '$name' in $MANIFEST" >&2; exit 1 ;; esac + OPTIONAL="$OPTIONAL $name" +done < "$MANIFEST" + # The install command for this machine's package manager, or every known one. install_hint() { # choco=$(manifest_field tool "$1" 7) @@ -561,9 +569,9 @@ set_value() { # -> the path given with --set, if any # --- --set: every named path has to work, or nothing is written ------------------- for entry in $(printf '%s\n' "$SETS" | sed -n 's/^\([^=]*\)=.*/\1/p'); do - case " $TOOLS " in + case " $TOOLS $OPTIONAL " in *" $entry "*) ;; - *) echo "preflight: --set names '$entry', which is not a tool this platform needs:$TOOLS" >&2 + *) echo "preflight: --set names '$entry', which is not a tool this platform needs:$TOOLS (optional:$OPTIONAL)" >&2 exit 1 ;; esac done @@ -705,6 +713,41 @@ $SELF --set $tool=" eval "FOUND_$tool=\$native" done +# --- optional tools: recorded when found, never a problem when not --------------- + +for tool in $OPTIONAL; do + label=$(manifest_field optional "$tool" 3) + given=$(set_value "$tool") + path="" + if [ -n "$given" ]; then + if usable "$given"; then + path=$given + else + problem "The path given for $label does not work: $given" \ + "$(manifest_field optional "$tool" 4)" \ + "Check the path - it has to be the $tool executable itself. +Then run: $SELF --set $tool=" + BAD_SET=1 + continue + fi + else + previous=$(recorded "$tool") || previous="" + if [ -n "$previous" ] && usable "$previous"; then + path=$previous + else + path=$(on_path "$tool" | head -n 1) + fi + fi + if [ -z "$path" ]; then + report ABSENT "$tool" - "optional, not installed" + continue + fi + version=$(version_of "$("$path" --version 2>/dev/null | tr -d '\r')") + native=$(native_path "$path") + report OK "$tool" "${version:--}" "$native" + eval "FOUND_$tool=\$native" +done + # --- install folder length (Windows, long paths off) ------------------------------ if [ "$PLATFORM" = windows ] && ! longpaths_enabled; then @@ -730,7 +773,7 @@ write_tools_file() { # { printf '{\n "schema": 1,\n "complete": %s,\n "tools": {' "$1" separator="" - for tool in $TOOLS; do + for tool in $TOOLS $OPTIONAL; do eval "value=\${FOUND_$tool:-}" [ -n "$value" ] || continue printf '%s\n "%s": "%s"' "$separator" "$tool" "$(json_string "$value")" diff --git a/tools/prerequisites.txt b/tools/prerequisites.txt index 454a2c6..52224ea 100644 --- a/tools/prerequisites.txt +++ b/tools/prerequisites.txt @@ -7,8 +7,14 @@ # limit|| # tool||||