README missing four required sections; TODO.md is stale and describes completed work as pending #46

New Issue

clawbot · 2026-08-09T03:43:16+02:00

clawbot commented

2026-08-09 03:43:16 +02:00

From the repo-standards audit. Docs-only, so per repo workflow this one skips adversarial review and goes straight to merge-ready once make check is green.

Divergences

README structure. Policy requires Description, Getting Started, Entrypoints, Rationale, Design, TODO, License, Author. Present today: Description, Entrypoints (this one is genuinely good — it opens by naming the Scripts to Rule Them All standard with the link and documents all twelve scripts), and Author. Missing as named sections: Getting Started, Rationale, Design, TODO, License.

The content mostly exists under different headings — "Installation" and "Quick Start" cover Getting Started, "Core Architecture" covers Design — so this is largely renaming plus two genuinely new sections.

Description line is non-compliant. Policy requires the first line to include name, purpose, category, license, and author, with this model: "µPaaS is an MIT-licensed Go web application by @sneak that receives git-frontend webhooks and deploys applications via Docker in realtime." The current opening states name, purpose and category but omits license and author. License appears only as prose at line 533 under # Author, never as its own section.

TODO.md is actively misleading. Its # Next Step section reads:

Bring the repo into policy compliance in one commit:
- Add fmt-check and hooks targets to the Makefile
- Add REPO_POLICIES.md and .editorconfig.
- Add .gitea/workflows/check.yml running make check.
- Verify Dockerfile base images are pinned by sha256.

All four are already done. fmt-check and hooks are at Makefile:57-61; REPO_POLICIES.md and .editorconfig are both present and byte-identical to canonical; .gitea/workflows/check.yml exists; all three Dockerfile stages are digest-pinned. An agent or contributor reading this would redo completed work. The third bullet is also wrong on its own terms — the workflow correctly runs script/cibuild, not make check directly.

Policy also wants the TODO in the README: "When planning, put the todo list in the README so a new agent can pick up where the last one left off."

Definition of done

README has all eight required sections by their policy names.
The first line names the project, purpose, category, license (WTFPL), and author (@sneak), following the model above.
License is its own section, separate from Author.
"Installation"/"Quick Start" become Getting Started; "Core Architecture" becomes Design; Rationale is written fresh — the existing second paragraph about replacing a directory of PGP-encrypted files is the substance of it.
The stale # Next Step block is cleared and its four completed items moved into # Completed Steps with the date they actually landed.
The TODO now points at this tracker as authoritative. Do not copy the issue list into the README — it will go stale exactly the way the current block did. A short TODO section naming the 1.0.0 milestone with a link is the right shape.
Markdown is formatted with the repo's own settings before committing: prettier, 4-space tabs, proseWrap: always (hard wrap at 80). The audit measured 19 lines over 80 columns in README.md and 5 in AGENTS.md.
make check green.

Implementation requirements

Docs-only. Zero changes to code, config, scripts, CI, or build files — that is what makes this eligible to skip review. If a code change turns out to be needed, stop and split it out.
Do not delete the DANGER warnings on the destructive commands. #39 will rewrite them once confirmation prompts exist; until then they are accurate and load-bearing.
The Entrypoints section is already correct and complete. Leave it alone apart from prose rewrapping.
Keep TODO.md as a file if you prefer it there, but its Next Step must reflect reality. The authoritative queue is the Gitea 1.0.0 milestone, and the README should say so.

From the repo-standards audit. Docs-only, so per repo workflow this one skips adversarial review and goes straight to `merge-ready` once `make check` is green. ## Divergences **README structure.** Policy requires Description, Getting Started, Entrypoints, Rationale, Design, TODO, License, Author. Present today: Description, Entrypoints (this one is genuinely good — it opens by naming the Scripts to Rule Them All standard with the link and documents all twelve scripts), and Author. Missing as named sections: **Getting Started**, **Rationale**, **Design**, **TODO**, **License**. The content mostly exists under different headings — "Installation" and "Quick Start" cover Getting Started, "Core Architecture" covers Design — so this is largely renaming plus two genuinely new sections. **Description line is non-compliant.** Policy requires the first line to include name, purpose, category, **license, and author**, with this model: *"µPaaS is an MIT-licensed Go web application by @sneak that receives git-frontend webhooks and deploys applications via Docker in realtime."* The current opening states name, purpose and category but omits license and author. License appears only as prose at line 533 under `# Author`, never as its own section. **TODO.md is actively misleading.** Its `# Next Step` section reads: ``` Bring the repo into policy compliance in one commit: - Add fmt-check and hooks targets to the Makefile - Add REPO_POLICIES.md and .editorconfig. - Add .gitea/workflows/check.yml running make check. - Verify Dockerfile base images are pinned by sha256. ``` **All four are already done.** `fmt-check` and `hooks` are at `Makefile:57-61`; `REPO_POLICIES.md` and `.editorconfig` are both present and byte-identical to canonical; `.gitea/workflows/check.yml` exists; all three Dockerfile stages are digest-pinned. An agent or contributor reading this would redo completed work. The third bullet is also wrong on its own terms — the workflow correctly runs `script/cibuild`, not `make check` directly. Policy also wants the TODO in the README: *"When planning, put the todo list in the README so a new agent can pick up where the last one left off."* ## Definition of done - README has all eight required sections by their policy names. - The first line names the project, purpose, category, license (WTFPL), and author (@sneak), following the model above. - License is its own section, separate from Author. - "Installation"/"Quick Start" become **Getting Started**; "Core Architecture" becomes **Design**; **Rationale** is written fresh — the existing second paragraph about replacing a directory of PGP-encrypted files is the substance of it. - The stale `# Next Step` block is cleared and its four completed items moved into `# Completed Steps` with the date they actually landed. - The TODO now points at this tracker as authoritative. **Do not copy the issue list into the README** — it will go stale exactly the way the current block did. A short TODO section naming the `1.0.0` milestone with a link is the right shape. - Markdown is formatted with the repo's own settings before committing: prettier, 4-space tabs, `proseWrap: always` (hard wrap at 80). The audit measured 19 lines over 80 columns in README.md and 5 in AGENTS.md. - `make check` green. ## Implementation requirements - Docs-only. **Zero changes to code, config, scripts, CI, or build files** — that is what makes this eligible to skip review. If a code change turns out to be needed, stop and split it out. - Do not delete the DANGER warnings on the destructive commands. #39 will rewrite them once confirmation prompts exist; until then they are accurate and load-bearing. - The Entrypoints section is already correct and complete. Leave it alone apart from prose rewrapping. - Keep TODO.md as a file if you prefer it there, but its `Next Step` must reflect reality. The authoritative queue is the Gitea `1.0.0` milestone, and the README should say so.

clawbot added this to the 1.0.0 milestone 2026-08-09 03:43:16 +02:00

clawbot commented

2026-08-09 03:44:21 +02:00

Manager note, for whoever picks this up: three entries in the old TODO.md 1.0 blocker list were verified against the current code and are already fixed. They should be deleted from TODO.md as part of this issue rather than carried forward or filed as work. Evidence below so the deletion is not taken on faith.

Command injection — fixed, both halves.
No shell is spawned anywhere in the repo; there is no sh -c. Every GPG invocation uses exec.CommandContext with an argv slice, and key IDs are validated first by an anchored allowlist (pgpunlocker.go:50-55, validateGPGKeyID at :371-381, applied at :384-394 and :434-442). Option injection via a leading - does not work: the value is passed as the operand after -r, which gpg consumes as a recipient, not as a new flag. The on-disk metadata key ID is never passed to an exec — GetGPGKeyID() feeds display in GetID() only.
The macOS half is fixed because the code no longer exists: keychainunlocker.go uses the go-keychain API directly (:490-578) and has no exec import at all. Item names are separately validated at :477-487.

Timing attacks — fixed, and the remaining comparisons are not vulnerabilities.
bytes.Equal is gone from non-test code; it survives only in test files. The two comparisons that remain are both non-constant-time and both fine. internal/cli/init.go:216 compares a passphrase the user typed twice at the same terminal — there is no oracle, and an attacker positioned to measure it can already read the buffer. internal/vault/vault.go:206-215 compares a double-SHA256 of a public key against a stored hash of that same public key; both operands are public values. crypto/subtle is not imported anywhere, and does not need to be. If you want constant-time compares at those two sites as defense in depth it is a four-line change, but it closes nothing.

Secure temporary file handling — moot as written.
There are no temp files. Zero hits for os.CreateTemp, MkdirTemp, ioutil.TempFile, or hand-built .tmp paths in non-test code; every write goes directly to its final path. Permissions are correct throughout — DirPerms = 0o700, FilePerms = 0o600 in internal/secret/constants.go, used by every MkdirAll and WriteFile.
Worth stating plainly: the reason there are no temp files is that writes are not atomic, which is #34. Fixing #34 introduces temp files holding key material, at which point this item becomes live again as a requirement of that work — and #34's definition of done already carries it (0600 at creation, in the destination directory, cleaned up on every error path). So delete it here, but do not conclude the concern is permanently retired.

Two further corrections to fold into the cleanup: the old list named internal/cli/version.go among the unprotected identity.String() sites — it contains no such call, and the accurate site list is now in #38. And internal/cli/crypto.go is described as holding the age key in a plain string; it is held in a LockedBuffer throughout, with one genuine residual at crypto.go:95, also captured in #38.

Everything else from that list is now filed individually under the 1.0.0 milestone.

Manager note, for whoever picks this up: three entries in the old `TODO.md` 1.0 blocker list were verified against the current code and are **already fixed**. They should be deleted from `TODO.md` as part of this issue rather than carried forward or filed as work. Evidence below so the deletion is not taken on faith. **Command injection — fixed, both halves.** No shell is spawned anywhere in the repo; there is no `sh -c`. Every GPG invocation uses `exec.CommandContext` with an argv slice, and key IDs are validated first by an anchored allowlist (`pgpunlocker.go:50-55`, `validateGPGKeyID` at `:371-381`, applied at `:384-394` and `:434-442`). Option injection via a leading `-` does not work: the value is passed as the operand after `-r`, which gpg consumes as a recipient, not as a new flag. The on-disk metadata key ID is never passed to an exec — `GetGPGKeyID()` feeds display in `GetID()` only. The macOS half is fixed because the code no longer exists: `keychainunlocker.go` uses the `go-keychain` API directly (`:490-578`) and has no `exec` import at all. Item names are separately validated at `:477-487`. **Timing attacks — fixed, and the remaining comparisons are not vulnerabilities.** `bytes.Equal` is gone from non-test code; it survives only in test files. The two comparisons that remain are both non-constant-time and both fine. `internal/cli/init.go:216` compares a passphrase the user typed twice at the same terminal — there is no oracle, and an attacker positioned to measure it can already read the buffer. `internal/vault/vault.go:206-215` compares a double-SHA256 of a public key against a stored hash of that same public key; both operands are public values. `crypto/subtle` is not imported anywhere, and does not need to be. If you want constant-time compares at those two sites as defense in depth it is a four-line change, but it closes nothing. **Secure temporary file handling — moot as written.** There are no temp files. Zero hits for `os.CreateTemp`, `MkdirTemp`, `ioutil.TempFile`, or hand-built `.tmp` paths in non-test code; every write goes directly to its final path. Permissions are correct throughout — `DirPerms = 0o700`, `FilePerms = 0o600` in `internal/secret/constants.go`, used by every `MkdirAll` and `WriteFile`. Worth stating plainly: the reason there are no temp files is that **writes are not atomic**, which is #34. Fixing #34 introduces temp files holding key material, at which point this item becomes live again as a requirement *of that work* — and #34's definition of done already carries it (0600 at creation, in the destination directory, cleaned up on every error path). So delete it here, but do not conclude the concern is permanently retired. Two further corrections to fold into the cleanup: the old list named `internal/cli/version.go` among the unprotected `identity.String()` sites — it contains no such call, and the accurate site list is now in #38. And `internal/cli/crypto.go` is described as holding the age key in a plain string; it is held in a `LockedBuffer` throughout, with one genuine residual at `crypto.go:95`, also captured in #38. Everything else from that list is now filed individually under the `1.0.0` milestone.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/secret#46