critical | §5.6 grant mechanism does not exist anywhere in the codebase | allowScriptExecution / bundles.<name>.allowScriptExecution has zero hits in src/, so today "third-party bundle → script asset" is unconditionally executable (script run:/setup: is already advisory-only per src/core/activation-policy.ts comments) — the whole §5.6 line is unbuilt, not just "resolve-time enforceable," and no code path currently gates it | grep across src/ for allowScriptExecution/ScriptExecution; src/core/activation-policy.ts rules 1-4 (env/task/write-activation only, no script-execution rule).
critical | §5.6's reused "ceiling philosophy" (tools: provenance gate, M5) is unverified against code | The spec says script-execution enforcement reuses "the writable-vs-third-party provenance ceiling for self-declared tools:" that "already lives" at the show layer, but no such ceiling/gate keyed on writable-vs-registryId for tools: was found anywhere under src/integrations/agent/, src/core/adapter/, or src/core/asset/ — if M5's premise is wrong, §5.6's plan to "consume the show layer's verdict" has nothing to consume and must build the primitive from scratch, changing the cost estimate | grep for provenance ceiling/toolPolicy in agent/adapter trees turned up no writable/registryId-keyed logic; only activation-policy.ts rule 4 (isSourceWriteActivated) and rule 1 (env dangerous-key) exist as analogous patterns.
major | §5.6 composition-chain bypass is not addressed by the per-bundle grant design | A writable task composing uses: tasks/<ref> where the referenced task (in a third-party bundle) itself targets uses: scripts/<n> resolves recursively (§5.1 { kind: task, ref } "resolves recursively once") — the spec never states whose bundle-grant applies (the composing writable task's bundle, which has no grant, or the referenced task's own third-party bundle, which needs one) or how §5.4's resolve stage attributes the grant when task-A (bundle X, writable) pulls task-B (bundle Y, third-party, ungranted) that pulls script-C — this is exactly the "composition chains" bypass route named in the audit brief and v8 gives it no explicit resolution rule | §5.1 recursive task resolution; §5.6 (grant keyed by bundle, not by resolved-asset chain); §2.3 "no nesting" only forbids workflow-in-step, not task-in-task depth-1 recursion through third-party bundles.
major | §5.6 grant granularity is per-bundle, not per-asset, so one grant silently activates every future script in that bundle including ones added after the grant (bundle-update bypass) | The grant text says "settable... by akm config set" against bundles.<name>.allowScriptExecution: true — a boolean on the whole bundle. §5.6 does not pin the grant to a content hash or a specific script set, so akm update on a third-party bundle can swap in new/modified script content (or a modified run:/cwd:) that inherits the prior grant with no re-confirmation — the audit brief's "bundle update swapping script content after grant" bypass is not closed by the design as stated | §5.6 grant text (bundles.<name>.allowScriptExecution: true); no mention of grant invalidation on content change; contrast with §5.3's hash-preimage discipline for dispatch identity, which has no counterpart for authorization identity.
major | §5.6 cron-fire-time vs sync-time grant evaluation is unspecified, and §5.4 says resolution happens at "resolve time," which for the task path is task sync/lint, not task run fire time | The audit brief specifically asks whether grants are evaluated at sync time (when the OS entry is installed) or fire time (when cron actually invokes akm task run <id>); §5.6 says "enforced at both executors" and "applied at resolve time (§5.4)" but §5.4's two-stage compile/resolve/freeze pipeline is described for workflows, and the task path's own freeze/resolve moment relative to sync vs the scheduler's task run invocation is never pinned — if the grant is checked at sync time only, revoking allowScriptExecution after sync leaves the OS-level cron entry still firing the script, since §5.2 says cron enable/disable is a separate enabled: gate, not a script-authorization gate | §5.6; §2.3 ("Trigger keys ... consumed only by akm task sync and, at fire time, the task runner's enabled gate runner.ts:169" — this passage explicitly documents enabled as fire-time-checked but says nothing analogous for the script-execution grant).
major | §5.5 "literal entries ... never redacted" collides with param-secrets.ts's own heuristic scanner for exactly this shape of leak, and the spec does not reconcile the two | param-secrets.ts exists specifically because author-written values (there: workflow params) that "look like credentials" are dangerous when they cannot be redacted — it scans key names and value entropy and warns. §5.5 freezes env: literal values into the plan with the identical non-redactable contract ("frozen into the plan, hashed, shown by brief, never redacted") but the spec's decision log gives no equivalent heuristic-warning obligation for literal env entries the way param-secrets.ts already provides for params — an author who writes env: [{ API_KEY: "sk-live-..." }] (a plausible mistake, not even malicious) gets zero warning at freeze/lint time despite the codebase already having the exact detector (detectSecretShapedParams) that could be pointed at literal env values too | src/workflows/exec/param-secrets.ts (rationale + detectSecretShapedParams); spec §5.5 (no mention of applying the existing secret-shape heuristic to literal env entries); §5.4 (lint's dry-freeze would be the natural hook and isn't used for this).
major | §5.5's AKM_ITEM (JSON) env injection has no shell-escaping/quoting story and no code precedent to check against | AKM_ITEM does not exist anywhere in src/ yet (grep confirms zero hits) — this is wholly new plumbing proposed by §5.5 ("a map: step with a shell target receives AKM_ITEM (JSON) and AKM_ITEM_INDEX per unit"). JSON serialized into a process env var is safe from shell metacharacter injection only if the executor sets it via the env object passed to spawn/Bun.spawn (not string-interpolated into a shell command line); the spec doesn't say which, and existing shell-invoking code in this codebase (src/tasks/backends/exec-utils.tsspawnCommand) passes argv arrays with no shell, which is the safe pattern, but nothing pins §5.5's AKM_ITEM assembly to that pattern rather than to a run: string that a script might eval/interpolate itself (e.g., a script doing node -e "$AKM_ITEM" would be JSON-injection-shaped if the JSON contains "/backtick sequences and the script author naively string-interpolates it into a nested command) | §5.5 "Env assembly moves per-unit ... receives AKM_ITEM (JSON)"; no code precedent (grep AKM_ITEM src/ → 0 hits); src/tasks/backends/exec-utils.tsspawnCommand shows the codebase's existing safe argv-array convention that §5.5 should but does not explicitly commit to.
major | §8 migration plan does not fit the actual 71bb686 backup/journal model for the .yml→.md file-level transform | migration-backup.ts's manifest/journal machinery (ARTIFACT_NAMES, MigrationBackupManifest, apply/restore journal + sentinel) covers exactly four opaque artifacts — config.json, state.db, workflow.db, index.db — and has no concept of per-bundle-file changes; the actual hardened per-file transform pattern in this codebase is task-target-ref-migration.ts's plan/apply split (read-before-write byte-fencing via current.equals(rewrite.before), single atomic rename per file, no cross-file transaction). §8's .yml→.md conversion is a two-file operation (create the new .md, remove the old .yml) that this per-file model does not support atomically: an interruption between writing the new .md and removing the old .yml leaves both present, which the spec's own "<id>.yml + <id>.md collide on one conceptId → same named error" rule would then fire on the migrator's own half-applied state, turning an interrupted migration into a self-inflicted hard error rather than a resumable step | scripts/akm-migrate/migration-backup.ts (ARTIFACT_NAMES, manifest scope); scripts/akm-migrate/migrate/legacy/task-target-ref-migration.ts (per-file plan/apply, single-file atomicity only); spec §8 tombstone-collision rule.
major | §8's "vendored, frozen copy of the v2 task parser" fix is not yet applied at 71bb686 — the cited defect is confirmed live, not already resolved | scripts/akm-migrate/migrate/legacy/task-target-ref-migration.ts line 25 currently does import { parseTaskDocument } from "../../../../src/tasks/parser" — the live, non-frozen parser — exactly the M7 finding the spec describes as needing a fix "as part of this work." This is consistent with the spec's own claim (not a contradiction), but it means the "frozen-migrator principle" is presently violated in the baseline the spec is built on, and v8 gives no interim mitigation for the window between baseline and when the vendoring lands — any src/tasks/parser change merged before the vendoring work starts will retroactively break akm-migrate silently, since nothing in the migrator's test suite pins the parser's frozen behavior independent of src/tasks/parser | scripts/akm-migrate/migrate/legacy/task-target-ref-migration.ts:25; spec §8 first bullet and §11 M7 row.
major | §8 read-only bundle handling for the .yml tombstone/collision rule doesn't address the same read-only-bundle carve-out already coded for the existing migration | task-target-ref-migration.ts's planTaskTargetRefMigration explicitly skips non-writable bundles and instead warns that their v1 .yml tasks "will not run after upgrade" (never rewritten, never deleted) — this is the precedent §8's new .yml→.md conversion must follow for read-only bundles, but §8 states only "task discovery keeps matching tasks/*.yml permanently, as a diagnostic" and "sync/doctor/lint each hard-error" without saying whether that hard-error applies to a read-only third-party bundle the operator cannot fix (contradicting the existing precedent of warn-and-continue for exactly that case) — operators with a read-only bundle carrying legacy .yml tasks would move from "warned, keeps working" (current) to "hard-errors sync/doctor/lint" (v8), a behavior regression not costed anywhere in §9 | §8 tombstone rule; task-target-ref-migration.ts lines 253-271 (existing read-only warn-and-skip precedent); spec §9 cost inventory has no line for this regression.
minor | §2.4 lenient placeholder filling is prompt-injection-adjacent but bounded by scope, not fully safe | Leaving unmatched {{name}}/$ARGUMENTS/$1-$9 verbatim (rather than erroring) means an operator who edits with: and mistypes a key, or pulls a shared command asset whose placeholder grammar drifted, gets a raw {{typo}} or literal $3 byte-string injected into the agent prompt with no runtime signal beyond a lint warning that a human must have already run and read — this is "silent garbage into an agent prompt" as the audit brief frames it; it's a correctness/quality risk more than an injection vector since the text is author-controlled, not attacker-controlled, but shared/imported command templates make the author and the operator different trust boundaries, and §2.4 doesn't add any freeze-time notice surfaced to the operator (only lint, which is opt-in) | §2.4 ("An unmatched placeholder or unused with: key is a lint warning and is left verbatim — not a runtime hard error"); §5.4 (lint dry-freeze exists but running lint is not mandatory before dispatch).
minor | §5.7 PATH-prepend hijack risk is named but Windows semantics are not actually specified | §5.7 says bare akm resolves "by PATH prepend in the child environment" replacing the argv-rewrite, and separately flags Windows via schtasks/PATHEXT/COMSPEC handling elsewhere in the codebase (redaction.ts allowlists PATHEXT, COMSPEC, WINDIR explicitly, suggesting Windows env quirks are already a known sharp edge) — but §5.7 never states whether the prepend uses ;-joined Path/PATH (Windows env vars are case-insensitive and there can be both Path and PATH keys colliding) nor whether a child script that does its own where akm/akm.cmd shim resolution could still pick up a stale/hijacked entry earlier in the original PATH the prepend is supposed to shadow (prepending only shadows if the child's PATH lookup honors first-match order, which is true for cmd.exe but not guaranteed for every shell/script interpreter a scripts/ asset might invoke) | §5.7; src/core/redaction.ts PATHEXT/COMSPEC/WINDIR allowlist entries (evidence Windows env handling is already fragile in this codebase); no cross-reference to src/tasks/resolve-akm-bin.ts's existing Windows-aware absolute-path resolution, which §5.7 could reuse instead of a PATH string prepend.
minor | §5.5 env-literal redaction carve-out has no cross-check against redactSensitiveValue's recursive, boundary-agnostic redaction, creating a silent divergence between two redaction call sites | src/core/redaction.ts's redactSensitiveValue recursively redacts every string leaf of a structured value using sensitiveValues before it "crosses a durable/output boundary" — a general-purpose function with no knowledge of the env literal/ref split §5.5 introduces; if any caller applies redactSensitiveValue to a frozen plan (or its env-literal entries) using a sensitiveValues set built only from ref-sourced secrets (per §5.5's "never redacted" contract for literals), literals are correctly preserved — but if a different caller (e.g. a generic log-sanitization pass) instead builds sensitiveValues more broadly and includes literal values, §5.5's "never redacted" guarantee silently breaks for that surface with no test/contract in the spec pinning which call sites must respect the split | src/core/redaction.ts:385-398 (redactSensitiveValue, generic/recursive, no literal/ref awareness); spec §5.5 (states the contract but does not name which call sites must special-case literals vs which already handle the split correctly).