From 30d0df2c691e075f564d4399249ed58fceec89c4 Mon Sep 17 00:00:00 2001 From: hakeemRash Date: Mon, 31 Aug 2026 20:27:56 +0300 Subject: [PATCH 01/14] chore(let-migrate): pin the merged main, not the feature branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No code change: `git diff 378cd6f 8496e49` is empty, so this is byte-identical to what v1.9.0 already ships. What changes is whether the pointer SURVIVES. v1.9.0 pinned `378cd6f`, a commit on `fix/portable-alter-table`. That branch has since merged (Let-Migrate PR #8), and a merged branch is a branch someone deletes — at which point `git submodule update` fails for anyone checking out the v1.9.0 tag, with nothing in the kernel to say why. `8496e49` is the tip of `main`, which is not going anywhere. This is the same failure `modules/http` was one branch-deletion away from earlier today, fixed the same way. No CHANGELOG entry and no version bump: a gitlink that resolves to identical content is not something a user of the kernel can observe, and auto-release reads the top heading, which stays `1.9.0`. --- modules/let-migrate | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/let-migrate b/modules/let-migrate index 378cd6f..8496e49 160000 --- a/modules/let-migrate +++ b/modules/let-migrate @@ -1 +1 @@ -Subproject commit 378cd6fb7eed020bf23ef192f5174f09e314bacd +Subproject commit 8496e493cb82b4f5f7c1abd8f4c88c0841267095 From ff159e06abed5549bf4ed2c6f53635873c27c3b4 Mon Sep 17 00:00:00 2001 From: hakeemRash Date: Mon, 31 Aug 2026 21:49:45 +0300 Subject: [PATCH 02/14] Merge branch 'main' of github.com:AlfaCode-Team/hkm-kernel into dev-mac From 9497cd99b1b32b43f41d5ec60eac4141c297590a Mon Sep 17 00:00:00 2001 From: hakeemRash Date: Tue, 1 Sep 2026 16:29:51 +0300 Subject: [PATCH 03/14] chore(let-migrate): update subproject commit reference --- modules/let-migrate | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/let-migrate b/modules/let-migrate index 88f255d..d4cb2ee 160000 --- a/modules/let-migrate +++ b/modules/let-migrate @@ -1 +1 @@ -Subproject commit 88f255ddb903bc9009cba50a87f211b9722abc72 +Subproject commit d4cb2ee319a35a424ee55084eaff4571b8f928e2 From c57bdd0f432d62d3d9b7ee0652e97177a23116fa Mon Sep 17 00:00:00 2001 From: hakeemRash Date: Tue, 1 Sep 2026 16:56:54 +0300 Subject: [PATCH 04/14] chore(let-migrate): update subproject commit reference to 735d95e --- modules/let-migrate | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/let-migrate b/modules/let-migrate index d4cb2ee..735d95e 160000 --- a/modules/let-migrate +++ b/modules/let-migrate @@ -1 +1 @@ -Subproject commit d4cb2ee319a35a424ee55084eaff4571b8f928e2 +Subproject commit 735d95ef2922940fc7767ab55d61a95f33befc3e From 3bf309426f5d0f4b13f6584c9e0e8875714d89da Mon Sep 17 00:00:00 2001 From: hakeemRash Date: Tue, 1 Sep 2026 17:35:45 +0300 Subject: [PATCH 05/14] chore(let-migrate): update subproject commit reference to 9ac3a8e --- modules/let-migrate | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/let-migrate b/modules/let-migrate index 735d95e..9ac3a8e 160000 --- a/modules/let-migrate +++ b/modules/let-migrate @@ -1 +1 @@ -Subproject commit 735d95ef2922940fc7767ab55d61a95f33befc3e +Subproject commit 9ac3a8ed16e03e95e88f9424602a6e0fa7866959 From 5980c133deecb53fbea4001f37d360ace6c6569b Mon Sep 17 00:00:00 2001 From: hakeemRash Date: Wed, 2 Sep 2026 12:23:33 +0300 Subject: [PATCH 06/14] chore(let-migrate): update subproject commit reference to f13c977 Restores the four driver fixes and the Laravel-parity alias that 9ac3a8e undid -- that commit put src/, tests/ and README back byte-for-byte to the state before a day of merged PR work, under a message describing a cleanup -- then carries them forward with what running them against real engines turned up. Verified by execution, not by compiling and reading: SQLite, MariaDB 12.3 and PostgreSQL 18 all run the full DDL lifecycle green. SQL Server is the one gap; no server was reachable and the sqlsrv extension is absent here, so its two fixes are argued from the T-SQL specification. tests/Live runs them the moment LETMIGRATE_DB_SQLSRV points at a server, and skips with the reason until then. docs/guides/18_MIGRATIONS.md: useCurrent() and useCurrentOnUpdate() now exist, so the anti-pattern entry saying they do not is corrected. The ->index() half of it still stands -- an index is declared on the Blueprint, not the column. --- docs/guides/18_MIGRATIONS.md | 11 +++++++---- modules/let-migrate | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/guides/18_MIGRATIONS.md b/docs/guides/18_MIGRATIONS.md index faa8f8a..c9e1ec9 100644 --- a/docs/guides/18_MIGRATIONS.md +++ b/docs/guides/18_MIGRATIONS.md @@ -240,9 +240,12 @@ $t->string('email') > primary key use the Blueprint method `$t->primary(['a', 'b'])`. Do not use > both on the same table. > -> **There is no fluent `->index()` or `->useCurrent()` column modifier.** -> Declare indexes with the Blueprint method `$t->index(['col'])` (see below), -> and a current-timestamp default with `->default('CURRENT_TIMESTAMP')`. +> **There is no fluent `->index()` column modifier.** Declare indexes with the +> Blueprint method `$t->index(['col'])` (see below). +> +> `->useCurrent()` and `->useCurrentOnUpdate()` DO exist — Laravel-parity +> aliases of `->default('CURRENT_TIMESTAMP')` and +> `->onUpdateCurrentTimestamp()`. Either spelling compiles identically. ### Timestamps Behavior @@ -841,7 +844,7 @@ echo $result->summary(); ✗ Write migrations without matching down() rollback ✗ Forget to run data migrations inside transactions (or explicit transaction handling) ✗ Use --seed in refresh without wiring SeederRunner to MigrateRefreshCommand -✗ Use a fluent `->index()` or `->useCurrent()` column modifier — they do NOT exist; use `$t->index(['col'])` and `->default('CURRENT_TIMESTAMP')` +✗ Use a fluent `->index()` column modifier — it does NOT exist; use `$t->index(['col'])` ✗ Combine a column `->primary()` with a Blueprint `$t->primary([...])` on the same table — pick one (double PRIMARY KEY error) ✗ Reference `seed:run`/`seed:fresh`/`seed:status` or `migrate:make` — the real commands are `db:seed` and `make:migration` ✗ Hardcode database table names — use string literals, never interpolation diff --git a/modules/let-migrate b/modules/let-migrate index 9ac3a8e..f13c977 160000 --- a/modules/let-migrate +++ b/modules/let-migrate @@ -1 +1 @@ -Subproject commit 9ac3a8ed16e03e95e88f9424602a6e0fa7866959 +Subproject commit f13c977d9d6e49c8f298b16395c5cc9a17a0b130 From a42473352d906a0ed5ce2e0937d75a826e8ca4af Mon Sep 17 00:00:00 2001 From: hakeemRash Date: Wed, 2 Sep 2026 12:29:30 +0300 Subject: [PATCH 07/14] =?UTF-8?q?release:=20v1.11.0=20=E2=80=94=20the=20mi?= =?UTF-8?q?gration=20engine=20works=20on=20every=20driver=20again?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A commit titled "refactor: remove deprecated methods" had put let-migrate's src/, tests/ and README back byte-for-byte to their state before a day of merged PR work, undoing four driver fixes and deleting the eight tests that covered them. Nothing was failing that the deletion fixed. This restores them and carries them forward with what running the compiler against real engines turned up -- including a PostgreSQL bug where every schema lookup silently matched nothing, because libpq's $1 placeholder is one PDO neither understands nor rejects. Verified by EXECUTION on SQLite, MariaDB 12.3 and PostgreSQL 18. SQL Server is the gap: no server was reachable and the sqlsrv extension is absent, so its two fixes are argued from the T-SQL specification rather than demonstrated. The new tests/Live suite runs them the moment LETMIGRATE_DB_SQLSRV points at a server, and skips with the reason until then -- never counted as a pass. --- CHANGELOG.md | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 940dc21..64e6d09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,73 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.11.0] - 2026-09-02 + +### Fixed +- **The migration engine only worked on MySQL** (`modules/let-migrate`). A + commit titled *"refactor: remove deprecated methods"* had restored `src/`, + `tests/` and the README byte-for-byte to their state before a day of merged + PR work — undoing four driver fixes and a Laravel-parity alias, and deleting + the eight tests that covered them. Nothing was failing that the deletion + fixed. Restored and carried forward: + - `ALTER TABLE` compiled MySQL syntax for every driver — additions batched + into one comma-separated statement, indexes added with `ADD KEY`, and drops + running columns BEFORE the indexes over them. A rollback written in the + correct order was reordered by the compiler into one that could not run + anywhere but MySQL, in the one direction nobody exercises until they + uninstall a plugin. + - PostgreSQL rejected `BOOLEAN DEFAULT 1`, and `modifyColumn()` emitted three + `;`-joined statements into a clause the extended query protocol refuses. + - SQLite could not add a foreign key to an existing table, and `modifyColumn()` + compiled `CREATE TABLE "__tmp_users" ()` — it was broken outright, because + the rebuild SQLite requires was driven from a blueprint holding only the + delta. It now reconstructs the full table from the `SchemaInspector`, + carrying existing indexes across and unwrapping defaults so a literal is + not re-quoted on every rebuild. + - Seeding a second database in one run died with *Cannot redeclare class* — + reachable the moment one run seeds once per driver, which is what + `hkm ground migrate` does. +- **PostgreSQL: every schema lookup silently matched nothing.** The driver and + inspector used libpq's `$1` placeholders, which PDO neither understands nor + rejects — so `tableExists()` answered `false` for a table with seven columns, + and the inspector reported no columns, indexes or foreign keys. Anything + guarded by `hasTable()`, and everything built on schema diffing or dumping, + was quietly wrong on that driver. Found only by executing against a live + server. +- **SQL Server emitted invalid T-SQL for every column addition and every + foreign key** — `ALTER TABLE … ADD COLUMN` (T-SQL has no `COLUMN` keyword + there) and `ON DELETE RESTRICT` (unimplemented; its actions are `NO ACTION`, + `CASCADE`, `SET NULL`, `SET DEFAULT`). Both are argued from the T-SQL + specification and are **not** verified against a live server — none was + reachable — but each replaces SQL the server rejects outright. +- **`MigrationConfig`: singular `path` overrode plural `paths`** instead of + acting as its fallback, so a config carrying both silently ran one directory + and ignored the array — failing by doing less work rather than by erroring. +- **`Blueprint::dropColumn()` accepted one column**, so `dropColumn('a', 'b')` + silently dropped only `a`. Now variadic, and the `drop*` methods chain. + +### Added +- **`useCurrent()` / `useCurrentOnUpdate()` / `bigIncrements()`** — Laravel + parity, so a ported migration compiles unchanged. Without them the failure is + a fatal *Call to undefined method* raised the moment the migration runs, + during a deploy. +- **`StatusRenderer`** — `migrate:status` as normalised data, aligned table + lines and JSON from one source, so the human and `--json` views cannot + disagree. +- **`tests/Live` — the migration compiler executed against every reachable + engine.** Configured with `LETMIGRATE_DB_MYSQL` / `_PGSQL` / `_SQLSRV` + (`GROUND_DB_*` honoured); each run uses its own scratch database and drops it. + A driver that is unconfigured or not answering SKIPS with the reason, never + counted as a pass. This release was verified on SQLite, MariaDB 12.3 and + PostgreSQL 18; SQL Server skipped, and says so. + +### Changed +- `docs/guides/18_MIGRATIONS.md` — `->useCurrent()` and `->useCurrentOnUpdate()` + now exist, so the anti-pattern entry saying they do not is corrected. The + `->index()` half stands: an index is declared on the Blueprint, not the + column. + + ## [1.10.1] - 2026-09-01 ### Fixed From 8ae30d060584e0fa7eb29289e17ce0f69ceef822 Mon Sep 17 00:00:00 2001 From: hakeemRash Date: Wed, 2 Sep 2026 13:52:51 +0300 Subject: [PATCH 08/14] chore(let-migrate): update subproject commit reference to 8982be2 Picks up the commit-msg hook and the composer wiring that points a clone at it. Without this bump the submodule checkout stays on f13c977, which has neither -- and that is the checkout anyone working in this workspace actually uses, so the hook would protect the standalone clone and nobody else. No functional change to the migration engine. --- modules/let-migrate | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/let-migrate b/modules/let-migrate index f13c977..8982be2 160000 --- a/modules/let-migrate +++ b/modules/let-migrate @@ -1 +1 @@ -Subproject commit f13c977d9d6e49c8f298b16395c5cc9a17a0b130 +Subproject commit 8982be2a3517a86fcc4cedfc7d71e0179d1aff47 From 8dd3d37034e5ec99b8b0245f8ec5a4ec7771a4be Mon Sep 17 00:00:00 2001 From: hakeemRash Date: Wed, 2 Sep 2026 18:03:26 +0300 Subject: [PATCH 09/14] =?UTF-8?q?release:=20v1.12.0=20=E2=80=94=20a=20depl?= =?UTF-8?q?oyed=20project=20is=20readable=20by=20the=20account=20that=20se?= =?UTF-8?q?rves=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `hkm install --production --owner=` only ever touched var/ and userdata/, so every directory a request actually reads — app/public_html, src/, vendor/, plugins/ — kept the deploying user's ownership and whatever mode the clone arrived with. The pool could write logs it was never going to reach the code to produce. Three separate reasons a PHP-FPM boot failed on a tree that runs fine from the shell: - the pass covered two directories out of the project. It now covers the whole tree, and runs LAST — composer install and the plugin fetch both create files (vendor/, plugins/) as whoever ran the command, so running at step 3 meant the two largest directories in the project were created after the permissions were "fixed"; - .env was chmod'd 0600, which no pool running as another account can read APP_KEY through. It is 0640 now — group-readable, never group-writable, never world-anything; - nothing reported that the pool could not TRAVERSE to the project, which is unfixable from inside it: a home directory is 0700 on a stock Debian install. The offending parents are named, and only named. The model is split ownership: code owned by the deploy user and reachable by the web server through the GROUP (2750/0640), var/ and userdata/ group-writable (2770/0660). Code is never group-writable in either profile — an FPM pool that can rewrite the PHP it executes turns any file-write bug into code execution. Every directory carries setgid, code included, so a file a later deploy lands does not take the deploying account's primary group and drop out of the share. An already-executable file keeps its exec bit, re-granted only where the profile grants read, so bin/psp and vendor/bin/* survive at 0750 rather than 0751. Separately, the installed kernel was left at whatever the installing account's umask produced. /opt/hkm-kernel is shared infrastructure — every pool on the box loads its PHP from that one tree and none of them runs as the installing account — so umask 027 or 077 left it unreadable to all of them, while the install reported success because the installer could obviously read what it had just written. install.sh now normalises the tree it lays down. --- CHANGELOG.md | 52 ++++ tools/docs/hkm-cli-usage.md | 109 ++++++-- tools/install.sh | 19 ++ tools/src/commands/install.zig | 482 +++++++++++++++++++++++++++++---- tools/src/lib/util.zig | 11 + tools/src/tests.zig | 1 + 6 files changed, 596 insertions(+), 78 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 64e6d09..38b9259 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,58 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.12.0] - 2026-09-02 + +### Fixed +- **A project installed with `--production --owner=` still could not be served + by PHP-FPM.** The pass only ever touched `var/` and `userdata/`, so every + directory a request actually reads — `app/public_html`, `src/`, `vendor/`, + `plugins/` — kept the deploying user's ownership and whatever mode the clone + arrived with. The pool could write logs it was never going to reach the code + to produce. Three separate reasons a boot failed, each fixed: + - the pass now covers the WHOLE project tree, and runs LAST — after + `composer install` and the plugin fetch, both of which create `vendor/` and + `plugins/` as whoever ran the command. Running at step 3, as it did, meant + the two largest directories in the project were created *after* the + permissions were "fixed". + - `.env` was chmod'd `0600`. PHP-FPM running as another account cannot read + `APP_KEY` through that, and the boot fails on a file whose mode bits look + deliberate. It is now `0640` — group-readable, never group-writable, never + world-anything. + - nothing reported that the pool could not TRAVERSE to the project. Reaching + `app/public_html/index.php` needs execute on every parent directory, and a + home directory is `0700` on a stock Debian install — unfixable from inside + the project, so the offending parents are now named (reported only, never + changed). + +- **The installed kernel was left at whatever the installing account's umask + produced** (`tools/install.sh`). `/opt/hkm-kernel` is shared infrastructure — + every PHP-FPM pool on the box loads its PHP out of that one tree, and none of + those pools runs as the account that installed it. With `umask 027` or `077` + the whole tree landed 0750/0700 and every site died with "Permission denied" + on a kernel file, while the install reported success because the installer + could obviously read what it had just written. The installer now normalises + the tree it lays down: directories traversable, files readable, and anything + that WAS executable still executable. + +### Changed +- `hkm install --production` / `--owner=` now apply a split-ownership model: + code owned by the deploy user and only READABLE through the web server's + group (`2750`/`0640`), `var/` and `userdata/` group-writable (`2770`/`0660`). + EVERY directory carries setgid, code included: the group is the only thing + granting the pool access, so a file created later — a log written at 3am, a + file a `git pull` lands — would otherwise take the creating account's primary + group and drop out of the share, and each deploy would silently un-share + whatever it touched. + Code is never group-writable in either profile — an FPM pool that can rewrite + the PHP it executes turns any file-write bug into code execution. An + already-executable file keeps its exec bit (re-granted only where the profile + grants read, so `bin/psp` and `vendor/bin/*` survive at `0750`, not `0751`), + and `.git` is skipped by both the chown and the chmod. +- The pass re-stats what it changed and reports any mode the filesystem + refused, instead of reporting success for a chmod the kernel rejected. + + ## [1.11.0] - 2026-09-02 ### Fixed diff --git a/tools/docs/hkm-cli-usage.md b/tools/docs/hkm-cli-usage.md index 2d47120..dc9c0fa 100755 --- a/tools/docs/hkm-cli-usage.md +++ b/tools/docs/hkm-cli-usage.md @@ -80,8 +80,8 @@ hkm install [path|name] [options] --no-plugins skip fetching the bootstrap's plugins --no-chmod skip fixing var/ and userdata/ mode bits --verify-plugins run each plugin's own test suite while installing (slow) ---production, --prod tighter mode bits (dir 0750/file 0640, no world access) ---owner=[:] chown var/ and userdata/ to this user[:group] (needs root/sudo) +--production, --prod harden the WHOLE tree: code 0750/0640, var+userdata 2770/0660 +--owner=[:] chown the whole project to this user[:group] (needs root/sudo) ``` What it does, in order: registers the project in the kernel registry; recreates @@ -95,33 +95,100 @@ right after scaffolding. ### `--production` / `--owner` — correct permissions AND ownership on a server -Dev mode chmods `var/`/`userdata/` to `0775`/`0664` (group-writable, world -readable) and stops there — good enough when the files are already owned by -whoever is running `hkm`. On a real server that is rarely the case: the web -server / PHP-FPM pool usually runs as its own account (`www-data`, `nginx`, -`app`, …), and CHMOD alone cannot fix that — only `chown` can. - -- `--production` swaps the mode bits for `0750`/`0640` (owner + group only, no - "other" access at all). It does **not** guess an owner — correctness matters - more than convenience here, and guessing wrong on a shared box is worse than - asking. -- `--owner=[:]` recursively `chown`s `var/` and `userdata/` to - that account. It is passed straight through to the system `chown`, so - `www-data`, `www-data:www-data` and `:www-data` (group only) all work. - Requires root/sudo unless the process already owns the target files — a - failed chown is reported per-directory, never swallowed silently. +Dev mode chmods `var/`/`userdata/` to `0775`/`0664` and stops there — good +enough when the files are already owned by whoever is running `hkm`. On a real +server that is never the case: the web server / PHP-FPM pool runs as its own +account (`www-data`, `nginx`, `app`, …), and it needs to READ every PHP file in +the project and EXECUTE (traverse) every directory holding one — not just write +to `var/`. Chmod alone cannot arrange that; only `chown` can. + +Passing `--production` or `--owner=` runs a **hardening pass over the whole +project**, as the LAST step of the install — after `composer install` and the +plugin fetch, because both create files (`vendor/`, `plugins/`) owned by +whoever ran the command. + +The model is **split ownership**: the deploy account keeps the code, the pool +reaches it through the GROUP. + +| Path | `--production` | default (`--owner` alone) | +|---|---|---| +| directories holding code | `2750` | `2755` | +| files holding code | `0640` | `0644` | +| an already-executable file (`bin/psp`, `vendor/bin/*`) | `0750` | `0755` | +| `var/`, `userdata/` directories | `2770` | `2775` | +| `var/`, `userdata/` files | `0660` | `0664` | +| `.env` | `0640` | `0640` | +| `.git` | untouched | untouched | + +**There is no chmod-only version of this.** Opening the tree with +`chmod -R o+rX` looks like it would avoid the chown, and it cannot: `.env` +carries `APP_KEY` and the database password, so world-readable is the one thing +it must never be — and `var/cache/manifests` needs the pool to WRITE (the boot +compiles manifests into it on every request unless `BOOT_CACHE=1`), which no +amount of "execute for others" grants. Reaching the tree through a shared GROUP +is what covers both. + +Five details are load-bearing: + +- **`.env` is `0640`, never `0600`.** PHP-FPM has to read `APP_KEY`, and a + `0600` `.env` owned by the deploy user is the single most common reason a tree + that runs fine from the shell fails to boot under a pool running as another + account. Group-readable, never group-writable, never world-anything. +- **Code is never group-writable**, in either profile. An FPM pool that can + rewrite the PHP it is executing turns any file-write bug into remote code + execution. Only `var/` and `userdata/` — which the application genuinely + writes — are group-writable. +- **Every directory carries setgid** (the `2` prefix), code included. The group + is the only thing granting the pool access, and a file created later — a log + the pool writes at 3am, a file a `git pull` lands — otherwise takes the + creating account's primary group and drops out of the share. Without it every + deploy silently un-shares whatever it touched, and the site 500s on a file + that was readable an hour ago. +- **An already-executable file keeps its exec bit**, re-granted only where the + profile grants read (so `0640` → `0750`, never `0751`). A flat `chmod 0640` + over the tree strips `bin/psp` and `vendor/bin/*`, and the install looks like + it worked right up until the first invocation. + +`.git` is skipped by both the chown and the chmod: it holds the whole history — +every secret ever committed and later removed included — nothing in a request +path reads it, and leaving it alone means the deploy user can still `git pull` +after a `sudo hkm install`. + +After the pass the command **re-stats what it changed** and reports any mode +that did not actually take (a filesystem refusing setgid, an ACL, not being +root), rather than reporting success for a chmod the kernel rejected. + +It also checks whether the pool can REACH the project at all: opening +`/home/deploy/shop/app/public_html/index.php` needs execute on every directory +down that path, and a home directory is `0700` on a stock Debian install. +Nothing inside the project can fix that, so the offending parents are named — +and reported only, never changed: widening a directory outside the project is +the operator's call. + +- `--owner=[:]` is passed straight through to the system `chown`, + so `www-data`, `deploy:www-data` and `:www-data` (group only) all work. + Requires root/sudo unless the process already owns the files; a failed chown + is reported, never swallowed silently. +- **`--owner=:www-data` (group only) is the form to reach for when the tree is + deployed by a CI account** — a Bitbucket/Jenkins user, a `git pull` from a + developer's login. It changes the GROUP and leaves the OWNER alone, so the + deploy account keeps writing to its own checkout while the pool gets in + through the group. Re-run it after a deploy that adds files; setgid keeps the + ones created inside existing directories correct on its own. - Set `HKM_PROD_OWNER` once in your deploy environment to avoid repeating `--owner=` on every run; an explicit `--owner=` flag always wins. -- `--production` with no `--owner` (and no `HKM_PROD_OWNER`) still tightens the - mode bits, but warns that ownership was left unchanged instead of guessing. +- `--production` with no `--owner` (and no `HKM_PROD_OWNER`) still applies the + mode bits, but warns that ownership was left unchanged instead of guessing — + correctness matters more than convenience, and guessing wrong on a shared box + is worse than asking. ```bash cd my-shop && hkm install # after a fresh git clone hkm install ./my-shop hkm install shop # by registered name hkm install --no-install # vendor/ already cached — skip composer -sudo hkm install --production --owner=www-data:www-data # on a server -HKM_PROD_OWNER=www-data:www-data sudo hkm install --production +sudo hkm install --production --owner=deploy:www-data # on a server +HKM_PROD_OWNER=deploy:www-data sudo hkm install --production ``` --- diff --git a/tools/install.sh b/tools/install.sh index 977117b..ef06cef 100755 --- a/tools/install.sh +++ b/tools/install.sh @@ -226,6 +226,25 @@ if [ -d "$DEST" ]; then mv "$DEST" "$OLD"; fi mv "$NEW" "$DEST" rm -rf "$OLD" +# ── make the installed kernel readable by the accounts that RUN it ────────── +# The kernel is SHARED infrastructure: every PHP-FPM pool on this box loads its +# PHP out of this one tree, and those pools run as their own accounts +# (www-data, nginx, a per-site user) — never as the account that ran this +# installer. `cp -R` above applies the INSTALLING account's umask, so on a box +# with umask 027 or 077 the whole tree lands mode 0750 or 0700 and every site +# dies with "Permission denied" on a kernel file it can see but not open. The +# install itself reports success, because the installer can obviously read what +# it just wrote. +# +# Normalise instead of trusting the umask: directories traversable, files +# readable, and anything that WAS executable still executable (bin/psp, +# vendor/bin/* — a flat 0644 pass strips them and the failure only shows up at +# the first invocation). There are no secrets in here; a project's .env lives +# in the project, not in the kernel. +find "$DEST" -type d -exec chmod 755 {} + 2>/dev/null || true +find "$DEST" -type f ! -perm -u+x -exec chmod 644 {} + 2>/dev/null || true +find "$DEST" -type f -perm -u+x -exec chmod 755 {} + 2>/dev/null || true + # Stage beside the destination, then rename over it. A plain `cp` truncates # and writes INTO the existing file, which fails with "Text file busy" the # moment that exact binary is the one currently running this script — i.e. diff --git a/tools/src/commands/install.zig b/tools/src/commands/install.zig index 01c085d..4bdc637 100644 --- a/tools/src/commands/install.zig +++ b/tools/src/commands/install.zig @@ -16,6 +16,9 @@ //! 5. `composer install` //! 6. fetch every plugin the project's own bootstrap wires (mirrors what //! `hkm new` does right after scaffolding — see lib/plugin_provision.zig) +//! 7. with --production / --owner: chown and chmod the WHOLE project for the +//! web server's account — last, because steps 5 and 6 create vendor/ and +//! plugins/ as whoever ran the command //! //! Every step besides directory creation can be skipped with a --no-* flag, for //! a CI image that already provisions one of them another way. @@ -65,17 +68,18 @@ const Options = struct { chmod: bool = true, verify_plugins: bool = false, help: bool = false, - /// --production: var/ and userdata/ get tighter mode bits (dir 0750, file - /// 0640 — no "other" access) instead of the dev defaults (dir 0775, file - /// 0664), and a chown failure (see `owner`) is reported per-path instead - /// of only implied by "no --owner given". + /// --production: run the hardening pass over the WHOLE project with no + /// "other" access at all — code 0750/0640, var+userdata 2770/0660, .env + /// 0640. Without it the pass still runs whenever `owner` is set, using the + /// group-and-world-readable dev modes (0775/0664). production: bool = false, - /// --owner=[:] (also HKM_PROD_OWNER) — chown var/ and - /// userdata/ to this user[:group], typically the account your web server - /// / PHP-FPM pool actually runs as. Passed straight to the system `chown`, - /// so `user`, `user:group` and `:group` (group-only) all work. Requires - /// root/sudo unless the process already owns the target files. Applies - /// whenever set, independent of --production and of --no-chmod. + /// --owner=[:] (also HKM_PROD_OWNER) — chown the whole + /// project to this user[:group], typically `deploy:www-data`: the deploy + /// account keeps the code, the web server / PHP-FPM pool reaches it through + /// the group. Passed straight to the system `chown`, so `user`, + /// `user:group` and `:group` (group-only) all work. Requires root/sudo + /// unless the process already owns the target files. Setting it triggers + /// the hardening pass on its own, independent of --production. owner: ?[]const u8 = null, }; @@ -136,8 +140,8 @@ fn printHelp() void { prompt.item("--no-plugins", "skip fetching the bootstrap's plugins"); prompt.item("--no-chmod", "skip fixing var/ and userdata/ mode bits"); prompt.item("--verify-plugins", "run each plugin's own test suite while installing (slow)"); - prompt.item("--production, --prod", "tighter mode bits (dir 0750/file 0640, no world access)"); - prompt.item("--owner=[:]", "chown var/ and userdata/ to this user[:group] (needs root/sudo)"); + prompt.item("--production, --prod", "harden the WHOLE tree: code 0750/0640, var+userdata 2770/0660"); + prompt.item("--owner=[:]", "chown the whole project to this user[:group] (needs root/sudo)"); prompt.item("--help, -h", "show this help"); prompt.blank(); prompt.section("Environment"); @@ -146,7 +150,8 @@ fn printHelp() void { prompt.blank(); prompt.section("Examples"); prompt.note("cd my-shop && hkm install"); - prompt.note("hkm install --production --owner=www-data:www-data"); + prompt.note("sudo hkm install --production --owner=deploy:www-data"); + prompt.muted("code readable+traversable by the pool's group, var/ and userdata/ writable, .env 0640."); prompt.outro("Run this once after `git clone` / `git pull` on a machine new to the project"); } @@ -198,21 +203,13 @@ pub fn run(allocator: std.mem.Allocator, io: Io, env: *EnvMap, args: []const []c prompt.muted("Runtime directories already present"); } - // 3. make sure they (and anything already inside them) are writable, and - // — in --production, or whenever --owner/HKM_PROD_OWNER is set — owned - // by the right user[:group] (typically the web server's own account). + // 3. make sure they (and anything already inside them) are writable by the + // account running this command — composer and the plugin fetch below + // both write into the project, so this cannot wait for the hardening + // pass in step 7. if (opts.chmod) { - fixPermissions(allocator, io, root, opts.production); - prompt.ok(if (opts.production) - "var/ and userdata/ set to production-safe permissions (0750/0640)" - else - "var/ and userdata/ set to writable permissions"); - } - if (opts.owner) |owner| { - if (owner.len > 0) fixOwnership(allocator, io, env, root, owner); - } else if (opts.production) { - prompt.warn("--production: no --owner given (and HKM_PROD_OWNER is unset) — ownership of var/ and userdata/ left unchanged."); - prompt.muted("pass --owner=[:] — typically your web server's account, e.g. www-data:www-data."); + fixPermissions(allocator, io, root); + prompt.muted("var/ and userdata/ set to writable permissions"); } // 4. .env — create from .env.example if absent, generate APP_KEY if empty. @@ -244,6 +241,17 @@ pub fn run(allocator: std.mem.Allocator, io: Io, env: *EnvMap, args: []const []c }; } + // 7. LAST — ownership and mode bits for the WHOLE project tree. + // + // Deliberately after composer and the plugin fetch: both create files + // (vendor/, plugins/) owned by whoever ran this command, so a pass any + // earlier would leave exactly the directories a request has to read + // owned by the wrong account — the reason a server boot fails under + // PHP-FPM while the same tree runs fine from the shell. + if (opts.production or opts.owner != null) { + hardenProject(allocator, io, env, root, opts.production, opts.owner); + } + prompt.note(""); prompt.note("Next steps:"); if (plugins_missing > 0) { @@ -361,62 +369,309 @@ fn ensureRuntimeDirs(allocator: std.mem.Allocator, io: Io, root: []const u8) !us /// `production` swaps the dev-friendly 0775/0664 for tighter 0750/0640 (no /// "other" access) — correct ownership (see fixOwnership) is what actually /// grants the web server access, so removing world access does not break it. -fn fixPermissions(allocator: std.mem.Allocator, io: Io, root: []const u8, production: bool) void { - const dir_mode: u32 = if (production) 0o750 else 0o775; - const file_mode: u32 = if (production) 0o640 else 0o664; - for ([_][]const u8{ "var", "userdata" }) |sub| { +/// Step 3: make `var/` and `userdata/` writable by the account running this +/// command, so composer and the plugin fetch can write into them. Deliberately +/// NOT the production mode bits — those are applied by the hardening pass at +/// the end, once every file that pass has to cover actually exists. +fn fixPermissions(allocator: std.mem.Allocator, io: Io, root: []const u8) void { + for (writable_subdirs) |sub| { const path = std.fmt.allocPrint(allocator, "{s}/{s}", .{ root, sub }) catch continue; if (!util.dirExists(Dir.cwd(), io, path)) continue; - util.chmodTreeWritable(allocator, io, path, dir_mode, file_mode, 8); + util.chmodTreeWritable(allocator, io, path, 0o775, 0o664, 8); } } // -------------------------------------------------------------------------- -// ownership +// hardening — the whole project tree, for a web server account // -------------------------------------------------------------------------- -/// chown var/ and userdata/ (recursively) to `owner`, reporting each path's -/// outcome explicitly — unlike chmod, a failed chown in production (wrong -/// privileges, a typo'd user/group) is exactly the kind of thing that should -/// NOT fail silently: the web server would still be unable to write. -fn fixOwnership(allocator: std.mem.Allocator, io: Io, env: *EnvMap, root: []const u8, owner: []const u8) void { +/// The subtrees the application WRITES to at runtime. Everything else in a +/// project is code the request only ever reads. +const writable_subdirs = [_][]const u8{ "var", "userdata" }; + +/// Never touched by the hardening pass. `.git` holds the whole history — every +/// secret ever committed and later removed included — and nothing in a request +/// path reads it, so widening it to the web server's group buys nothing and +/// gives away everything. It also stays owned by whoever cloned the repo, so a +/// later `git pull` as the deploy user still works after a `sudo hkm install`. +const harden_skip = [_][]const u8{".git"}; + +/// Mode bits for a hardening pass, in the split-ownership model this command +/// is built around: the code stays owned by the DEPLOY user and the web server +/// account reaches it through the GROUP. +/// +/// That is why every mode here grants the group read and directory-execute but +/// never write on code — an FPM pool that can rewrite the PHP it is executing +/// turns any file-write bug into remote code execution. Only `var/` and +/// `userdata/`, which the application genuinely writes, are group-writable, and +/// their directories carry setgid (`2` prefix) so a log file the pool creates +/// at 3am inherits the group instead of becoming unreadable to the deploy user. +const Modes = struct { + /// directories holding code + dir: u32, + /// regular files holding code + file: u32, + /// directories under var/ and userdata/ — setgid + group-writable + writable_dir: u32, + /// regular files under var/ and userdata/ + writable_file: u32, + /// .env and friends — group-READABLE, because PHP-FPM has to read APP_KEY, + /// and never world-readable whichever profile is in force. A 0600 .env is + /// the single most common reason a tree that runs from the shell fails to + /// boot under a pool running as another account. + secret: u32 = 0o640, + + fn of(production: bool) Modes { + return if (production) .{ + // setgid on code directories too, not just the writable ones: the + // group is the only thing granting the pool access, and a deploy + // that lands new files (a git pull, a plugin fetch) creates them + // with the DEPLOYING account's primary group unless the parent + // directory says otherwise. Without it every deploy silently + // un-shares whatever it touched, and the site 500s on a file that + // was readable an hour ago. + .dir = 0o2750, + .file = 0o640, + .writable_dir = 0o2770, + .writable_file = 0o660, + } else .{ + // World-readable, which is the point of the non-production profile + // — but NOT group-writable on code. The group here is the web + // server's, and 0664 source would let the pool rewrite the PHP it + // is executing. Only var/ and userdata/ below are group-writable. + .dir = 0o2755, + .file = 0o644, + .writable_dir = 0o2775, + .writable_file = 0o664, + }; + } +}; + +/// Apply ownership and then mode bits to the entire project. +/// +/// Order matters: chown FIRST, chmod second. POSIX lets chown clear the setuid +/// and setgid bits, so a chown running after the chmod would strip the setgid +/// this pass puts on `var/` — and the symptom (files the pool creates being +/// unreadable to the deploy user) shows up days later, nowhere near the cause. +fn hardenProject( + allocator: std.mem.Allocator, + io: Io, + env: *EnvMap, + root: []const u8, + production: bool, + owner: ?[]const u8, +) void { if (@import("builtin").os.tag == .windows) { - prompt.warn("--owner is not supported on Windows — skipped."); + prompt.warn("--production/--owner adjust POSIX mode bits and ownership — skipped on Windows."); return; } - var ok: usize = 0; - var total: usize = 0; - for ([_][]const u8{ "var", "userdata" }) |sub| { + prompt.note(""); + if (owner) |o| { + if (o.len > 0) fixOwnership(allocator, io, env, root, o); + } else { + prompt.warn("--production: no --owner given (and HKM_PROD_OWNER is unset) — ownership left unchanged."); + prompt.muted("pass --owner=[:] — typically your web server's account, e.g. deploy:www-data."); + } + + const m = Modes.of(production); + hardenTree(allocator, io, root, m); + prompt.ok(std.fmt.allocPrint( + allocator, + "Permissions applied — code {o}/{o}, var+userdata {o}/{o}, .env {o}", + .{ m.dir, m.file, m.writable_dir, m.writable_file, m.secret }, + ) catch "Permissions applied"); + + verifyModes(allocator, io, root, m); + reportTraversal(allocator, io, root); +} + +/// Re-stat the paths that decide whether the application boots, and say so when +/// a mode did not actually take. +/// +/// Every chmod in this pass is best-effort by contract (`util.chmodPath` +/// swallows the error), which is right for one file deep in `vendor/` and wrong +/// for `var/` — a setgid bit refused because the process is not in the target +/// group leaves a tree that looks hardened and is not. The kernel will not tell +/// you either: it fails later, at the first write, as a permission error on a +/// path whose ownership was just reported as correct. +fn verifyModes(allocator: std.mem.Allocator, io: Io, root: []const u8, m: Modes) void { + var bad: std.ArrayList([]const u8) = .empty; + + const Want = struct { path: []const u8, mode: u32 }; + var wants: std.ArrayList(Want) = .empty; + wants.append(allocator, .{ .path = root, .mode = m.dir }) catch return; + for (writable_subdirs) |sub| { const path = std.fmt.allocPrint(allocator, "{s}/{s}", .{ root, sub }) catch continue; - if (!util.dirExists(Dir.cwd(), io, path)) continue; - total += 1; - if (chownPath(io, env, path, owner)) { - ok += 1; - } else { - prompt.warn(std.fmt.allocPrint( - allocator, - "chown {s} {s} failed — needs root/sudo, or that user/group doesn't exist.", - .{ owner, sub }, - ) catch "chown failed — needs root/sudo, or that user/group doesn't exist."); + if (util.dirExists(Dir.cwd(), io, path)) wants.append(allocator, .{ .path = path, .mode = m.writable_dir }) catch {}; + } + const env_path = std.fmt.allocPrint(allocator, "{s}/.env", .{root}) catch null; + if (env_path) |ep| { + if (util.fileExists(io, ep)) wants.append(allocator, .{ .path = ep, .mode = m.secret }) catch {}; + } + + for (wants.items) |w| { + const actual = util.statMode(io, w.path) orelse continue; + if (actual == w.mode) continue; + bad.append(allocator, std.fmt.allocPrint( + allocator, + " {s} — wanted {o}, is {o}", + .{ w.path, w.mode, actual }, + ) catch continue) catch {}; + } + + if (bad.items.len == 0) return; + + prompt.warn("Some mode bits did not take — the pass reports what it asked for, not what the filesystem accepted:"); + for (bad.items) |line| prompt.muted(line); + prompt.muted("Usually: not running as root/sudo, a filesystem that refuses setgid, or an ACL overriding the mode."); +} + +/// Walk the project root once, dispatching each top-level entry to the mode +/// pair that belongs to it: `var/` and `userdata/` get the writable set, +/// everything else the read-only code set, and `.env*` the secret mode. +fn hardenTree(allocator: std.mem.Allocator, io: Io, root: []const u8, m: Modes) void { + util.chmodPath(io, root, m.dir); + + var dir = Dir.cwd().openDir(io, root, .{ .iterate = true }) catch return; + defer dir.close(io); + var it = dir.iterate(); + while (it.next(io) catch null) |entry| { + if (skipped(entry.name)) continue; + const child = std.fmt.allocPrint(allocator, "{s}/{s}", .{ root, entry.name }) catch continue; + switch (entry.kind) { + // A symlink's mode bits are not consulted by anything; chmod would + // follow it and rewrite a target that may sit outside the project. + .sym_link => continue, + .directory => { + if (contains(&writable_subdirs, entry.name)) { + applyTree(allocator, io, child, m.writable_dir, m.writable_file, 32); + } else { + applyTree(allocator, io, child, m.dir, m.file, 32); + } + }, + .file => chmodFile(io, child, if (std.mem.startsWith(u8, entry.name, ".env")) m.secret else m.file), + else => {}, } } +} + +/// Recursively chmod one subtree. Depth-limited and best-effort, matching +/// `util.chmodTreeWritable`: a directory that cannot be opened is skipped +/// rather than failing the command. 32 is chosen to clear a real `vendor/`, +/// which routinely nests past the 8 the writable-dirs pass uses. +fn applyTree(allocator: std.mem.Allocator, io: Io, path: []const u8, dir_mode: u32, file_mode: u32, depth: usize) void { + util.chmodPath(io, path, dir_mode); + if (depth == 0) return; + + var dir = Dir.cwd().openDir(io, path, .{ .iterate = true }) catch return; + defer dir.close(io); + var it = dir.iterate(); + while (it.next(io) catch null) |entry| { + if (skipped(entry.name)) continue; + const child = std.fmt.allocPrint(allocator, "{s}/{s}", .{ path, entry.name }) catch continue; + switch (entry.kind) { + .sym_link => continue, + .directory => applyTree(allocator, io, child, dir_mode, file_mode, depth - 1), + .file => chmodFile(io, child, file_mode), + else => {}, + } + } +} + +/// chmod one regular file to `mode`, KEEPING it executable if it already was. +/// +/// A flat `chmod 0640` over the tree is the obvious implementation and it +/// breaks the install: `bin/psp`, `vendor/bin/*` and every shipped shell script +/// lose their exec bit, and the failure surfaces as "command not found" long +/// after this command reported success. The exec bit is re-granted exactly +/// where `mode` grants read, so it never widens access beyond the profile. +fn chmodFile(io: Io, path: []const u8, mode: u32) void { + const current = util.statMode(io, path) orelse { + util.chmodPath(io, path, mode); + return; + }; + util.chmodPath(io, path, withExecBit(mode, (current & 0o111) != 0)); +} + +/// `mode`, plus an execute bit wherever `mode` already grants READ — and only +/// when the file was executable to begin with. Deriving x from r rather than +/// hardcoding 0o111 is what keeps the profile intact: under --production a +/// script comes out 0750, not 0751, so "no access for other" still holds for +/// the one class of file where a stray x bit is worth the most to an attacker. +fn withExecBit(mode: u32, executable: bool) u32 { + if (!executable) return mode; + return mode | ((mode & 0o444) >> 2); +} + +fn skipped(name: []const u8) bool { + return contains(&harden_skip, name); +} + +fn contains(haystack: []const []const u8, needle: []const u8) bool { + for (haystack) |h| { + if (std.mem.eql(u8, h, needle)) return true; + } + return false; +} + +// -------------------------------------------------------------------------- +// ownership +// -------------------------------------------------------------------------- + +/// chown the whole project to `owner`, reporting the outcome explicitly — +/// unlike chmod, a failed chown (wrong privileges, a typo'd user/group) is +/// exactly the kind of thing that should NOT fail silently: the web server +/// would still be unable to read the code or write its logs. +/// +/// `.git` is chowned separately — it is not, so that the deploy user keeps +/// being able to `git pull` after a `sudo hkm install`. That is why the root +/// itself is chowned non-recursively and each top-level entry individually, +/// rather than one `chown -R` over the project. +fn fixOwnership(allocator: std.mem.Allocator, io: Io, env: *EnvMap, root: []const u8, owner: []const u8) void { + var failed: usize = 0; + + if (!chownPath(io, env, root, owner, false)) failed += 1; + + var dir = Dir.cwd().openDir(io, root, .{ .iterate = true }) catch { + prompt.warn("Could not read the project root — ownership left unchanged."); + return; + }; + defer dir.close(io); + + var it = dir.iterate(); + while (it.next(io) catch null) |entry| { + if (skipped(entry.name)) continue; + const child = std.fmt.allocPrint(allocator, "{s}/{s}", .{ root, entry.name }) catch continue; + if (!chownPath(io, env, child, owner, true)) failed += 1; + } - if (total > 0 and ok == total) { - prompt.ok(std.fmt.allocPrint(allocator, "var/ and userdata/ owned by {s}", .{owner}) catch "var/ and userdata/ ownership fixed"); + if (failed == 0) { + prompt.ok(std.fmt.allocPrint(allocator, "Project owned by {s} (.git left as-is)", .{owner}) catch "Ownership fixed"); + } else { + prompt.warn(std.fmt.allocPrint( + allocator, + "chown {s} failed on {d} path(s) — needs root/sudo, or that user/group doesn't exist.", + .{ owner, failed }, + ) catch "chown failed — needs root/sudo, or that user/group doesn't exist."); } } -/// `chown -R `. Shells out rather than resolving the user/group +/// `chown [-R] `. Shells out rather than resolving the user/group /// name to a uid/gid natively — the OS's own NSS already knows how to do that /// correctly (files, LDAP, whatever `/etc/nsswitch.conf` says), and `chown` /// already accepts `user`, `user:group` and `:group` (group-only) verbatim, so /// passing `owner` straight through keeps that flexibility for free. Returns /// whether the process exited 0. -fn chownPath(io: Io, env: *EnvMap, path: []const u8, owner: []const u8) bool { +fn chownPath(io: Io, env: *EnvMap, path: []const u8, owner: []const u8, recursive: bool) bool { const chown_bin = env.get("HKM_CHOWN_BIN") orelse "chown"; + const argv: []const []const u8 = if (recursive) + &.{ chown_bin, "-R", owner, path } + else + &.{ chown_bin, owner, path }; + var child = std.process.spawn(io, .{ - .argv = &.{ chown_bin, "-R", owner, path }, + .argv = argv, .environ_map = env, .stdin = .ignore, .stdout = .ignore, @@ -430,6 +685,44 @@ fn chownPath(io: Io, env: *EnvMap, path: []const u8, owner: []const u8) bool { }; } +// -------------------------------------------------------------------------- +// traversal +// -------------------------------------------------------------------------- + +/// Report ancestor directories the web server account cannot pass through. +/// +/// Getting the project's OWN permissions right is not sufficient: to open +/// `/home/deploy/shop/app/public_html/index.php` the pool needs execute on +/// EVERY directory down the path, and a home directory is 0700 on a stock +/// Debian install. Nothing inside the project can fix that, and the resulting +/// failure reads as a permission error on a file whose mode bits are visibly +/// correct — so name the actual directory instead of leaving it to be guessed. +/// +/// Reported, never changed: widening a directory that is not part of the +/// project is the operator's call, not this command's. +fn reportTraversal(allocator: std.mem.Allocator, io: Io, root: []const u8) void { + var blocked: std.ArrayList([]const u8) = .empty; + + var cursor: ?[]const u8 = util.parentOf(root); + while (cursor) |path| : (cursor = util.parentOf(path)) { + if (util.statMode(io, path)) |mode| { + // No execute for "other" — reachable only by the owner or a member + // of the directory's group, which a web server account rarely is. + if ((mode & 0o001) == 0) { + blocked.append(allocator, std.fmt.allocPrint(allocator, "{s} ({o})", .{ path, mode }) catch path) catch {}; + } + } + if (std.mem.eql(u8, path, "/")) break; + } + + if (blocked.items.len == 0) return; + + prompt.warn("The web server may not be able to REACH the project — these parent directories deny traversal to others:"); + for (blocked.items) |item| prompt.muted(std.fmt.allocPrint(allocator, " {s}", .{item}) catch item); + prompt.muted("Each one needs execute for the pool's account: `chmod o+x `, add the account to its group, or"); + prompt.muted("move the project somewhere the web server already reaches (/var/www, /srv)."); +} + // -------------------------------------------------------------------------- // .env / APP_KEY // -------------------------------------------------------------------------- @@ -548,3 +841,78 @@ fn composerInstall(allocator: std.mem.Allocator, io: Io, env: *EnvMap, root: []c else => prompt.warn("composer install was interrupted."), } } + +// -------------------------------------------------------------------------- +// tests +// -------------------------------------------------------------------------- + +test "withExecBit leaves a non-executable file's mode alone" { + try std.testing.expectEqual(@as(u32, 0o640), withExecBit(0o640, false)); + try std.testing.expectEqual(@as(u32, 0o664), withExecBit(0o664, false)); + try std.testing.expectEqual(@as(u32, 0o660), withExecBit(0o660, false)); +} + +test "withExecBit re-grants x exactly where the mode grants r" { + // The regression this guards: a flat chmod 0640 over the tree strips the + // exec bit from bin/psp and vendor/bin/*, and the install only looks like + // it worked until the first invocation. + try std.testing.expectEqual(@as(u32, 0o750), withExecBit(0o640, true)); + try std.testing.expectEqual(@as(u32, 0o775), withExecBit(0o664, true)); + try std.testing.expectEqual(@as(u32, 0o770), withExecBit(0o660, true)); +} + +test "withExecBit never widens beyond the profile" { + // --production grants nothing to "other", so neither may the exec bit. + for ([_]u32{ 0o640, 0o660, 0o600 }) |mode| { + try std.testing.expectEqual(@as(u32, 0), withExecBit(mode, true) & 0o007); + } +} + +test "production modes deny other, dev modes keep the previous defaults" { + const prod = Modes.of(true); + try std.testing.expectEqual(@as(u32, 0o2750), prod.dir); + try std.testing.expectEqual(@as(u32, 0o640), prod.file); + try std.testing.expectEqual(@as(u32, 0o2770), prod.writable_dir); + try std.testing.expectEqual(@as(u32, 0o660), prod.writable_file); + for ([_]u32{ prod.dir, prod.file, prod.writable_dir, prod.writable_file, prod.secret }) |m| { + try std.testing.expectEqual(@as(u32, 0), m & 0o007); + } + + // Dev keeps the historic 0775/0664 for the runtime tree (what the pre- + // hardening `fixPermissions` applied), but code is only world-READABLE. + const dev = Modes.of(false); + try std.testing.expectEqual(@as(u32, 0o2755), dev.dir); + try std.testing.expectEqual(@as(u32, 0o644), dev.file); + try std.testing.expectEqual(@as(u32, 0o2775), dev.writable_dir); + try std.testing.expectEqual(@as(u32, 0o664), dev.writable_file); +} + +test "both profiles make var/ group-writable and setgid, and .env group-readable" { + for ([_]Modes{ Modes.of(true), Modes.of(false) }) |m| { + // group write on the runtime tree — the pool has to write logs + try std.testing.expect(m.writable_dir & 0o020 != 0); + try std.testing.expect(m.writable_file & 0o020 != 0); + // setgid on BOTH trees: the writable one so a file the pool creates + // keeps the deploy user's group, the code one so a deploy that pulls + // new files does not un-share them from the pool. + try std.testing.expect(m.writable_dir & 0o2000 != 0); + try std.testing.expect(m.dir & 0o2000 != 0); + // group READ but never group WRITE on .env — FPM reads APP_KEY, and a + // 0600 .env is the most common reason an FPM boot fails on a tree that + // runs fine from the shell. + try std.testing.expect(m.secret & 0o040 != 0); + try std.testing.expectEqual(@as(u32, 0), m.secret & 0o020); + try std.testing.expectEqual(@as(u32, 0), m.secret & 0o007); + // code is never group-writable — an FPM pool that can rewrite the PHP + // it executes turns any file-write bug into code execution. + try std.testing.expectEqual(@as(u32, 0), m.file & 0o020); + try std.testing.expectEqual(@as(u32, 0), m.dir & 0o020); + } +} + +test "skipped protects .git and nothing else" { + try std.testing.expect(skipped(".git")); + try std.testing.expect(!skipped("var")); + try std.testing.expect(!skipped(".env")); + try std.testing.expect(!skipped("vendor")); +} diff --git a/tools/src/lib/util.zig b/tools/src/lib/util.zig index 24ff695..a4b17de 100644 --- a/tools/src/lib/util.zig +++ b/tools/src/lib/util.zig @@ -241,6 +241,17 @@ pub fn chmodPath(io: Io, path: []const u8, mode: u32) void { Dir.cwd().setFilePermissions(io, path, @enumFromInt(mode), .{}) catch {}; } +/// A path's current mode bits, or null when it cannot be stat'd (gone, or a +/// directory this process may not look into). Symlinks are NOT followed: the +/// caller is fixing permissions inside a tree, and a symlink's target is +/// routinely outside it — following one would rechmod a file the walk never +/// intended to touch. Always null on Windows, which has no mode bits. +pub fn statMode(io: Io, path: []const u8) ?u32 { + if (@import("builtin").os.tag == .windows) return null; + const st = Dir.cwd().statFile(io, path, .{ .follow_symlinks = false }) catch return null; + return @intFromEnum(st.permissions) & 0o7777; +} + /// Recursively make `path` writable: `dirMode` (e.g. 0o775) on every directory /// including `path` itself, `fileMode` (e.g. 0o664) on every regular file /// beneath it. Fixes a runtime tree (var/, userdata/) left behind by a diff --git a/tools/src/tests.zig b/tools/src/tests.zig index 6278dcc..027083b 100644 --- a/tools/src/tests.zig +++ b/tools/src/tests.zig @@ -27,6 +27,7 @@ const std = @import("std"); test { _ = @import("commands/cli.zig"); _ = @import("commands/discover.zig"); + _ = @import("commands/install.zig"); _ = @import("commands/doctor.zig"); _ = @import("commands/list.zig"); _ = @import("commands/module.zig"); From a866d5b4a298f6fac700b4d126afa32df02221fe Mon Sep 17 00:00:00 2001 From: hakeemRash Date: Wed, 2 Sep 2026 18:42:42 +0300 Subject: [PATCH 10/14] fix(plugins): a public plugin repo asked for a GitHub account MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `FileManager` was the one hyphenated plugin missing from the slug override table, so it resolved to hkm-plugin-filemanager — a repository that does not exist. GitHub answers 404 for "does not exist" and "not yours" alike; it will not confirm a private repository to an anonymous request. Git cannot tell the two apart, assumed the second, and stopped to ask for a username and password that no account could have satisfied, on a plugin anyone can clone. Added the override, plus tests pinning every multi-word folder to its real hyphenated slug and round-tripping it back to the PSR-4 folder name, so the next repo added with a hyphen cannot drift the same way. Separately, the fetch inherited the terminal, so an unreachable remote hung the whole install on a password box until somebody killed it — on a deploy box or in CI, indefinitely. Every git invocation now runs with GIT_TERMINAL_PROMPT=0 and SSH BatchMode=yes: the remote fails immediately and the call site names the plugin and the URL, which is the information actually needed. HKM_GIT_INTERACTIVE=1 restores the prompt for a genuinely private remote to authenticate against by hand. --- CHANGELOG.md | 18 +++++++++++++ tools/src/lib/plugin_git.zig | 33 +++++++++++++++++++++++- tools/src/lib/plugin_registry.zig | 42 +++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38b9259..e49a7da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 the tree it lays down: directories traversable, files readable, and anything that WAS executable still executable. +- **A plugin fetch asked for a GitHub account, for a repo that is public.** + `FileManager` was the one hyphenated plugin missing from the slug override + table, so it resolved to `hkm-plugin-filemanager` — a repository that does not + exist. GitHub answers **404 for "does not exist" and "not yours" alike**; it + will not confirm a private repo to an anonymous request. Git cannot tell those + apart, assumed the second, and stopped to ask for a username and password that + no account could have satisfied. Added the override, plus tests pinning every + multi-word folder to its real hyphenated slug (and round-tripping back to the + PSR-4 folder name) so the next repo added with a hyphen cannot drift the same + way. +- **Git could block a deploy on a credential prompt.** The plugin fetch inherited + the terminal, so an unreachable remote hung `hkm install` on a password box + until somebody killed it — on a deploy box or in CI, indefinitely. Every git + invocation now runs with `GIT_TERMINAL_PROMPT=0` and SSH `BatchMode=yes`: a bad + remote fails immediately and the call site names the plugin and URL, which is + the information actually needed. `HKM_GIT_INTERACTIVE=1` restores the prompt + for a genuinely private remote you intend to authenticate against by hand. + ### Changed - `hkm install --production` / `--owner=` now apply a split-ownership model: code owned by the deploy user and only READABLE through the web server's diff --git a/tools/src/lib/plugin_git.zig b/tools/src/lib/plugin_git.zig index 8b46765..9341e6b 100644 --- a/tools/src/lib/plugin_git.zig +++ b/tools/src/lib/plugin_git.zig @@ -12,6 +12,7 @@ const std = @import("std"); const semver = @import("semver.zig"); const prompt = @import("prompt.zig"); +const util = @import("util.zig"); const Dir = std.Io.Dir; const Io = std.Io; @@ -42,7 +43,35 @@ pub fn available(allocator: std.mem.Allocator, io: Io, env: *EnvMap) bool { } /// Run git capturing stdout. Returns null when the command failed. +/// Git must never stop and ask for a password. +/// +/// GitHub answers **404 for a repository that does not exist and for one the +/// caller may not see** — it will not confirm a private repo's existence to an +/// anonymous request. Git cannot tell those apart, so it assumes the second and +/// falls back to asking for a username and password. The result is a credential +/// prompt in the middle of a plugin fetch for a repo that is PUBLIC and simply +/// renamed, misspelled, or not published yet: no account can unlock it, typing +/// one cannot help, and an unattended `hkm install` on a deploy box or in CI +/// hangs there until somebody kills it. +/// +/// `GIT_TERMINAL_PROMPT=0` turns that into git's own one-line error naming the +/// URL, which the call site already prints — the information actually needed. +/// `BatchMode=yes` does the same for an SSH remote's passphrase prompt. +/// +/// Set `HKM_GIT_INTERACTIVE=1` to restore the prompt for a genuinely private +/// remote you intend to authenticate against by hand. An existing +/// `GIT_SSH_COMMAND` is left alone — it is the user's own, and it may already +/// carry the identity file the fetch depends on. +fn noPrompt(env: *EnvMap) void { + if (util.envIsTruthy(env, "HKM_GIT_INTERACTIVE")) return; + env.put("GIT_TERMINAL_PROMPT", "0") catch {}; + if (env.get("GIT_SSH_COMMAND") == null) { + env.put("GIT_SSH_COMMAND", "ssh -o BatchMode=yes") catch {}; + } +} + fn capture(allocator: std.mem.Allocator, io: Io, env: *EnvMap, argv: []const []const u8) ?[]const u8 { + noPrompt(env); const res = std.process.run(allocator, io, .{ .argv = argv, .environ_map = env }) catch return null; switch (res.term) { .exited => |c| if (c != 0) return null, @@ -53,6 +82,7 @@ fn capture(allocator: std.mem.Allocator, io: Io, env: *EnvMap, argv: []const []c /// Run git with stdio inherited, so clone/fetch progress reaches the terminal. fn passthrough(io: Io, env: *EnvMap, argv: []const []const u8) !u8 { + noPrompt(env); var child = try std.process.spawn(io, .{ .argv = argv, .environ_map = env, @@ -331,7 +361,8 @@ pub fn isDirty(allocator: std.mem.Allocator, io: Io, env: *EnvMap, dir: []const pub fn explain(e: GitError) []const u8 { return switch (e) { GitError.GitMissing => "git is not installed or not on PATH", - GitError.RemoteUnreachable => "could not reach the remote (offline, private, or the repo does not exist)", + GitError.RemoteUnreachable => "could not reach the remote — it does not exist, is private, or you are offline " + ++ "(GitHub answers 404 for 'missing' and 'not yours' alike; check the spelling first)", GitError.RefNotFound => "the requested version does not exist on the remote", GitError.CommandFailed => "the git command failed", }; diff --git a/tools/src/lib/plugin_registry.zig b/tools/src/lib/plugin_registry.zig index 699f00e..b667962 100644 --- a/tools/src/lib/plugin_registry.zig +++ b/tools/src/lib/plugin_registry.zig @@ -30,6 +30,7 @@ pub const default_org = "AlfaCode-Team"; /// scattering special cases through the commands. const slug_overrides = [_]struct { folder: []const u8, slug: []const u8 }{ .{ .folder = "DevTools", .slug = "dev-tools" }, + .{ .folder = "FileManager", .slug = "file-manager" }, .{ .folder = "HttpClient", .slug = "http-client" }, .{ .folder = "RedisCache", .slug = "redis-cache" }, .{ .folder = "SecurityFilters", .slug = "security-filters" }, @@ -369,3 +370,44 @@ test "only the configured org's plugin repos count as first-party" { try std.testing.expect(!remoteIsFirstParty(&env, "https://git.internal/AlfaCode-Team/hkm-plugin-logger.git")); try std.testing.expect(!remoteIsFirstParty(&env, "/srv/git/hkm-plugin-logger.git")); } + +test "every multi-word plugin folder maps to its real hyphenated repo slug" { + // These are the repos that actually exist under the org with a hyphen. A + // folder missing from slug_overrides silently loses the hyphen — the URL + // 404s, and because GitHub answers 404 for "private" too, git asks for a + // username and password for a repo that is PUBLIC. FileManager was missing + // here, and that is exactly how it presented: an account prompt during + // `hkm install`, on a plugin anyone can clone. + const cases = [_]struct { folder: []const u8, slug: []const u8 }{ + .{ .folder = "DevTools", .slug = "dev-tools" }, + .{ .folder = "FileManager", .slug = "file-manager" }, + .{ .folder = "HttpClient", .slug = "http-client" }, + .{ .folder = "RedisCache", .slug = "redis-cache" }, + .{ .folder = "SecurityFilters", .slug = "security-filters" }, + .{ .folder = "SocialAuth", .slug = "social-auth" }, + }; + for (cases) |c| { + const got = try slugFor(std.testing.allocator, c.folder); + defer std.testing.allocator.free(got); + try std.testing.expectEqualStrings(c.slug, got); + // and the round trip back to the PSR-4 folder name must survive + const back = try canonicalName(std.testing.allocator, c.slug); + defer std.testing.allocator.free(back); + try std.testing.expectEqualStrings(c.folder, back); + } +} + +test "single-word repos are NOT hyphenated by accident" { + const cases = [_]struct { folder: []const u8, slug: []const u8 }{ + .{ .folder = "SiteSEO", .slug = "siteseo" }, + .{ .folder = "ViteManifest", .slug = "vitemanifest" }, + .{ .folder = "OAuth2", .slug = "oauth2" }, + .{ .folder = "View", .slug = "view" }, + .{ .folder = "Crypto", .slug = "crypto" }, + }; + for (cases) |c| { + const got = try slugFor(std.testing.allocator, c.folder); + defer std.testing.allocator.free(got); + try std.testing.expectEqualStrings(c.slug, got); + } +} From 790444cc30f7d44362d6524a4d6c639910f42906 Mon Sep 17 00:00:00 2001 From: hakeemRash Date: Thu, 3 Sep 2026 02:21:38 +0300 Subject: [PATCH 11/14] =?UTF-8?q?release:=20v1.13.0=20=E2=80=94=20the=20ac?= =?UTF-8?q?count=20that=20serves=20a=20project=20can=20read=20its=20plugin?= =?UTF-8?q?s,=20and=20.env=20can=20be=20audited?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --owner stopped at the project boundary. A project's plugins are symlinks into the global store, and both halves of the hardening pass skip symlinks on purpose, so every plugin file kept the deploying user's ownership under a report that said the project was owned by the pool. It now chowns the store versions the project links to, and the traversal check covers the store's own parents — the store defaults under $HOME, which a sudo deploy resolves to /root/.cache. hkm env is new: a .env accumulates duplicate keys that no parser reports, because the loader resolves them silently and the last active assignment wins. It reports them with the live one marked, resolves them one prompt at a time, and groups the file by declaring plugin then by feature. Enabling an already-enabled plugin now tops up its env block, so a plugin that declares a new config[] entry in a later version stops failing the boot on a key nothing wrote; new keys merge into the block the plugin already owns instead of opening a second one. resolveRoot walks up to find proj.json, so every command that takes a project works from anywhere inside it. An explicit path stays exact. --- CHANGELOG.md | 73 ++++ tools/src/commands/env.zig | 646 +++++++++++++++++++++++++++++++++ tools/src/commands/install.zig | 216 ++++++++++- tools/src/commands/plugins.zig | 48 ++- tools/src/lib/env_file.zig | 453 +++++++++++++++++++++++ tools/src/lib/plugin_env.zig | 125 +++++-- tools/src/lib/services.zig | 65 ++++ tools/src/main.zig | 7 + tools/src/tests.zig | 2 + 9 files changed, 1597 insertions(+), 38 deletions(-) create mode 100644 tools/src/commands/env.zig create mode 100644 tools/src/lib/env_file.zig diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b777f4..2d8ecae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,79 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.13.0] - 2026-09-03 + +### Fixed +- **`hkm install --owner=` left every plugin file owned by the deploying user.** + A project's plugins are not in the project: `hkm plugins install` keeps one + copy per (plugin, version, origin) in the global store and links the project + at it, so `plugins/Logger` is a symlink out of the tree. Both halves of the + hardening pass stopped at that boundary by design — `hardenTree` skips + symlinks because a chmod would follow one and rewrite a target outside the + project, and the chown only walked the project root. The result was a project + that verified clean and could not serve: every file the pool has to read + first, every Provider and every controller a route resolves to, still belonged + to whoever ran the command, under a report that said `Project owned by + deploy:www-data`. `--owner` now also chowns the store versions the project + links to, plus the directories between them and the store root so the trees it + just chowned can be reached. Only the versions THIS project links to: the store + is shared by every project on the machine, and claiming all of it for one + project's web account is not that command's call. +- **`--production` reported a reachable project while the plugins were + unreachable.** The traversal check walked the parents of the project root only. + Since the store moved out of the project it defaults to `$HOME/.cache`, which a + deploy under sudo resolves to `/root/.cache` — 0700 on every mainstream distro + — so the chown succeeded on every entry and the site still could not read one + of them. The check now covers the store's own parents, with its own remedy: + relocate the store (`hkm plugins store --set=`, or `HKM_PLUGIN_STORE`) rather + than widen a home directory to reach a cache. +- **A plugin that gained an env var never got it.** `hkm plugins enable` returns + early when the plugin and its dependencies are already wired, so a plugin + declaring a new `config[]` entry in a later version left an `.env` block that + was now incomplete — and the boot failed on the missing key with nothing + pointing at the cause. Enabling an already-enabled plugin now tops up its + block. Safe by construction: the seeder only ever ADDS keys the file does not + already mention, in any form, so a real secret is never rewritten. +- **Re-seeding wrote a second block for the same plugin.** The append was + unconditional, so a plugin seeded twice got two `# ─── Auth ───` headings, and + three after that. Every key was still present exactly once, so nothing broke — + the grouping the block exists to provide just quietly stopped being true. New + keys are now merged into the block the plugin already owns, keeping the blank + line that separates it from the next one. + +### Added +- **`hkm env` — audit and tidy a project's `.env`.** A dotenv file accumulates: + a plugin seeds its block on enable, someone appends a key at the bottom to try + something, a second plugin declares a variable the first one already did. None + of that is an error anywhere. The loader resolves a repeated key silently, the + boot succeeds, and the value in effect is whichever line happens to be last — + a file that works and does not say what it is doing. + - `hkm env` reports duplicates with every occurrence's line number and marks + which one is live. That marker is the point: `LoadEnvironment::setVar` + overwrites on each call and the cascade reads a file top to bottom, so the + LAST active assignment wins — the opposite of what most people assume when + they append a key to the bottom of a .env. + - `hkm env dedupe` asks per key rather than choosing. The right survivor is + not derivable: `DB_HOST=localhost` on line 12 and `DB_HOST=10.0.0.4` on line + 88 are both plausible, and the one in effect is as likely to be the accident + as the intent. `--keep=effective` is the scriptable form that cannot change + behaviour; `--keep=first` / `--keep=last` are positional. + - `hkm env group` reorders the file into blocks: a key a plugin declares in its + `module.json` `config[]` goes under that plugin, otherwise under the feature + its prefix names, otherwise `Ungrouped`. Comments attached to a key move with + it, comments attached to nothing are rescued into a `Notes` block rather than + dropped, and the pass refuses to write unless every key AND every + informational comment that went in comes out again. + - Every write leaves the previous file beside it as `.env.bak`, at 0600. +- **A project is found from anywhere inside it.** `resolveRoot` checked the exact + working directory, so `hkm env` in `/app` answered "'.' is neither a + project folder (with proj.json) nor a registered name" about a project one + directory up. It now walks up to the filesystem root, the way git, composer and + npm all find theirs — for every command that takes a `[path|name]`, not just + `env`. An EXPLICIT path stays exact: the same resolver backs + `hkm install --owner`, and a command that chowns a tree must never quietly + retarget itself above where it was pointed. + ## [1.12.1] - 2026-09-02 ### Fixed diff --git a/tools/src/commands/env.zig b/tools/src/commands/env.zig new file mode 100644 index 0000000..f98dc94 --- /dev/null +++ b/tools/src/commands/env.zig @@ -0,0 +1,646 @@ +//! `hkm env` — audit and tidy a project's `.env`. +//! +//! hkm env [path|name] what is in it: duplicates, groups, orphans +//! hkm env dedupe [path|name] resolve duplicate keys, one prompt each +//! hkm env group [path|name] reorder it into blocks, by plugin then feature +//! +//! ## Why this exists +//! +//! A `.env` accumulates. A plugin seeds its block on enable, someone appends a +//! key at the bottom to try something, a second plugin declares a variable the +//! first one already did — and none of it is an error anywhere. The loader +//! resolves a repeated key silently, the boot succeeds, and the value in effect +//! is whichever line happens to be last. That is the failure this command is +//! for: not a file that is broken, a file that works and does not say what it +//! is doing. +//! +//! Which is also why `dedupe` asks instead of picking. The right survivor is +//! not derivable — `DB_HOST=localhost` on line 12 and `DB_HOST=10.0.0.4` on +//! line 88 are both plausible, and the one currently in effect is as likely to +//! be the accident as the intent. The command's job is to show which is live +//! and let the person who knows decide. +//! +//! Nothing is written without a `.env.bak` beside it. + +const std = @import("std"); +const prompt = @import("../lib/prompt.zig"); +const util = @import("../lib/util.zig"); +const services = @import("../lib/services.zig"); +const envfile = @import("../lib/env_file.zig"); +const plugin_env = @import("../lib/plugin_env.zig"); + +const Dir = std.Io.Dir; +const Io = std.Io; +const EnvMap = std.process.Environ.Map; + +const Action = enum { audit, dedupe, group }; + +const Options = struct { + action: Action = .audit, + target: []const u8 = "", + dry_run: bool = false, + /// Non-interactive resolution: keep the first or the last occurrence. + keep: ?Keep = null, + help: bool = false, +}; + +const Keep = enum { first, last, effective }; + +fn parse(args: []const []const u8) Options { + var o = Options{}; + var i: usize = 2; + while (i < args.len) : (i += 1) { + const a = args[i]; + if (std.mem.eql(u8, a, "--help") or std.mem.eql(u8, a, "-h")) { + o.help = true; + } else if (std.mem.eql(u8, a, "--dry-run") or std.mem.eql(u8, a, "-n")) { + o.dry_run = true; + } else if (std.mem.eql(u8, a, "--keep=first")) { + o.keep = .first; + } else if (std.mem.eql(u8, a, "--keep=last")) { + o.keep = .last; + } else if (std.mem.eql(u8, a, "--keep=effective")) { + o.keep = .effective; + } else if (std.mem.startsWith(u8, a, "--")) { + continue; + } else if (i == 2 and isAction(a)) { + o.action = actionOf(a); + } else if (o.target.len == 0) { + o.target = a; + } + } + return o; +} + +fn isAction(a: []const u8) bool { + return std.mem.eql(u8, a, "audit") or std.mem.eql(u8, a, "analyse") or + std.mem.eql(u8, a, "analyze") or std.mem.eql(u8, a, "dedupe") or + std.mem.eql(u8, a, "dedup") or std.mem.eql(u8, a, "group"); +} + +fn actionOf(a: []const u8) Action { + if (std.mem.eql(u8, a, "dedupe") or std.mem.eql(u8, a, "dedup")) return .dedupe; + if (std.mem.eql(u8, a, "group")) return .group; + return .audit; +} + +fn printHelp() void { + prompt.intro("hkm env — audit and tidy a project's .env"); + prompt.section("Usage"); + prompt.item("hkm env [path|name]", "what is in it: duplicates, groups, keys no plugin declares"); + prompt.item("hkm env dedupe [path|name]", "resolve duplicate keys — one prompt per key"); + prompt.item("hkm env group [path|name]", "reorder into blocks, by declaring plugin then by feature"); + prompt.blank(); + prompt.section("Options"); + prompt.item("--dry-run, -n", "show the result without writing"); + prompt.item("--keep=effective", "dedupe without prompting: keep the line the loader actually uses"); + prompt.item("--keep=first", "dedupe without prompting: keep the topmost occurrence"); + prompt.item("--keep=last", "dedupe without prompting: keep the bottom occurrence"); + prompt.item("--help, -h", "show this help"); + prompt.blank(); + prompt.section("Notes"); + prompt.muted("The LAST active assignment wins at load time, not the first — so a key"); + prompt.muted("appended at the bottom silently overrides the one in its proper block."); + prompt.muted("Every write leaves the previous file beside it as .env.bak."); +} + +pub fn run(allocator: std.mem.Allocator, io: Io, env: *EnvMap, args: []const []const u8) !u8 { + const opts = parse(args); + if (opts.help) { + printHelp(); + return 0; + } + + const root = (try services.resolveRoot(allocator, io, env, opts.target)) orelse { + prompt.err(try std.fmt.allocPrint( + allocator, + "'{s}' is neither a project folder (with proj.json) nor a registered name.", + .{if (opts.target.len == 0) "." else opts.target}, + )); + return 1; + }; + + const f = try envfile.read(allocator, io, root); + if (f.content.len == 0) { + prompt.intro("hkm env"); + prompt.err(try std.fmt.allocPrint(allocator, "no .env at {s}", .{f.path})); + prompt.muted("create one with: hkm install"); + return 1; + } + + const file = try envfile.parse(allocator, f.content); + const claims = try readClaims(allocator, io, root); + + return switch (opts.action) { + .audit => try audit(allocator, io, f.path, file, claims), + .dedupe => try dedupe(allocator, io, f.path, f.content, file, opts), + .group => try group(allocator, io, f.path, f.content, file, claims, opts), + }; +} + +// ── which plugin declares which key ────────────────────────────────────────── + +const Claim = struct { key: []const u8, plugin: []const u8 }; + +/// Map every key declared in an installed plugin's `module.json` `config[]` to +/// that plugin. This is the authoritative half of the grouping: a key a plugin +/// declares belongs to that plugin, whatever its prefix happens to spell. +fn readClaims(allocator: std.mem.Allocator, io: Io, root: []const u8) ![]const Claim { + var out: std.ArrayList(Claim) = .empty; + + const plugins_dir = try std.fmt.allocPrint(allocator, "{s}/plugins", .{root}); + var dir = Dir.cwd().openDir(io, plugins_dir, .{ .iterate = true }) catch return out.items; + defer dir.close(io); + + var it = dir.iterate(); + while (it.next(io) catch null) |entry| { + // A plugin is a directory or a symlink into the store — both resolve. + if (entry.name.len == 0 or entry.name[0] == '.') continue; + // `entry.name` points into the iterator's own buffer and is overwritten + // by the next next() call — it has to be duped before it outlives this + // iteration, or the stored group name is whatever the next entry is. + const name = try allocator.dupe(u8, entry.name); + const vars = plugin_env.readVars(allocator, io, plugins_dir, name) catch continue; + for (vars) |v| try out.append(allocator, .{ .key = v.key, .plugin = name }); + } + + return out.items; +} + +fn claimOf(claims: []const Claim, key: []const u8) ?[]const u8 { + for (claims) |c| { + if (std.mem.eql(u8, c.key, key)) return c.plugin; + } + return null; +} + +/// The block a key belongs in: its declaring plugin, else its prefix's feature, +/// else Ungrouped. +fn groupOf(claims: []const Claim, key: []const u8) []const u8 { + if (claimOf(claims, key)) |p| return p; + return envfile.prefixGroup(key) orelse envfile.ungrouped; +} + +// ── audit ──────────────────────────────────────────────────────────────────── + +fn audit( + allocator: std.mem.Allocator, + io: Io, + path: []const u8, + file: envfile.File, + claims: []const Claim, +) !u8 { + _ = io; + prompt.intro("hkm env"); + prompt.muted(path); + + var active: usize = 0; + for (file.records) |r| { + if (r.active) active += 1; + } + prompt.blank(); + prompt.item("keys", try std.fmt.allocPrint( + allocator, + "{d} ({d} set, {d} commented)", + .{ file.records.len, active, file.records.len - active }, + )); + + const dups = try envfile.duplicates(allocator, file); + + // ── duplicates ── + prompt.blank(); + prompt.section("Duplicates"); + if (dups.len == 0) { + prompt.ok("no key appears twice"); + } else { + for (dups) |d| { + const live = envfile.effective(file, d); + prompt.warn(try std.fmt.allocPrint(allocator, "{s} — {d} occurrences", .{ d.key, d.at.len })); + for (d.at, 0..) |rec, i| { + const r = file.records[rec]; + prompt.muted(try std.fmt.allocPrint( + allocator, + " line {d:>4} {s}{s}={s}{s}", + .{ + r.line + 1, + if (r.active) "" else "# ", + r.key, + elide(r.value), + if (live != null and live.? == i) " ← in effect" else "", + }, + )); + } + } + prompt.blank(); + prompt.muted("resolve them with: hkm env dedupe"); + } + + // ── groups ── + prompt.blank(); + prompt.section("Groups"); + const names = try groupNames(allocator, file, claims); + for (names) |g| { + var n: usize = 0; + for (file.records) |r| { + if (std.mem.eql(u8, groupOf(claims, r.key), g)) n += 1; + } + prompt.item(g, try std.fmt.allocPrint(allocator, "{d} key(s)", .{n})); + } + prompt.blank(); + prompt.muted("reorder the file into these blocks with: hkm env group"); + + prompt.outro(if (dups.len == 0) "no duplicates" else "duplicates found"); + return if (dups.len == 0) 0 else 1; +} + +/// Shorten a value for display. A .env is full of secrets; an audit that prints +/// a 400-character key into a terminal — and a scrollback, and a screen share — +/// has widened the blast radius of the thing it was asked to tidy. +fn elide(value: []const u8) []const u8 { + if (value.len <= 24) return value; + return value[0..24]; +} + +/// Every group present, plugins first (alphabetically), then features, with +/// Ungrouped last so the keys nothing claims are where you look for them. +fn groupNames(allocator: std.mem.Allocator, file: envfile.File, claims: []const Claim) ![]const []const u8 { + var out: std.ArrayList([]const u8) = .empty; + for (file.records) |r| { + const g = groupOf(claims, r.key); + var seen = false; + for (out.items) |o| { + if (std.mem.eql(u8, o, g)) { + seen = true; + break; + } + } + if (!seen) try out.append(allocator, g); + } + + // Plugin-declared groups sort before heuristic ones; Ungrouped goes last. + const rank = struct { + fn of(claims_: []const Claim, name: []const u8) u8 { + if (std.mem.eql(u8, name, envfile.ungrouped)) return 2; + for (claims_) |c| { + if (std.mem.eql(u8, c.plugin, name)) return 0; + } + return 1; + } + }; + + const Ctx = struct { claims: []const Claim }; + std.mem.sort([]const u8, out.items, Ctx{ .claims = claims }, struct { + fn lt(ctx: Ctx, a: []const u8, b: []const u8) bool { + const ra = rank.of(ctx.claims, a); + const rb = rank.of(ctx.claims, b); + if (ra != rb) return ra < rb; + return std.mem.order(u8, a, b) == .lt; + } + }.lt); + + return out.items; +} + +// ── dedupe ─────────────────────────────────────────────────────────────────── + +fn dedupe( + allocator: std.mem.Allocator, + io: Io, + path: []const u8, + before: []const u8, + file: envfile.File, + opts: Options, +) !u8 { + prompt.intro("hkm env dedupe"); + prompt.muted(path); + + const dups = try envfile.duplicates(allocator, file); + if (dups.len == 0) { + prompt.ok("no duplicate keys — nothing to do"); + return 0; + } + + var drop: std.ArrayList(usize) = .empty; + var resolved: usize = 0; + + for (dups) |d| { + const live = envfile.effective(file, d); + + const choice = if (opts.keep) |k| autoChoice(file, d, live, k) else blk: { + prompt.blank(); + var items: std.ArrayList([]const u8) = .empty; + for (d.at, 0..) |rec, i| { + const r = file.records[rec]; + try items.append(allocator, try std.fmt.allocPrint( + allocator, + "line {d:>4} {s}{s}={s}{s}", + .{ + r.line + 1, + if (r.active) "" else "# ", + r.key, + elide(r.value), + if (live != null and live.? == i) " (in effect now)" else "", + }, + )); + } + try items.append(allocator, "leave this key alone"); + + const label = try std.fmt.allocPrint( + allocator, + "{s} appears {d} times — which line should remain?", + .{ d.key, d.at.len }, + ); + break :blk prompt.select(label, items.items) orelse items.items.len - 1; + }; + + // The extra trailing option, or a cancelled prompt: change nothing. + if (choice >= d.at.len) continue; + + resolved += 1; + for (d.at, 0..) |rec, i| { + if (i == choice) continue; + try drop.append(allocator, file.records[rec].line); + } + } + + if (drop.items.len == 0) { + prompt.blank(); + prompt.muted("nothing selected — file unchanged"); + return 0; + } + + const after = try envfile.withoutLines(allocator, file, drop.items); + + prompt.blank(); + prompt.ok(try std.fmt.allocPrint( + allocator, + "{d} key(s) resolved, {d} line(s) removed", + .{ resolved, drop.items.len }, + )); + + if (opts.dry_run) { + prompt.muted("dry run — nothing written"); + return 0; + } + + try envfile.write(allocator, io, path, before, after); + prompt.ok(try std.fmt.allocPrint(allocator, "written — previous file kept at {s}.bak", .{path})); + return 0; +} + +fn autoChoice(file: envfile.File, d: envfile.Duplicate, live: ?usize, keep: Keep) usize { + return switch (keep) { + .first => 0, + .last => d.at.len - 1, + // Preserving the value the application is running on right now is the + // only automatic answer that cannot change behaviour. With nothing + // active there is nothing in effect to preserve, so keep the last. + .effective => live orelse blk: { + _ = file; + break :blk d.at.len - 1; + }, + }; +} + +// ── group ──────────────────────────────────────────────────────────────────── + +fn group( + allocator: std.mem.Allocator, + io: Io, + path: []const u8, + before: []const u8, + file: envfile.File, + claims: []const Claim, + opts: Options, +) !u8 { + prompt.intro("hkm env group"); + prompt.muted(path); + + const dups = try envfile.duplicates(allocator, file); + if (dups.len > 0) { + // Reordering a file with duplicates would move the losing copies next + // to the winner, where they look deliberate. Worse, "last wins" is + // positional, so the reorder can change WHICH ONE the loader picks — + // a rewrite that silently alters the running configuration. + prompt.err(try std.fmt.allocPrint( + allocator, + "{d} duplicate key(s) — resolve them before grouping.", + .{dups.len}, + )); + prompt.muted("grouping moves lines, and the last assignment is the one that wins,"); + prompt.muted("so reordering a duplicated key can change which value is in effect."); + prompt.muted("run: hkm env dedupe"); + return 1; + } + + const names = try groupNames(allocator, file, claims); + + var out: std.ArrayList(u8) = .empty; + + // Every line this rewrite has placed somewhere. What is left over at the + // end is what would otherwise be silently dropped — see the rescue pass. + const used = try allocator.alloc(bool, file.lines.len); + @memset(used, false); + + // Preamble — whatever a person put at the top of the file, kept verbatim. + for (file.lines[0..file.preamble], 0..) |line, i| { + try out.appendSlice(allocator, line); + try out.append(allocator, '\n'); + used[i] = true; + } + trimTrailingBlanks(&out); + + for (names) |g| { + if (out.items.len > 0) try out.appendSlice(allocator, "\n\n"); + try out.appendSlice(allocator, try std.fmt.allocPrint( + allocator, + "# ─── {s} ───────────────────────────────────────────────\n", + .{g}, + )); + + for (file.records) |r| { + if (!std.mem.eql(u8, groupOf(claims, r.key), g)) continue; + + // Carry the record's attached explanation with it. Only two kinds + // of line are dropped: a wordless rule, and a banner whose label is + // a group this pass is re-emitting anyway. A labelled rule that + // says something — `# --- s3 driver (MinIO) ---` — is information, + // and survives. + var i = r.first; + while (i < r.line) : (i += 1) { + used[i] = true; + if (envfile.isRule(file.lines[i])) continue; + if (envfile.headerLabel(file.lines[i])) |label| { + if (isGroupName(names, label)) continue; + } + try out.appendSlice(allocator, file.lines[i]); + try out.append(allocator, '\n'); + } + try out.appendSlice(allocator, file.lines[r.line]); + try out.append(allocator, '\n'); + used[r.line] = true; + } + } + + // Rescue pass. A comment block separated from every key by a blank line — + // or trailing after the last one — belongs to no record and would simply + // cease to exist. These are routinely the most important lines in the file + // ("NEVER commit actual values for these"), so they are kept verbatim, in + // order, under a heading that says why they are no longer where they were. + var orphans: std.ArrayList([]const u8) = .empty; + for (file.lines, 0..) |line, i| { + if (used[i]) continue; + const t = std.mem.trim(u8, line, " \t\r"); + if (t.len == 0 or envfile.isRule(line)) continue; + // This pass's OWN heading from a previous run. Without this the Notes + // block orphans itself and grows by two lines every time the command + // is run — which is the difference between a tidy-up you can run twice + // and one you can run once. + if (envfile.headerLabel(line)) |label| { + if (std.mem.eql(u8, label, notes_label)) continue; + } + if (std.mem.eql(u8, t, notes_note)) continue; + try orphans.append(allocator, line); + } + + if (orphans.items.len > 0) { + try out.appendSlice(allocator, "\n\n"); + try out.appendSlice(allocator, "# ─── " ++ notes_label ++ " ───────────────────────────────────────────────\n"); + try out.appendSlice(allocator, notes_note ++ "\n"); + for (orphans.items) |line| { + try out.appendSlice(allocator, line); + try out.append(allocator, '\n'); + } + } + + trimTrailingBlanks(&out); + try out.append(allocator, '\n'); + + prompt.blank(); + for (names) |g| { + var n: usize = 0; + for (file.records) |r| { + if (std.mem.eql(u8, groupOf(claims, r.key), g)) n += 1; + } + prompt.item(g, try std.fmt.allocPrint(allocator, "{d} key(s)", .{n})); + } + + // Everything that went in must come out. A reorder that drops a key takes a + // secret with it, and one that drops a comment takes the only explanation + // of a setting — so both are counted rather than trusted. This check is + // what caught the rescue pass being necessary in the first place. + const check = try envfile.parse(allocator, out.items); + if (check.records.len != file.records.len) { + prompt.err(try std.fmt.allocPrint( + allocator, + "refusing to write: {d} keys in, {d} out.", + .{ file.records.len, check.records.len }, + )); + return 1; + } + + const before_notes = try envfile.informationalComments(allocator, file); + const after_notes = try envfile.informationalComments(allocator, check); + if (try lostComments(allocator, before_notes, after_notes, names)) |lost| { + prompt.err("refusing to write: the rewrite would drop comment lines."); + prompt.muted(lost); + return 1; + } + + prompt.blank(); + if (opts.dry_run) { + prompt.muted("dry run — nothing written"); + return 0; + } + + try envfile.write(allocator, io, path, before, out.items); + prompt.ok(try std.fmt.allocPrint( + allocator, + "{d} keys regrouped into {d} block(s) — previous file kept at {s}.bak", + .{ file.records.len, names.len, path }, + )); + return 0; +} + +/// Heading this pass writes over the comments that belong to no single key. +const notes_label = "Notes"; +const notes_note = "# Comments that were not attached to any single key."; + +fn isGroupName(names: []const []const u8, label: []const u8) bool { + for (names) |n| { + if (std.mem.eql(u8, n, label)) return true; + } + return false; +} + +/// The first informational comment present before the rewrite and absent after, +/// or null when none was lost. A banner this pass re-emits is not a loss. +fn lostComments( + allocator: std.mem.Allocator, + before: []const []const u8, + after: []const []const u8, + names: []const []const u8, +) !?[]const u8 { + for (before) |b| { + if (envfile.headerLabel(b)) |label| { + if (isGroupName(names, label)) continue; + } + var found = false; + for (after) |a| { + if (std.mem.eql(u8, a, b)) { + found = true; + break; + } + } + if (!found) return try std.fmt.allocPrint(allocator, " first missing: {s}", .{b}); + } + return null; +} + +fn trimTrailingBlanks(out: *std.ArrayList(u8)) void { + while (out.items.len > 0 and (out.items[out.items.len - 1] == '\n' or out.items[out.items.len - 1] == '\r')) { + _ = out.pop(); + } +} + +// ── tests ──────────────────────────────────────────────────────────────────── + +test "the action word is optional and the target survives it" { + try std.testing.expectEqual(Action.audit, parse(&.{ "hkm", "env" }).action); + try std.testing.expectEqual(Action.dedupe, parse(&.{ "hkm", "env", "dedupe" }).action); + try std.testing.expectEqual(Action.group, parse(&.{ "hkm", "env", "group", "shop" }).action); + try std.testing.expectEqualStrings("shop", parse(&.{ "hkm", "env", "group", "shop" }).target); + // No action word — the bare argument is the project, not a typo'd verb. + try std.testing.expectEqualStrings("shop", parse(&.{ "hkm", "env", "shop" }).target); + try std.testing.expectEqual(Action.audit, parse(&.{ "hkm", "env", "shop" }).action); +} + +test "keep flags parse" { + try std.testing.expectEqual(Keep.effective, parse(&.{ "hkm", "env", "dedupe", "--keep=effective" }).keep.?); + try std.testing.expectEqual(Keep.first, parse(&.{ "hkm", "env", "dedupe", "--keep=first" }).keep.?); + try std.testing.expect(parse(&.{ "hkm", "env", "dedupe" }).keep == null); + try std.testing.expect(parse(&.{ "hkm", "env", "-n" }).dry_run); +} + +test "a plugin's claim beats the prefix table" { + const claims = [_]Claim{.{ .key = "DB_HOST", .plugin = "Tenancy" }}; + // The prefix table would say Database; the plugin that declares it wins. + try std.testing.expectEqualStrings("Tenancy", groupOf(&claims, "DB_HOST")); + try std.testing.expectEqualStrings("Database", groupOf(&claims, "DB_PORT")); + try std.testing.expectEqualStrings(envfile.ungrouped, groupOf(&claims, "STRIPE_KEY")); +} + +test "autoChoice keeps what is running when asked for the effective line" { + const a = std.testing.allocator; + var arena = std.heap.ArenaAllocator.init(a); + defer arena.deinit(); + const al = arena.allocator(); + + const f = try envfile.parse(al, "A=1\nA=2\n# A=3\n"); + const d = (try envfile.duplicates(al, f))[0]; + const live = envfile.effective(f, d); + + try std.testing.expectEqual(@as(usize, 1), autoChoice(f, d, live, .effective)); // A=2 + try std.testing.expectEqual(@as(usize, 0), autoChoice(f, d, live, .first)); + try std.testing.expectEqual(@as(usize, 2), autoChoice(f, d, live, .last)); +} diff --git a/tools/src/commands/install.zig b/tools/src/commands/install.zig index 4bdc637..cbb9fe3 100644 --- a/tools/src/commands/install.zig +++ b/tools/src/commands/install.zig @@ -17,7 +17,8 @@ //! 6. fetch every plugin the project's own bootstrap wires (mirrors what //! `hkm new` does right after scaffolding — see lib/plugin_provision.zig) //! 7. with --production / --owner: chown and chmod the WHOLE project for the -//! web server's account — last, because steps 5 and 6 create vendor/ and +//! web server's account, plus the plugin-store versions its plugins/ +//! symlinks point at — last, because steps 5 and 6 create vendor/ and //! plugins/ as whoever ran the command //! //! Every step besides directory creation can be skipped with a --no-* flag, for @@ -36,6 +37,8 @@ const services = @import("../lib/services.zig"); const plugin_assets = @import("../lib/plugin_assets.zig"); const plugin_provision = @import("../lib/plugin_provision.zig"); const plugins_cmd = @import("plugins.zig"); +const installer = @import("../lib/plugin_install.zig"); +const pstore = @import("../lib/plugin_store.zig"); const Dir = std.Io.Dir; const Io = std.Io; @@ -74,7 +77,8 @@ const Options = struct { /// group-and-world-readable dev modes (0775/0664). production: bool = false, /// --owner=[:] (also HKM_PROD_OWNER) — chown the whole - /// project to this user[:group], typically `deploy:www-data`: the deploy + /// project, AND the plugin-store versions its plugins/ symlinks resolve to, + /// to this user[:group], typically `deploy:www-data`: the deploy /// account keeps the code, the web server / PHP-FPM pool reaches it through /// the group. Passed straight to the system `chown`, so `user`, /// `user:group` and `:group` (group-only) all work. Requires root/sudo @@ -141,7 +145,7 @@ fn printHelp() void { prompt.item("--no-chmod", "skip fixing var/ and userdata/ mode bits"); prompt.item("--verify-plugins", "run each plugin's own test suite while installing (slow)"); prompt.item("--production, --prod", "harden the WHOLE tree: code 0750/0640, var+userdata 2770/0660"); - prompt.item("--owner=[:]", "chown the whole project to this user[:group] (needs root/sudo)"); + prompt.item("--owner=[:]", "chown the project AND its linked plugin store entries to this user[:group] (needs root/sudo)"); prompt.item("--help, -h", "show this help"); prompt.blank(); prompt.section("Environment"); @@ -468,7 +472,10 @@ fn hardenProject( prompt.note(""); if (owner) |o| { - if (o.len > 0) fixOwnership(allocator, io, env, root, o); + if (o.len > 0) { + fixOwnership(allocator, io, env, root, o); + fixPluginStoreOwnership(allocator, io, env, root, o); + } } else { prompt.warn("--production: no --owner given (and HKM_PROD_OWNER is unset) — ownership left unchanged."); prompt.muted("pass --owner=[:] — typically your web server's account, e.g. deploy:www-data."); @@ -483,7 +490,7 @@ fn hardenProject( ) catch "Permissions applied"); verifyModes(allocator, io, root, m); - reportTraversal(allocator, io, root); + reportTraversal(allocator, io, env, root); } /// Re-stat the paths that decide whether the application boots, and say so when @@ -657,6 +664,114 @@ fn fixOwnership(allocator: std.mem.Allocator, io: Io, env: *EnvMap, root: []cons } } +/// chown the plugin-store entries this project's `plugins/*` symlinks point at. +/// +/// A project's plugins are not IN the project. `hkm plugins install` keeps one +/// copy per (plugin, version, origin) in the global store and links the project +/// at it (lib/plugin_store.zig), so `plugins/Logger` is a symlink out of the +/// tree. Both halves of the hardening pass stop at that boundary by design: +/// `hardenTree` skips symlinks because a chmod would follow one and rewrite a +/// target outside the project, and `fixOwnership` only walks the project root. +/// +/// The result, before this pass, was a project that verified clean and could +/// not serve: every file the pool had to READ FIRST — every Provider, every +/// controller a route resolves to — was still owned by whoever ran the command, +/// and the report said "Project owned by deploy:www-data". +/// +/// Only the versions THIS project links to are touched. The store is shared by +/// every project on the machine, and taking ownership of all of it on behalf of +/// one project's web account is not this command's call. +fn fixPluginStoreOwnership( + allocator: std.mem.Allocator, + io: Io, + env: *EnvMap, + root: []const u8, + owner: []const u8, +) void { + const plugins_dir = std.fmt.allocPrint(allocator, "{s}/plugins", .{root}) catch return; + var dir = Dir.cwd().openDir(io, plugins_dir, .{ .iterate = true }) catch return; + defer dir.close(io); + + // The resolved store, used ONLY to bound how far up a target we may walk. + // A link pointing somewhere else entirely — a working copy someone is + // editing — gets its own tree chown'd and nothing above it. + const store: ?[]const u8 = blk: { + const fallback = fb: { + const p = installer.pluginsRoot(allocator, io, env, root) catch break :fb root; + break :fb util.parentOf(p) orelse root; + }; + break :blk pstore.root(allocator, env, fallback) catch null; + }; + + var done: std.ArrayList([]const u8) = .empty; + var linked: usize = 0; + var failed: usize = 0; + + var it = dir.iterate(); + while (it.next(io) catch null) |entry| { + const link = std.fmt.allocPrint(allocator, "{s}/{s}", .{ plugins_dir, entry.name }) catch continue; + // A real directory is inside the project — fixOwnership already had it. + if (!util.isSymlink(io, link)) continue; + const target = util.linkTarget(allocator, io, link) orelse continue; + // Relative targets stay inside the project, absolute ones are the store. + if (target.len == 0 or target[0] != '/') continue; + // A dangling link has nothing to chown; `hkm plugins verify` reports it. + if (!util.dirExists(Dir.cwd(), io, target)) continue; + linked += 1; + + if (!chownOnce(allocator, io, env, &done, target, owner, true)) failed += 1; + + // Everything between the store root and the version directory has to be + // traversable by the new owner too, or the tree just chown'd cannot be + // reached. Walk up only INSIDE the store, never above it: the store's + // own parents are a user's cache or home, and chowning those to a web + // account on behalf of one project would be a machine-wide surprise. + const s_root = store orelse continue; + if (!util.isInside(target, s_root)) continue; + var cursor: ?[]const u8 = util.parentOf(target); + while (cursor) |dir_path| : (cursor = util.parentOf(dir_path)) { + if (!util.isInside(dir_path, s_root)) break; + if (!chownOnce(allocator, io, env, &done, dir_path, owner, false)) failed += 1; + if (std.mem.eql(u8, util.trimSlash(dir_path), util.trimSlash(s_root))) break; + } + } + + if (linked == 0) return; + + if (failed == 0) { + prompt.ok(std.fmt.allocPrint( + allocator, + "{d} linked plugin store entr{s} owned by {s}", + .{ linked, if (linked == 1) @as([]const u8, "y") else "ies", owner }, + ) catch "Plugin store ownership fixed"); + } else { + prompt.warn(std.fmt.allocPrint( + allocator, + "chown {s} failed on {d} plugin store path(s) — the pool cannot read those plugins.", + .{ owner, failed }, + ) catch "chown failed on the plugin store — the pool cannot read those plugins."); + } +} + +/// chown `path`, remembering it so a path reached through several links — the +/// store root, a plugin directory holding two pinned versions — is chown'd once +/// rather than once per link. +fn chownOnce( + allocator: std.mem.Allocator, + io: Io, + env: *EnvMap, + done: *std.ArrayList([]const u8), + path: []const u8, + owner: []const u8, + recursive: bool, +) bool { + for (done.items) |p| { + if (std.mem.eql(u8, p, path)) return true; + } + done.append(allocator, path) catch {}; + return chownPath(io, env, path, owner, recursive); +} + /// `chown [-R] `. Shells out rather than resolving the user/group /// name to a uid/gid natively — the OS's own NSS already knows how to do that /// correctly (files, LDAP, whatever `/etc/nsswitch.conf` says), and `chown` @@ -700,27 +815,96 @@ fn chownPath(io: Io, env: *EnvMap, path: []const u8, owner: []const u8, recursiv /// /// Reported, never changed: widening a directory that is not part of the /// project is the operator's call, not this command's. -fn reportTraversal(allocator: std.mem.Allocator, io: Io, root: []const u8) void { +fn reportTraversal(allocator: std.mem.Allocator, io: Io, env: *EnvMap, root: []const u8) void { var blocked: std.ArrayList([]const u8) = .empty; + collectBlocked(allocator, io, util.parentOf(root), &blocked); + + if (blocked.items.len > 0) { + prompt.warn("The web server may not be able to REACH the project — these parent directories deny traversal to others:"); + for (blocked.items) |item| prompt.muted(std.fmt.allocPrint(allocator, " {s}", .{item}) catch item); + prompt.muted("Each one needs execute for the pool's account: `chmod o+x `, add the account to its group, or"); + prompt.muted("move the project somewhere the web server already reaches (/var/www, /srv)."); + } - var cursor: ?[]const u8 = util.parentOf(root); + reportStoreTraversal(allocator, io, env, root); +} + +/// The same check for the PLUGIN STORE, which the project reaches by symlink. +/// +/// Worth its own pass and its own advice: the store defaults to `$HOME/.cache` +/// (lib/plugin_store.zig), and a deploy run under sudo resolves that to +/// `/root/.cache` — a directory that is 0700 on every mainstream distro. The +/// chown above then succeeds on every entry and the site still cannot read one +/// of them, because the denial is a level above anything this command owns. +/// +/// The remedy differs too. Widening a home directory to reach a cache is the +/// wrong trade; the store is relocatable precisely so it does not have to be. +fn reportStoreTraversal(allocator: std.mem.Allocator, io: Io, env: *EnvMap, root: []const u8) void { + const fallback = fb: { + const p = installer.pluginsRoot(allocator, io, env, root) catch break :fb root; + break :fb util.parentOf(p) orelse root; + }; + const store = pstore.root(allocator, env, fallback) catch return; + // Nothing installed from the store — no reason to talk about it. + if (!util.dirExists(Dir.cwd(), io, store)) return; + // A store INSIDE the project is covered by the project's own walk above. + if (util.isInside(store, root)) return; + // Say nothing about a store this project does not actually reach into — + // the warning below asserts that its plugins/ links point there. + if (!linksIntoStore(allocator, io, root, store)) return; + + var blocked: std.ArrayList([]const u8) = .empty; + collectBlocked(allocator, io, store, &blocked); + if (blocked.items.len == 0) return; + + prompt.warn("The web server cannot REACH the plugin store — the project's plugins/ symlinks point into it:"); + for (blocked.items) |item| prompt.muted(std.fmt.allocPrint(allocator, " {s}", .{item}) catch item); + prompt.muted(std.fmt.allocPrint( + allocator, + " store: {s}", + .{store}, + ) catch ""); + prompt.muted("Move it somewhere the pool already reaches rather than widening a home directory:"); + prompt.muted(" hkm plugins store --set=/var/lib/hkm/plugin-store (or: export HKM_PLUGIN_STORE=…)"); + prompt.muted("then re-point this project's links with: hkm plugins lock"); +} + +/// True when at least one `plugins/*` entry is a symlink resolving into +/// `store`. Cheap enough to run unconditionally: a project has a handful of +/// plugins, and this reads only the link targets, never the trees behind them. +fn linksIntoStore(allocator: std.mem.Allocator, io: Io, root: []const u8, store: []const u8) bool { + const plugins_dir = std.fmt.allocPrint(allocator, "{s}/plugins", .{root}) catch return false; + var dir = Dir.cwd().openDir(io, plugins_dir, .{ .iterate = true }) catch return false; + defer dir.close(io); + + var it = dir.iterate(); + while (it.next(io) catch null) |entry| { + const link = std.fmt.allocPrint(allocator, "{s}/{s}", .{ plugins_dir, entry.name }) catch continue; + if (!util.isSymlink(io, link)) continue; + const target = util.linkTarget(allocator, io, link) orelse continue; + if (util.isInside(target, store)) return true; + } + return false; +} + +/// Walk from `start` up to `/`, collecting every directory that denies +/// traversal to "other" — reachable only by its owner or a member of its +/// group, which a web server account rarely is. +fn collectBlocked( + allocator: std.mem.Allocator, + io: Io, + start: ?[]const u8, + blocked: *std.ArrayList([]const u8), +) void { + var cursor: ?[]const u8 = start; while (cursor) |path| : (cursor = util.parentOf(path)) { if (util.statMode(io, path)) |mode| { - // No execute for "other" — reachable only by the owner or a member - // of the directory's group, which a web server account rarely is. if ((mode & 0o001) == 0) { blocked.append(allocator, std.fmt.allocPrint(allocator, "{s} ({o})", .{ path, mode }) catch path) catch {}; } } if (std.mem.eql(u8, path, "/")) break; } - - if (blocked.items.len == 0) return; - - prompt.warn("The web server may not be able to REACH the project — these parent directories deny traversal to others:"); - for (blocked.items) |item| prompt.muted(std.fmt.allocPrint(allocator, " {s}", .{item}) catch item); - prompt.muted("Each one needs execute for the pool's account: `chmod o+x `, add the account to its group, or"); - prompt.muted("move the project somewhere the web server already reaches (/var/www, /srv)."); } // -------------------------------------------------------------------------- diff --git a/tools/src/commands/plugins.zig b/tools/src/commands/plugins.zig index 28dc5a1..41c6509 100644 --- a/tools/src/commands/plugins.zig +++ b/tools/src/commands/plugins.zig @@ -851,7 +851,14 @@ fn enableWithDeps( if (steps.items.len == 0) { prompt.warn(try std.fmt.allocPrint(allocator, "{s} and all its dependencies are already enabled.", .{folder})); if (missing.items.len > 0) noteMissingDomains(allocator, missing.items); - prompt.outro("No changes made"); + + // Wiring is unchanged, but the plugin's config[] may not be. A plugin + // that gains a variable in a later version has an .env block that is + // now incomplete, and the boot fails on the missing key with nothing + // pointing at the cause. Re-seeding here is safe by construction: seed() + // only ever ADDS keys the file does not already mention, in any form. + const added = reseedEnv(allocator, io, root, folder, if (located) |l| l.dir else null, dry_run); + prompt.outro(if (added > 0) "Env block updated" else "No changes made"); return 0; } @@ -974,6 +981,45 @@ pub fn supportHelpersExpr(allocator: std.mem.Allocator, io: Io, env: *EnvMap, ro /// Enable ONE plugin into `source`, returning the updated text (no file write). /// Publishes assets + runs migrations as a side effect (skipped on dry-run). +/// Top up a plugin's `.env` block with variables its module.json declares and +/// the file does not have yet, merging into the block it already owns. +/// +/// Returns how many were added. Every failure is a warning rather than an error: +/// this runs on a command whose job was already done, and a .env that could not +/// be written is not a reason to report the enable itself as failed. +fn reseedEnv( + allocator: std.mem.Allocator, + io: Io, + root: []const u8, + folder: []const u8, + dir: ?[]const u8, + dry_run: bool, +) usize { + const d = dir orelse return 0; + const vars = penv.readVars(allocator, io, d, folder) catch return 0; + if (vars.len == 0) return 0; + + const seeded = penv.seed(allocator, io, root, folder, vars, dry_run) catch |e| { + prompt.warn(std.fmt.allocPrint( + allocator, + "could not update .env ({t}) — add {s}'s new config[] variables by hand.", + .{ e, folder }, + ) catch folder); + return 0; + }; + if (seeded.added.len == 0) return 0; + + prompt.ok(std.fmt.allocPrint(allocator, "{s} {d} new env var(s) to {s}'s block:", .{ + if (dry_run) "would add" else "Added", + seeded.added.len, + folder, + }) catch "Added new env vars"); + for (seeded.added) |v| { + prompt.muted(std.fmt.allocPrint(allocator, " {s}", .{v.key}) catch v.key); + } + return seeded.added.len; +} + fn enableOne( allocator: std.mem.Allocator, io: Io, diff --git a/tools/src/lib/env_file.zig b/tools/src/lib/env_file.zig new file mode 100644 index 0000000..b9b263b --- /dev/null +++ b/tools/src/lib/env_file.zig @@ -0,0 +1,453 @@ +//! Read a project's `.env` as records rather than lines, so it can be audited +//! and rewritten without losing what a person wrote in it. +//! +//! ## Why a record, not a line +//! +//! A dotenv file is not a key/value store on disk — it is a document. The +//! comment above a key explains it, the blank line below it separates a +//! section, and a key that appears twice is a bug that no parser reports +//! because the loader silently resolves it. Anything that rewrites the file has +//! to preserve the first two while surfacing the third. +//! +//! So a `Record` is an assignment plus the contiguous comment block directly +//! above it, and everything before the first record is a preamble that stays +//! put. Moving a record moves its explanation with it. +//! +//! ## Which duplicate is in effect +//! +//! LoadEnvironment::setVar overwrites `$_ENV[$name]` on every call and the +//! cascade walks a file top to bottom, so within one file the LAST active +//! assignment wins. That is the opposite of what most people assume when they +//! append a key to the bottom of a .env "to try something", and it is why +//! `effective()` exists: an audit that cannot say which line is actually live +//! is not an audit. +//! +//! A commented assignment (`# KEY=`) is parsed as a record too. The seeder +//! writes optional variables that way, so treating them as prose would make +//! every optional plugin knob invisible to the grouping and re-seed it forever. + +const std = @import("std"); +const util = @import("util.zig"); + +const Io = std.Io; +const Dir = std.Io.Dir; + +/// One `KEY=value` assignment, with the comment block attached above it. +pub const Record = struct { + key: []const u8, + /// Everything right of the first `=`, untrimmed of trailing comments. + value: []const u8, + /// False when the line is commented out (`# KEY=…`). + active: bool, + /// Index into `File.lines` of the assignment itself. + line: usize, + /// First line of the attached comment block — equals `line` when none. + first: usize, +}; + +pub const File = struct { + /// Every line of the file, in order, without terminators. + lines: []const []const u8, + records: []const Record, + /// Lines before the first record: the file's banner. Never reordered. + preamble: usize, + /// True when the file ended with a newline, so a rewrite can match it. + trailing_newline: bool, +}; + +/// A key that appears more than once, with every place it appears. +pub const Duplicate = struct { + key: []const u8, + /// Indices into `File.records`, in file order. + at: []const usize, +}; + +/// True when `name` is a syntactically valid environment key. +fn isKeyChar(c: u8, first: bool) bool { + if (c == '_') return true; + if (c >= 'A' and c <= 'Z') return true; + if (c >= 'a' and c <= 'z') return true; + if (!first and c >= '0' and c <= '9') return true; + return false; +} + +/// Split `line` into a key and the text right of `=`, or null when it is not an +/// assignment. Handles the commented form by reporting `active = false`. +pub fn assignment(line: []const u8) ?struct { key: []const u8, value: []const u8, active: bool } { + var s = std.mem.trim(u8, line, " \t\r"); + if (s.len == 0) return null; + + var active = true; + if (s[0] == '#') { + active = false; + // Step past the marker and any run of them: `## KEY=` is still a + // commented assignment, and a person writing one means it. + while (s.len > 0 and (s[0] == '#' or s[0] == ' ' or s[0] == '\t')) s = s[1..]; + if (s.len == 0) return null; + } + + // Optional `export ` prefix — valid dotenv, and dropping it silently would + // make `export DB_HOST=` invisible to a duplicate check that sees `DB_HOST=`. + if (std.mem.startsWith(u8, s, "export ")) s = std.mem.trimStart(u8, s["export ".len..], " \t"); + + const eq = std.mem.indexOfScalar(u8, s, '=') orelse return null; + const key = std.mem.trim(u8, s[0..eq], " \t"); + if (key.len == 0) return null; + + for (key, 0..) |c, i| { + if (!isKeyChar(c, i == 0)) return null; + } + + return .{ .key = key, .value = std.mem.trim(u8, s[eq + 1 ..], " \t"), .active = active }; +} + +const rule_chars = "-=_\u{2500}\u{2501}\u{2550}#*"; + +/// True for a comment carrying no words at all: a bare `#`, or a rule of +/// dashes / box characters. Always safe to drop and regenerate. +pub fn isRule(line: []const u8) bool { + var s = std.mem.trim(u8, line, " \t\r"); + if (s.len == 0 or s[0] != '#') return false; + s = std.mem.trim(u8, s[1..], " \t"); + if (s.len == 0) return true; + return std.mem.trim(u8, s, rule_chars).len == 0; +} + +/// The label of a `# ─── Name ───` header, or null when the line is not one. +/// +/// A LABELLED rule is not decoration — `# --- s3 driver (MinIO) ---` is the +/// only place that fact is written down, and an earlier version of this code +/// deleted three such lines from a real .env because they matched the shape of +/// a banner. So the label is returned rather than judged here, and the caller +/// drops the line only when the label is one it is about to re-emit itself. +pub fn headerLabel(line: []const u8) ?[]const u8 { + var s = std.mem.trim(u8, line, " \t\r"); + if (s.len == 0 or s[0] != '#') return null; + s = std.mem.trim(u8, s[1..], " \t"); + if (s.len == 0) return null; + + const head = std.mem.trimStart(u8, s, rule_chars); + if (head.len == s.len) return null; // no leading rule — ordinary prose + const label = std.mem.trim(u8, std.mem.trimEnd(u8, head, rule_chars), " \t"); + if (label.len == 0) return null; // pure rule — isRule's business + return label; +} + +/// Comment lines that carry information: not blank, not a bare rule. The unit +/// a rewrite must never lose. +pub fn informationalComments(allocator: std.mem.Allocator, file: File) ![]const []const u8 { + var out: std.ArrayList([]const u8) = .empty; + for (file.lines) |line| { + const t = std.mem.trim(u8, line, " \t\r"); + if (t.len == 0 or t[0] != '#') continue; + if (isRule(line)) continue; + if (assignment(line) != null) continue; // a commented-out key is a record + try out.append(allocator, t); + } + return out.items; +} + +/// Parse `content` into records. Never fails: a line that is not an assignment +/// is simply not a record, which is what makes this safe to run on any file. +pub fn parse(allocator: std.mem.Allocator, content: []const u8) !File { + var lines: std.ArrayList([]const u8) = .empty; + var it = std.mem.splitScalar(u8, content, '\n'); + while (it.next()) |l| try lines.append(allocator, std.mem.trimEnd(u8, l, "\r")); + + // splitScalar yields a trailing empty field for a file ending in a newline. + const trailing = lines.items.len > 0 and lines.items[lines.items.len - 1].len == 0; + if (trailing) _ = lines.pop(); + + var records: std.ArrayList(Record) = .empty; + var block: ?usize = null; + var preamble: usize = 0; + + for (lines.items, 0..) |line, i| { + const trimmed = std.mem.trim(u8, line, " \t\r"); + + if (assignment(line)) |a| { + try records.append(allocator, .{ + .key = a.key, + .value = a.value, + .active = a.active, + .line = i, + .first = block orelse i, + }); + if (records.items.len == 1) preamble = block orelse i; + block = null; + continue; + } + + if (trimmed.len == 0) { + // A blank line breaks the attachment: a comment separated from a + // key by whitespace is a section note, not that key's explanation. + block = null; + continue; + } + + if (trimmed[0] == '#') { + if (block == null) block = i; + continue; + } + + block = null; + } + + if (records.items.len == 0) preamble = lines.items.len; + + return .{ + .lines = lines.items, + .records = records.items, + .preamble = preamble, + .trailing_newline = trailing, + }; +} + +/// Keys appearing in more than one record, in first-appearance order. +pub fn duplicates(allocator: std.mem.Allocator, file: File) ![]const Duplicate { + var out: std.ArrayList(Duplicate) = .empty; + var seen: std.ArrayList([]const u8) = .empty; + + for (file.records, 0..) |r, i| { + var already = false; + for (seen.items) |k| { + if (std.mem.eql(u8, k, r.key)) { + already = true; + break; + } + } + if (already) continue; + try seen.append(allocator, r.key); + + var at: std.ArrayList(usize) = .empty; + try at.append(allocator, i); + for (file.records[i + 1 ..], i + 1..) |other, j| { + if (std.mem.eql(u8, other.key, r.key)) try at.append(allocator, j); + } + if (at.items.len > 1) try out.append(allocator, .{ .key = r.key, .at = at.items }); + } + + return out.items; +} + +/// Index into `dup.at` of the record actually in effect: the LAST active one. +/// Null when every occurrence is commented out — then nothing is in effect and +/// the key's value comes from the plugin's own default. +pub fn effective(file: File, dup: Duplicate) ?usize { + var found: ?usize = null; + for (dup.at, 0..) |rec, i| { + if (file.records[rec].active) found = i; + } + return found; +} + +/// Rebuild the file with the assignment lines at `drop` removed. +/// +/// ONLY the assignment lines. The comment block above a dropped key stays, on +/// purpose: it is routinely a section header that belongs to the whole block +/// below it, and deleting a `# ─── Database ───` because the first key under it +/// lost a duplicate vote would be a silent, unrelated edit. +pub fn withoutLines(allocator: std.mem.Allocator, file: File, drop: []const usize) ![]const u8 { + var out: std.ArrayList(u8) = .empty; + for (file.lines, 0..) |line, i| { + var skip = false; + for (drop) |d| { + if (d == i) { + skip = true; + break; + } + } + if (skip) continue; + try out.appendSlice(allocator, line); + try out.append(allocator, '\n'); + } + if (!file.trailing_newline and out.items.len > 0) _ = out.pop(); + return out.items; +} + +// ── grouping ───────────────────────────────────────────────────────────────── + +/// A key prefix and the feature it belongs to. Longest match wins, so the table +/// is ordered longest-first and `matchPrefix` does not have to sort it. +/// +/// This is the FALLBACK. A key a plugin declares in its module.json `config[]` +/// is grouped under that plugin instead — that mapping is authoritative, this +/// one is a guess about a key nothing claims. +pub const prefix_groups = [_]struct { prefix: []const u8, group: []const u8 }{ + .{ .prefix = "DATABASE_", .group = "Database" }, + .{ .prefix = "SESSION_", .group = "Session" }, + .{ .prefix = "STORAGE_", .group = "Storage" }, + .{ .prefix = "TENANCY_", .group = "Tenancy" }, + .{ .prefix = "TENANT_", .group = "Tenancy" }, + .{ .prefix = "SECURITY_", .group = "Security" }, + .{ .prefix = "COOKIE_", .group = "Cookie" }, + .{ .prefix = "LOGGER_", .group = "Logging" }, + .{ .prefix = "REDIS_", .group = "Redis" }, + .{ .prefix = "QUEUE_", .group = "Queue" }, + .{ .prefix = "CACHE_", .group = "Cache" }, + .{ .prefix = "ROUTE_", .group = "Routing" }, + .{ .prefix = "MAIL_", .group = "Mail" }, + .{ .prefix = "SMTP_", .group = "Mail" }, + .{ .prefix = "VIEW_", .group = "Views" }, + .{ .prefix = "EDGE_", .group = "Edge" }, + .{ .prefix = "AUTH_", .group = "Security" }, + .{ .prefix = "CSRF_", .group = "Security" }, + .{ .prefix = "CORS_", .group = "Security" }, + .{ .prefix = "JWT_", .group = "Security" }, + .{ .prefix = "LOG_", .group = "Logging" }, + .{ .prefix = "JOB_", .group = "Queue" }, + .{ .prefix = "SMS_", .group = "SMS" }, + .{ .prefix = "SEO_", .group = "SEO" }, + .{ .prefix = "AWS_", .group = "Storage" }, + .{ .prefix = "S3_", .group = "Storage" }, + .{ .prefix = "HKM_", .group = "Platform" }, + .{ .prefix = "APP_", .group = "Application" }, + .{ .prefix = "DB_", .group = "Database" }, +}; + +/// The group a key falls into when no plugin declares it. +pub fn prefixGroup(key: []const u8) ?[]const u8 { + var best: ?[]const u8 = null; + var best_len: usize = 0; + for (prefix_groups) |g| { + if (g.prefix.len <= best_len) continue; + if (std.mem.startsWith(u8, key, g.prefix)) { + best = g.group; + best_len = g.prefix.len; + } + } + return best; +} + +/// Name used for everything the plugins and the prefix table both disown. +pub const ungrouped = "Ungrouped"; + +/// Read `.env` from a project root. Returns "" when there is none, so callers +/// can report an empty analysis rather than an error. +pub fn read(allocator: std.mem.Allocator, io: Io, projectRoot: []const u8) !struct { path: []const u8, content: []const u8 } { + const path = try std.fs.path.join(allocator, &.{ projectRoot, ".env" }); + const content = Dir.cwd().readFileAlloc(io, path, allocator, .limited(8 * 1024 * 1024)) catch ""; + return .{ .path = path, .content = content }; +} + +/// Write `content` to the project's `.env`, keeping a `.env.bak` of what was +/// there. The backup is not politeness: this file holds the only copy of every +/// secret the application has, and a rewrite that loses one costs a great deal +/// more than the disk the copy takes. +pub fn write(allocator: std.mem.Allocator, io: Io, path: []const u8, before: []const u8, content: []const u8) !void { + const backup = try std.fmt.allocPrint(allocator, "{s}.bak", .{path}); + if (before.len > 0) { + try util.writeFileAtomic(io, backup, before); + util.chmod600(io, backup); + } + try util.writeFileAtomic(io, path, content); + util.chmod600(io, path); +} + +// ── tests ──────────────────────────────────────────────────────────────────── + +test "assignment parses active, commented and exported forms" { + try std.testing.expectEqualStrings("A", assignment("A=1").?.key); + try std.testing.expect(assignment("A=1").?.active); + try std.testing.expect(!assignment("# A=1").?.active); + try std.testing.expect(!assignment("## A=1").?.active); + try std.testing.expectEqualStrings("A", assignment("export A=1").?.key); + try std.testing.expectEqualStrings("1", assignment("A = 1").?.value); +} + +test "prose and rules are not assignments" { + try std.testing.expect(assignment("# see APP_KEY for details") == null); + try std.testing.expect(assignment("") == null); + try std.testing.expect(assignment("# ─── Auth ───") == null); + // A key with a dash is not a valid env name, so this is prose. + try std.testing.expect(assignment("not-a-key=1") == null); +} + +test "a comment directly above a key attaches, one across a blank line does not" { + const a = std.testing.allocator; + var arena = std.heap.ArenaAllocator.init(a); + defer arena.deinit(); + + const f = try parse(arena.allocator(), "# banner\n\n# explains A\nA=1\n\n# loose\n\nB=2\n"); + try std.testing.expectEqual(@as(usize, 2), f.records.len); + try std.testing.expectEqual(@as(usize, 2), f.records[0].first); // "# explains A" + try std.testing.expectEqual(@as(usize, 3), f.records[0].line); + try std.testing.expectEqual(f.records[1].line, f.records[1].first); // nothing attached + // Lines 0-1 ("# banner" + the blank) are the banner; line 2 belongs to A. + try std.testing.expectEqual(@as(usize, 2), f.preamble); +} + +test "the LAST active assignment is the one in effect" { + const a = std.testing.allocator; + var arena = std.heap.ArenaAllocator.init(a); + defer arena.deinit(); + const al = arena.allocator(); + + const f = try parse(al, "A=1\n# A=2\nA=3\nB=1\n"); + const dups = try duplicates(al, f); + try std.testing.expectEqual(@as(usize, 1), dups.len); + try std.testing.expectEqualStrings("A", dups[0].key); + try std.testing.expectEqual(@as(usize, 3), dups[0].at.len); + // Index 2 within `at` — the third occurrence, `A=3`. + try std.testing.expectEqual(@as(usize, 2), effective(f, dups[0]).?); +} + +test "a key whose every occurrence is commented has nothing in effect" { + const a = std.testing.allocator; + var arena = std.heap.ArenaAllocator.init(a); + defer arena.deinit(); + const al = arena.allocator(); + + const f = try parse(al, "# A=1\n# A=2\n"); + const dups = try duplicates(al, f); + try std.testing.expect(effective(f, dups[0]) == null); +} + +test "APP_KEY_ID and APP_KEY are different keys" { + const a = std.testing.allocator; + var arena = std.heap.ArenaAllocator.init(a); + defer arena.deinit(); + const al = arena.allocator(); + + const f = try parse(al, "APP_KEY=1\nAPP_KEY_ID=2\n"); + try std.testing.expectEqual(@as(usize, 0), (try duplicates(al, f)).len); +} + +test "withoutLines drops the assignment and keeps the header above it" { + const a = std.testing.allocator; + var arena = std.heap.ArenaAllocator.init(a); + defer arena.deinit(); + const al = arena.allocator(); + + const src = "# ─── Database ───\nDB_HOST=old\nDB_PORT=3306\nDB_HOST=new\n"; + const f = try parse(al, src); + const out = try withoutLines(al, f, &.{1}); + try std.testing.expectEqualStrings("# ─── Database ───\nDB_PORT=3306\nDB_HOST=new\n", out); +} + +test "prefixGroup takes the longest match" { + try std.testing.expectEqualStrings("Database", prefixGroup("DB_HOST").?); + try std.testing.expectEqualStrings("Database", prefixGroup("DATABASE_URL").?); + try std.testing.expectEqualStrings("Application", prefixGroup("APP_ENV").?); + try std.testing.expectEqualStrings("Session", prefixGroup("SESSION_DRIVER").?); + try std.testing.expect(prefixGroup("STRIPE_SECRET") == null); +} + +test "isRule matches only wordless comments" { + try std.testing.expect(isRule("# ─────────────")); + try std.testing.expect(isRule("#")); + try std.testing.expect(isRule("# ---------")); + try std.testing.expect(!isRule("# ─── Auth ───")); + try std.testing.expect(!isRule("# set this before booting")); + try std.testing.expect(!isRule("DB_HOST=1")); +} + +test "headerLabel reads a banner's label and leaves prose alone" { + try std.testing.expectEqualStrings("Auth", headerLabel("# ─── Auth ───").?); + try std.testing.expectEqualStrings( + "s3 driver (MinIO)", + headerLabel("# --- s3 driver (MinIO) ---").?, + ); + try std.testing.expect(headerLabel("# set this before booting") == null); + try std.testing.expect(headerLabel("# ─────") == null); +} diff --git a/tools/src/lib/plugin_env.zig b/tools/src/lib/plugin_env.zig index 6e2201f..aa5ef51 100644 --- a/tools/src/lib/plugin_env.zig +++ b/tools/src/lib/plugin_env.zig @@ -25,6 +25,7 @@ const std = @import("std"); const util = @import("util.zig"); +const envfile = @import("env_file.zig"); const Io = std.Io; const Dir = std.Io.Dir; @@ -150,8 +151,45 @@ pub fn hasKey(content: []const u8, key: []const u8) bool { return false; } +/// Byte offset just past the last non-blank line of this plugin's existing +/// block, or null when the file has no block for it yet. +/// +/// Without this, a plugin that gains a variable in a later version seeds a +/// SECOND `# ─── Auth ───` block on the next enable, and a third after that. +/// The file still works — every key is present exactly once — but the grouping +/// it was written to provide quietly stops being true, which is the whole point +/// of the block. +fn insertionPoint(content: []const u8, pluginName: []const u8) ?usize { + var found = false; + var end: ?usize = null; + var pos: usize = 0; + + while (pos <= content.len) { + const nl = std.mem.indexOfScalarPos(u8, content, pos, '\n') orelse content.len; + const line = content[pos..nl]; + + if (envfile.headerLabel(line)) |label| { + if (found) break; // the next block starts here + if (std.mem.eql(u8, label, pluginName)) found = true; + } else if (found and std.mem.trim(u8, line, " \t\r").len > 0) { + end = nl; + } + + if (nl == content.len) break; + pos = nl + 1; + } + + return if (found) (end orelse null) else null; +} + /// Append every variable of `vars` that the project's `.env` does not already /// mention, under a labelled block. Creates the file when absent. +/// +/// Two rules, both load-bearing. A key already in the file — set, or commented +/// out — is never rewritten, so a real secret is never clobbered by a re-enable +/// or by a second plugin that happens to declare the same variable. And the new +/// keys go into this plugin's OWN block, merged into it when one already +/// exists. pub fn seed( allocator: std.mem.Allocator, io: Io, @@ -179,27 +217,12 @@ pub fn seed( return .{ .added = missing.items, .skipped = skipped, .path = path, .created = created }; } - var out: std.ArrayList(u8) = .empty; - try out.appendSlice(allocator, existing); - - // Exactly one blank line before the block, whatever the file ended with. - if (out.items.len > 0) { - while (out.items.len > 0 and (out.items[out.items.len - 1] == '\n' or out.items[out.items.len - 1] == '\r')) { - _ = out.pop(); - } - try out.appendSlice(allocator, "\n\n"); - } - - try out.appendSlice(allocator, try std.fmt.allocPrint( - allocator, - "# ─── {s} ─────────────────────────────────────────────────\n" ++ - "# Declared in the plugin's module.json config[]. Added by `hkm plugins enable`.\n", - .{pluginName}, - )); - + // The variable lines themselves, built once — they go either into this + // plugin's existing block or into a fresh one. + var body: std.ArrayList(u8) = .empty; for (missing.items) |v| { if (v.default) |d| { - try out.appendSlice(allocator, try std.fmt.allocPrint(allocator, "{s}={s}\n", .{ v.key, d })); + try body.appendSlice(allocator, try std.fmt.allocPrint(allocator, "{s}={s}\n", .{ v.key, d })); continue; } @@ -207,7 +230,7 @@ pub fn seed( // Active but empty. The kernel counts '' as missing, so the boot // still stops here until a real value is supplied — which is the // correct outcome for something like an API key. - try out.appendSlice(allocator, try std.fmt.allocPrint( + try body.appendSlice(allocator, try std.fmt.allocPrint( allocator, "{s}= # REQUIRED{s} — set this before booting\n", .{ v.key, typeSuffix(allocator, v.type_name) }, @@ -217,13 +240,50 @@ pub fn seed( // Optional with no default: COMMENTED. Writing it empty would be read as // the string '' and would quietly beat the plugin's own default. - try out.appendSlice(allocator, try std.fmt.allocPrint( + try body.appendSlice(allocator, try std.fmt.allocPrint( allocator, "# {s}= # optional{s}\n", .{ v.key, typeSuffix(allocator, v.type_name) }, )); } + var out: std.ArrayList(u8) = .empty; + + if (insertionPoint(existing, pluginName)) |at| { + // Merge into the block this plugin already owns. + const rest = existing[at..]; + const tail = std.mem.trimStart(u8, rest, "\n"); + // How the block was separated from whatever follows it. The inserted + // lines go INSIDE the block, so that separation has to be put back — + // otherwise every re-seed pulls the next block up by one line. + const newlines = rest.len - tail.len; + + try out.appendSlice(allocator, existing[0..at]); + try out.append(allocator, '\n'); + try out.appendSlice(allocator, body.items); + var n: usize = 1; + while (n < newlines) : (n += 1) try out.append(allocator, '\n'); + try out.appendSlice(allocator, tail); + } else { + try out.appendSlice(allocator, existing); + + // Exactly one blank line before the block, whatever the file ended with. + if (out.items.len > 0) { + while (out.items.len > 0 and (out.items[out.items.len - 1] == '\n' or out.items[out.items.len - 1] == '\r')) { + _ = out.pop(); + } + try out.appendSlice(allocator, "\n\n"); + } + + try out.appendSlice(allocator, try std.fmt.allocPrint( + allocator, + "# ─── {s} ─────────────────────────────────────────────────\n" ++ + "# Declared in the plugin's module.json config[]. Added by `hkm plugins enable`.\n", + .{pluginName}, + )); + try out.appendSlice(allocator, body.items); + } + Dir.cwd().writeFile(io, .{ .sub_path = path, .data = out.items }) catch |e| return e; // A .env holds secrets; a freshly created one should not be world-readable. @@ -257,3 +317,26 @@ test "hasKey does not match a longer key with the same prefix" { test "hasKey ignores a key mentioned only in prose" { try std.testing.expect(!hasKey("# see APP_KEY for details\n", "APP_KEY")); } + +test "insertionPoint finds a plugin's own block and ignores the next one" { + const src = + "APP_KEY=x\n\n" ++ + "# ─── Auth ───\n" ++ + "AUTH_TTL=60\n\n" ++ + "# ─── Mail ───\n" ++ + "MAIL_HOST=smtp\n"; + + // Just past `AUTH_TTL=60` — inside Auth, not swallowing the Mail block. + const at = insertionPoint(src, "Auth").?; + try std.testing.expectEqualStrings("APP_KEY=x\n\n# ─── Auth ───\nAUTH_TTL=60", src[0..at]); + + try std.testing.expect(insertionPoint(src, "Storage") == null); +} + +test "a key already in the file is never rewritten, set or commented" { + // Both forms count as present: rewriting a commented one would grow the + // file on every enable, and rewriting a set one would clobber a real secret. + try std.testing.expect(hasKey("DEMO_SECRET=live-value\n", "DEMO_SECRET")); + try std.testing.expect(hasKey("# DEMO_MODE=\n", "DEMO_MODE")); + try std.testing.expect(!hasKey("DEMO_MODE_X=1\n", "DEMO_MODE")); +} diff --git a/tools/src/lib/services.zig b/tools/src/lib/services.zig index 47f33eb..5e5c91f 100644 --- a/tools/src/lib/services.zig +++ b/tools/src/lib/services.zig @@ -15,6 +15,20 @@ const EnvMap = std.process.Environ.Map; /// Resolve the project root directory from a path or registered name. /// Returns an absolute path to a folder that contains a proj.json. +/// `path` and every directory above it, nearest first, ending at "/". +/// +/// Split out from `resolveRoot` so the walk itself is testable without a +/// filesystem: the interesting part is the sequence, not the stat. +pub fn ancestors(allocator: std.mem.Allocator, path: []const u8) ![]const []const u8 { + var out: std.ArrayList([]const u8) = .empty; + var cursor: ?[]const u8 = util.trimSlash(path); + while (cursor) |p| : (cursor = util.parentOf(p)) { + try out.append(allocator, if (p.len == 0) "/" else p); + if (p.len == 0 or std.mem.eql(u8, p, "/")) break; + } + return out.items; +} + pub fn resolveRoot(allocator: std.mem.Allocator, io: Io, env: *EnvMap, target: []const u8) !?[]const u8 { // No target → current working directory. const candidate = if (target.len == 0) (env.get("PWD") orelse ".") else target; @@ -25,6 +39,21 @@ pub fn resolveRoot(allocator: std.mem.Allocator, io: Io, env: *EnvMap, target: [ return abs; } + // CWD MODE: walk up. Every other tool open in that same terminal — git, + // composer, npm — finds its project from anywhere inside it, and being told + // "'.' is neither a project folder nor a registered name" while standing in + // `/app` is a worse answer than the directory above holds. + // + // Only when no target was given. An EXPLICIT path stays exact: `hkm install + // ./tools` quietly hardening the parent project instead of failing is the + // kind of help nobody wants from a command that chowns things. + if (target.len == 0) { + for ((try ancestors(allocator, abs))[1..]) |dir| { + const marker = try std.fmt.allocPrint(allocator, "{s}/proj.json", .{dir}); + if (util.fileExists(io, marker)) return dir; + } + } + // NAME MODE: look the name up in the kernel registry. if (target.len > 0) { if (try registry.resolvePath(allocator, io, env)) |jsonPath| { @@ -160,3 +189,39 @@ pub fn replace(allocator: std.mem.Allocator, input: []const u8, needle: []const try out.appendSlice(allocator, rest); return out.toOwnedSlice(allocator); } + +// ── tests ────────────────────────────────────────────────────── + +test "ancestors walks from the directory up to the root, nearest first" { + const a = std.testing.allocator; + var arena = std.heap.ArenaAllocator.init(a); + defer arena.deinit(); + + const got = try ancestors(arena.allocator(), "/srv/app/src/Http"); + try std.testing.expectEqual(@as(usize, 5), got.len); + try std.testing.expectEqualStrings("/srv/app/src/Http", got[0]); + try std.testing.expectEqualStrings("/srv/app/src", got[1]); + try std.testing.expectEqualStrings("/srv/app", got[2]); + try std.testing.expectEqualStrings("/srv", got[3]); + try std.testing.expectEqualStrings("/", got[4]); +} + +test "ancestors terminates on the root itself" { + const a = std.testing.allocator; + var arena = std.heap.ArenaAllocator.init(a); + defer arena.deinit(); + + const got = try ancestors(arena.allocator(), "/"); + try std.testing.expectEqual(@as(usize, 1), got.len); + try std.testing.expectEqualStrings("/", got[0]); +} + +test "a trailing slash does not produce a duplicate first entry" { + const a = std.testing.allocator; + var arena = std.heap.ArenaAllocator.init(a); + defer arena.deinit(); + + const got = try ancestors(arena.allocator(), "/srv/app/"); + try std.testing.expectEqualStrings("/srv/app", got[0]); + try std.testing.expectEqualStrings("/srv", got[1]); +} diff --git a/tools/src/main.zig b/tools/src/main.zig index 1be382b..26ec0de 100644 --- a/tools/src/main.zig +++ b/tools/src/main.zig @@ -5,6 +5,7 @@ const update_cmd = @import("commands/update.zig"); const run_cmd = @import("commands/run.zig"); const list_cmd = @import("commands/list.zig"); const discover_cmd = @import("commands/discover.zig"); +const env_cmd = @import("commands/env.zig"); const plugins_cmd = @import("commands/plugins.zig"); const module_cmd = @import("commands/module.zig"); const ui_cmd = @import("commands/ui.zig"); @@ -32,6 +33,7 @@ fn printHelp(allocator: std.mem.Allocator, io: std.Io, env: *std.process.Environ prompt.item("hkm list", "list registered projects (alias: ls)"); prompt.item("hkm discover [root]", "find projects on disk and register them (alias: scan)"); prompt.item("hkm plugins [path|name]", "analyse a project's enabled plugins/modules"); + prompt.item("hkm env [audit|dedupe|group]", "audit a project's .env: duplicate keys, grouping"); prompt.item("hkm module [create|delete]", "scaffold a first-party kernel package (modules/)"); prompt.item("hkm ui [sync|list|link|clean]", "federate enabled plugins' UIs into the frontend"); prompt.item("hkm update ", "refresh a project's kernel registry entry"); @@ -385,6 +387,11 @@ fn dispatch(init: std.process.Init.Minimal, mm: *memory.Manager) !u8 { defer scope.end(); return try discover_cmd.run(scope.allocator(), io, &env_map, args); } + if (std.mem.eql(u8, cmd, "env")) { + var scope = CmdScope.begin(mm, "env"); + defer scope.end(); + return try env_cmd.run(scope.allocator(), io, &env_map, args); + } if (std.mem.eql(u8, cmd, "module")) { var scope = CmdScope.begin(mm, "module"); defer scope.end(); diff --git a/tools/src/tests.zig b/tools/src/tests.zig index 027083b..477dcb1 100644 --- a/tools/src/tests.zig +++ b/tools/src/tests.zig @@ -29,6 +29,7 @@ test { _ = @import("commands/discover.zig"); _ = @import("commands/install.zig"); _ = @import("commands/doctor.zig"); + _ = @import("commands/env.zig"); _ = @import("commands/list.zig"); _ = @import("commands/module.zig"); _ = @import("commands/new.zig"); @@ -43,6 +44,7 @@ test { _ = @import("constants.zig"); _ = @import("lib/banner.zig"); _ = @import("lib/composer_version.zig"); + _ = @import("lib/env_file.zig"); _ = @import("lib/install_scope.zig"); _ = @import("lib/inspector/dashboard.zig"); _ = @import("lib/inspector/meminspector.zig"); From ad294c87107251ad5dfc997d140ec1e01c815299 Mon Sep 17 00:00:00 2001 From: hakeemRash Date: Thu, 3 Sep 2026 02:26:38 +0300 Subject: [PATCH 12/14] fix(env-example): TENANCY_CONTROL_PLANE is a bool, not a hostname MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The example read TENANCY_CONTROL_PLANE=admin.example.com, which parses as "the control plane is served from this host". The Tenancy plugin declares the key as type: bool, and any non-empty string is truthy — so uncommenting the documented value switched tenant routing off entirely, which is the opposite of what a multi-tenant deployment is configuring. Adds TENANCY_CENTRAL_DOMAINS, a declared key the example never mentioned, and names the modes. Verified against the plugin's module.json config[], which remains the authority. --- .env.example | 12 +++++++----- CHANGELOG.md | 8 ++++++++ 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/.env.example b/.env.example index ca586b8..fa03c07 100644 --- a/.env.example +++ b/.env.example @@ -103,11 +103,13 @@ HTTP_CLIENT_CONNECT_TIMEOUT=10 HTTP_CLIENT_RETRY=2 HTTP_CLIENT_MAX_RESPONSE_BYTES=33554432 # 32 MiB OOM guard -# ── Multi-tenancy (Tenancy plugin — control plane) ─────────────────────────── -# Only needed when deploying the multi-tenant control plane. -# TENANCY_MODE=subdomain -# TENANCY_BASE_DOMAINS=example.com -# TENANCY_CONTROL_PLANE=admin.example.com +# ── Multi-tenancy (Tenancy plugin) ─────────────────────────────────────────── +# Only needed when the Tenancy plugin is enabled. See that plugin's README for +# the authoritative list — these are the ones a domain-mode deployment needs. +# TENANCY_MODE=domain # claim | domain | host +# TENANCY_BASE_DOMAINS=example.com # domain mode: tenant label hangs off these +# TENANCY_CENTRAL_DOMAINS=example.com,admin.example.com # hosts served CENTRAL (else 404 under strict routing) +# TENANCY_CONTROL_PLANE=false # bool — true disables tenant routing entirely # ── Views / Frontend (View + ViteManifest plugins) ─────────────────────────── # VIEW_PATHS= # extra template roots, prepended to the cascade diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d8ecae..37ee7cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 pointing at the cause. Enabling an already-enabled plugin now tops up its block. Safe by construction: the seeder only ever ADDS keys the file does not already mention, in any form, so a real secret is never rewritten. +- **`.env.example` documented the Tenancy control-plane switch as a hostname.** + `TENANCY_CONTROL_PLANE=admin.example.com` reads as "the control plane lives + here"; the plugin declares the key as `type: bool`, where any non-empty string + is truthy — so the example value silently turned tenant routing OFF for anyone + who uncommented it. Corrected to a bool, with `TENANCY_CENTRAL_DOMAINS` (a + real declared key that was missing) added beside it and the mode values named. + The plugin's own `module.json` stays the authority; this is the example + catching up to it. - **Re-seeding wrote a second block for the same plugin.** The append was unconditional, so a plugin seeded twice got two `# ─── Auth ───` headings, and three after that. Every key was still present exactly once, so nothing broke — From 6f1065bc2fe776777a9c87d8d397ebdaf957b37d Mon Sep 17 00:00:00 2001 From: hakeemRash Date: Thu, 3 Sep 2026 02:35:34 +0300 Subject: [PATCH 13/14] fix(graph-seo): add optional alt property for Open Graph image --- projects/Http/Controllers/Concerns/InteractsWithGraphSeo.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/projects/Http/Controllers/Concerns/InteractsWithGraphSeo.php b/projects/Http/Controllers/Concerns/InteractsWithGraphSeo.php index a84fe43..cd495f6 100644 --- a/projects/Http/Controllers/Concerns/InteractsWithGraphSeo.php +++ b/projects/Http/Controllers/Concerns/InteractsWithGraphSeo.php @@ -157,6 +157,7 @@ protected function seoFor( ?string $searchUrl = null, string $locale = 'en_US', bool $index = true, + ?string $alt = null, ?Request $request = null, ): string { $siteName ??= (string) (env('APP_NAME') ?: ''); @@ -191,6 +192,7 @@ protected function seoFor( $og = $this->openGraph($ogType, $title) ->url($url) ->locale($locale) + ->addProperty('og', 'image:alt',$alt) // fallback for OG image alt ->twitterLargeImage(); if ($description !== '') { From 985c49fb15f1a17e3baaa3350ee4c31ed25e563b Mon Sep 17 00:00:00 2001 From: hakeemRash Date: Thu, 3 Sep 2026 02:39:18 +0300 Subject: [PATCH 14/14] fix(graph-seo): update Open Graph image handling to use alt text from title --- projects/Http/Controllers/Concerns/InteractsWithGraphSeo.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/projects/Http/Controllers/Concerns/InteractsWithGraphSeo.php b/projects/Http/Controllers/Concerns/InteractsWithGraphSeo.php index cd495f6..440bae1 100644 --- a/projects/Http/Controllers/Concerns/InteractsWithGraphSeo.php +++ b/projects/Http/Controllers/Concerns/InteractsWithGraphSeo.php @@ -192,7 +192,6 @@ protected function seoFor( $og = $this->openGraph($ogType, $title) ->url($url) ->locale($locale) - ->addProperty('og', 'image:alt',$alt) // fallback for OG image alt ->twitterLargeImage(); if ($description !== '') { @@ -202,7 +201,7 @@ protected function seoFor( $og->siteName($siteName); } if ($image !== null && $image !== '') { - $og->image($this->ogImage($image, 1200, 630, $title, $request)); + $og->image($this->ogImage($image, 1200, 630, $alt ?? $title, $request)); } if ($og instanceof Article) { if (($data['authorUrl'] ?? '') !== '') {