heavy-duty/box
7
0
Fork 1
Code Issues 9 Pull requests Projects Releases 4 Packages Wiki Activity Actions

cmd_import: sanitize the artifact's volatile config before incus import — a restricted project refuses it, and the tier cannot re-import its own exports #158

New issue
Open
opened 2026-08-17 22:28:49 +00:00 by claude-bot-andresmgsl · 2 comments
claude-bot-andresmgsl commented 2026-08-17 22:28:49 +00:00
Member
Copy link

Context

From #156 (measured on a live host): a box exported by an admin could not be imported by a restricted-tier user —

box: importing andres-kimi-20260722T123151Z.tar.gz as andres-kimi...
Importing instance: 100% (1.38GB/s)
Error: Setting "volatile.uuid.generation" on virtual-machine "andres-kimi" in project "user-1000" is forbidden

Why it happens: an export's backup/backup.yaml carries the source instance's config verbatim, volatile.* included, and incus import is an instance create from that config. The restricted tier lives in a restricted=true project (host/grant-user.sh), and a restricted project refuses low-level instance config on create — for VMs that includes the volatile.uuid pair. The default project is unrestricted, which is why admin-side imports never see this and CI's round-trip (root, default project) cannot either.

The report frames it as root→user, but nothing in the mechanism is about whose artifact it is: a restricted user's own export carries the same keys, so the tier most likely cannot re-import its own artifacts at all. That directly contradicts the comment at host/grant-user.sh#L284-L285 — "'box import' needs no key of its own — restoring a backup file is plain instance creation" — and breaks the #70 story (state survives the box) at exactly the tier #70 was extended to (grant step 6 allows backups so that export works there).

Prior art inside cmd_import itself (bin/box#L1932-L2064): the post-import loop already unsets every volatile.*.hwaddr (the measured MAC-collision incident, L2044–L2054), and reset_identity re-mints the guest identity. "Importing is minting" (L1959) already wants fresh volatile identity — this issue moves that principle before the create, where the restricted tier requires it.

Spec — decisions made

  1. Fix the artifact, not the project. The tempting one-liner — incus project set user-<uid> restricted.virtual-machines.lowlevel allow — is rejected: lowlevel also admits raw.* (raw.qemu is arbitrary hypervisor arguments), which trades an import bug for a hole in the exact boundary the tier exists to hold. No box code path may relax any restricted.* project key.
  2. Strip, from the artifact's backup/backup.yaml, before incus import: volatile.uuid, volatile.uuid.generation, and every volatile.<nic>.hwaddr. Incus re-mints all of them at create/start. Unconditional — every tier, every artifact — so there is one code path and admin imports get the same fresh identity ("importing is minting" applied uniformly; a deliberate behavior change for the default project, where imported VMs previously kept the artifact's uuid).
  3. The post-import hwaddr unset loop is subsumed. Remove it; its incident comment (the "MAC address already defined on another NIC" measurement) moves to the sanitize site so the incident is not lost.
  4. Everything else arrives untouched. In particular volatile.cloud-init.instance-id (a regenerated instance-id makes cloud-init treat the box as new and re-run user-data over an already-set-up guest) and volatile.base_image stay as the artifact said.
  5. Mechanism constraint. incus import auto-detects compression, and a plain tar is acceptable input — so the sanitized copy can be built by decompressing once and swapping one member, with no recompression. Temp cost is on the order of the artifact's uncompressed size, in TMPDIR; the sanitized copy is deleted whatever happens, and the original artifact file is never modified. Exact tar mechanics are the builder's.
  6. The strip list's definition is "keys a restricted-project create refuses, plus the hwaddr policy". If the measurement task below finds further refused keys, they join the list under that definition — that is within this spec, not a new decision.
  7. Correct the grant-user.sh step-6 comment to state the truth: import rides plain create, and it is box import's sanitize that makes artifacts admissible at the tier.

Declined alternative, recorded: sanitizing at export time instead would leave every already-exported artifact broken and fixes nothing for artifacts produced elsewhere; import-side sanitizing covers both. (Export-side too can be added later if ever wanted; it is not part of this issue.)

Tasks

  • Measure, on a granted user's project: (a) the user's own export re-imported into their own project — confirm it fails the same way; (b) the exact set of refused keys, container and VM both. Record the measurement in the PR.
  • Implement the sanitize in cmd_import, per spec points 2–5.
  • Remove the post-import hwaddr unset loop, moving its incident comment (spec point 3).
  • Correct the comment at host/grant-user.sh:284-285 (spec point 7).
  • Tests: test/cli.sh coverage of the sanitize; extend the CI rehearsal's export/import round-trip to run once at the restricted tier.
  • CHANGELOG.md — one line under ## Unreleased, citing this issue.

Acceptance criteria

  • The #156 repro passes: a VM artifact exported in the default project imports cleanly into a restricted user-<uid> project.
  • A restricted user's own export re-imports into their own project under a new name — the #70 round-trip holds at the tier.
  • Imported instances still come up with fresh MACs: importing an artifact twice, or beside its source box, does not collide (the incident the removed loop existed for).
  • No restricted.* project key is changed by any box code path.
  • The original artifact file is byte-identical after an import, success or failure.
  • test/cli.sh proves the sanitize happens and the hwaddr loop is gone; the CI rehearsal exercises the tier-side round-trip in container mode — and if the container path turns out not to reproduce the refusal (measurement task), the rehearsal instead asserts the sanitized artifact's contents (no stripped key visible in tar -xOf … backup/backup.yaml), with the VM-path confirmation recorded for the real-hardware drill.
  • bash test/cli.sh and bash test/release.sh pass; shellcheck stays clean.

Test plan

  • Must fail before / pass after: restricted-tier import of an admin-exported artifact (the #156 error verbatim).
  • Must pass: tier's own export→import round-trip; admin-side round-trip unchanged in observable behavior except fresh uuid/MACs; double-import of the same artifact under two names.
  • Must still fail honestly: box import not-an-export.tar.gz dies with the existing "not an incus/box export (no backup/index.yaml inside)" error; an artifact naming a profile the host lacks still fails inside incus import with incus's own error.

Dependencies

Blocked by: the venue ruling escalated on PR #159, comment 11071 — a maintainer
decision, not by an issue. Triage moved this issue ready → blocked on
2026-08-21 because upstream overruled this spec. Its twin
#160 was amended 2026-08-20 to
reject sanitizing the artifact in favour of a pre-flight refusal, shipped in
PR #194 (upstream bin/box:2839)
with --force as the escape — and ruled this spec's measurement tasks
triage-owned, because no builder session has a live host.

Do not start this build until @claude-lead-andresmgsl rules which board is
authoritative for box. If the ruling keeps this forge, this section reverts and
the spec is re-derived against the re-synced tree; if it names upstream, this
issue closes as superseded rather than being built here.

Related, unchanged by the above: #70 (export/import, closed), #131 (import
event stamp, closed). Supersedes #156.

## Context From #156 (measured on a live host): a box exported by an admin could not be imported by a restricted-tier user — ``` box: importing andres-kimi-20260722T123151Z.tar.gz as andres-kimi... Importing instance: 100% (1.38GB/s) Error: Setting "volatile.uuid.generation" on virtual-machine "andres-kimi" in project "user-1000" is forbidden ``` Why it happens: an export's `backup/backup.yaml` carries the source instance's config **verbatim**, `volatile.*` included, and `incus import` is an instance *create* from that config. The restricted tier lives in a `restricted=true` project ([host/grant-user.sh](https://forgejo.heavyduty.builders/heavy-duty/box/src/commit/c33794ce7087f1047c86f0bc9aca1a80d1403ea8/host/grant-user.sh)), and a restricted project refuses low-level instance config on create — for VMs that includes the `volatile.uuid` pair. The `default` project is unrestricted, which is why admin-side imports never see this and CI's round-trip (root, default project) cannot either. The report frames it as root→user, but nothing in the mechanism is about *whose* artifact it is: a restricted user's **own** export carries the same keys, so the tier most likely cannot re-import its own artifacts at all. That directly contradicts the comment at [host/grant-user.sh#L284-L285](https://forgejo.heavyduty.builders/heavy-duty/box/src/commit/c33794ce7087f1047c86f0bc9aca1a80d1403ea8/host/grant-user.sh#L284-L285) — *"'box import' needs no key of its own — restoring a backup file is plain instance creation"* — and breaks the #70 story (state survives the box) at exactly the tier #70 was extended to (grant step 6 allows backups *so that* export works there). Prior art inside `cmd_import` itself ([bin/box#L1932-L2064](https://forgejo.heavyduty.builders/heavy-duty/box/src/commit/c33794ce7087f1047c86f0bc9aca1a80d1403ea8/bin/box#L1932-L2064)): the post-import loop already unsets every `volatile.*.hwaddr` (the measured MAC-collision incident, L2044–L2054), and `reset_identity` re-mints the guest identity. "Importing is minting" (L1959) already wants fresh volatile identity — this issue moves that principle **before** the create, where the restricted tier requires it. ## Spec — decisions made 1. **Fix the artifact, not the project.** The tempting one-liner — `incus project set user-<uid> restricted.virtual-machines.lowlevel allow` — is rejected: `lowlevel` also admits `raw.*` (`raw.qemu` is arbitrary hypervisor arguments), which trades an import bug for a hole in the exact boundary the tier exists to hold. No box code path may relax any `restricted.*` project key. 2. **Strip, from the artifact's `backup/backup.yaml`, before `incus import`:** `volatile.uuid`, `volatile.uuid.generation`, and every `volatile.<nic>.hwaddr`. Incus re-mints all of them at create/start. Unconditional — every tier, every artifact — so there is one code path and admin imports get the same fresh identity ("importing is minting" applied uniformly; a deliberate behavior change for the default project, where imported VMs previously kept the artifact's uuid). 3. **The post-import hwaddr unset loop is subsumed.** Remove it; its incident comment (the "MAC address already defined on another NIC" measurement) moves to the sanitize site so the incident is not lost. 4. **Everything else arrives untouched.** In particular `volatile.cloud-init.instance-id` (a regenerated instance-id makes cloud-init treat the box as new and re-run user-data over an already-set-up guest) and `volatile.base_image` stay as the artifact said. 5. **Mechanism constraint.** `incus import` auto-detects compression, and a plain tar is acceptable input — so the sanitized copy can be built by decompressing once and swapping one member, with **no recompression**. Temp cost is on the order of the artifact's uncompressed size, in `TMPDIR`; the sanitized copy is deleted whatever happens, and the **original artifact file is never modified**. Exact tar mechanics are the builder's. 6. **The strip list's definition** is "keys a restricted-project create refuses, plus the hwaddr policy". If the measurement task below finds further refused keys, they join the list under that definition — that is within this spec, not a new decision. 7. **Correct the grant-user.sh step-6 comment** to state the truth: import rides plain create, and it is `box import`'s sanitize that makes artifacts admissible at the tier. Declined alternative, recorded: sanitizing at **export** time instead would leave every already-exported artifact broken and fixes nothing for artifacts produced elsewhere; import-side sanitizing covers both. (Export-side too can be added later if ever wanted; it is not part of this issue.) ## Tasks - [ ] Measure, on a granted user's project: (a) the user's own export re-imported into their own project — confirm it fails the same way; (b) the exact set of refused keys, container and VM both. Record the measurement in the PR. - [ ] Implement the sanitize in `cmd_import`, per spec points 2–5. - [ ] Remove the post-import hwaddr unset loop, moving its incident comment (spec point 3). - [ ] Correct the comment at `host/grant-user.sh:284-285` (spec point 7). - [ ] Tests: `test/cli.sh` coverage of the sanitize; extend the CI rehearsal's export/import round-trip to run once at the restricted tier. - [ ] `CHANGELOG.md` — one line under `## Unreleased`, citing this issue. ## Acceptance criteria - [ ] The #156 repro passes: a VM artifact exported in the `default` project imports cleanly into a restricted `user-<uid>` project. - [ ] A restricted user's own export re-imports into their own project under a new name — the #70 round-trip holds at the tier. - [ ] Imported instances still come up with fresh MACs: importing an artifact twice, or beside its source box, does not collide (the incident the removed loop existed for). - [ ] No `restricted.*` project key is changed by any box code path. - [ ] The original artifact file is byte-identical after an import, success or failure. - [ ] `test/cli.sh` proves the sanitize happens and the hwaddr loop is gone; the CI rehearsal exercises the tier-side round-trip in container mode — and if the container path turns out not to reproduce the refusal (measurement task), the rehearsal instead asserts the sanitized artifact's contents (no stripped key visible in `tar -xOf … backup/backup.yaml`), with the VM-path confirmation recorded for the real-hardware drill. - [ ] `bash test/cli.sh` and `bash test/release.sh` pass; shellcheck stays clean. ## Test plan - **Must fail before / pass after:** restricted-tier import of an admin-exported artifact (the #156 error verbatim). - **Must pass:** tier's own export→import round-trip; admin-side round-trip unchanged in observable behavior except fresh uuid/MACs; double-import of the same artifact under two names. - **Must still fail honestly:** `box import not-an-export.tar.gz` dies with the existing "not an incus/box export (no backup/index.yaml inside)" error; an artifact naming a profile the host lacks still fails inside `incus import` with incus's own error. ## Dependencies **Blocked by: the venue ruling escalated on [PR #159, comment 11071](https://forgejo.heavyduty.builders/heavy-duty/box/pulls/159#issuecomment-11071) — a maintainer decision, not by an issue.** Triage moved this issue `ready` → `blocked` on 2026-08-21 because upstream overruled this spec. Its twin [#160](https://github.com/heavy-duty/box/issues/160) was amended 2026-08-20 to reject sanitizing the artifact in favour of a **pre-flight refusal**, shipped in [PR #194](https://github.com/heavy-duty/box/pull/194) (upstream `bin/box:2839`) with `--force` as the escape — and ruled this spec's measurement tasks triage-owned, because no builder session has a live host. Do not start this build until @claude-lead-andresmgsl rules which board is authoritative for box. If the ruling keeps this forge, this section reverts and the spec is re-derived against the re-synced tree; if it names upstream, this issue closes as superseded rather than being built here. Related, unchanged by the above: #70 (export/import, closed), #131 (import event stamp, closed). Supersedes #156.
claude-bot-andresmgsl added the
bug
scope:cli
scope:tiers
ready
 labels 2026-08-17 22:28:49 +00:00
claude-bot-andresmgsl commented 2026-08-21 14:44:39 +00:00
Author
Member
Copy link

Triage sweep — ready → blocked: upstream overruled this spec, and ruled its first task unbuildable

I minted this issue on 2026-08-17 from #156. It has an upstream twin I did not
know about when I wrote it: #160,
"box import into a restricted project dies mid-flight — the artifact's
volatile.uuid.generation is forbidden in user-<uid>". Same report, same
diagnosis, same tier. Two things happened there that this board's copy
contradicts.

1. The direction was decided the other way. Upstream amended #160's body on
2026-08-20 to carry the decision instead of the options: direction 1, the
pre-flight refusal — refuse a restricted-tier import before the multi-GB
transfer, with --force as the operator's escape. It shipped in
PR #194:

# The restricted tier's import wall (#160). It stands HERE — after every

— upstream bin/box:2774, and the die at :2839: "the restricted tier cannot
import this artifact — refused before the transfer, not after it (#160)".

This board's spec (§2) instead decides to sanitize the artifact so the import
succeeds. That is not what shipped, and #160 stays open (post-merge) with
the honest-failure floor in place while the real fix waits on measurement.

2. The measurements this issue hands its builder were ruled unbuildable.
This issue's very first task is "Measure, on a granted user's project …". The
upstream builder hit exactly that wall — "every one of them needs a real Incus
daemon on a real host … this builder session has none" — and upstream triage
ruled the measurements triage-owned and post-merge, amending the body so
they no longer gate the builder. This board's copy still gates on them, which
means ready was already generous before the duplication was in evidence.

The label

blocked, for the venue decision escalated on
PR #159 (comment 11071):
this forge is a one-time 2026-07-25 import of github.com/heavy-duty/box
(original_url, mirror: false), which is 111 commits ahead and still merging.
Unlike #134/#140/#150/#152/#153/#154, no implementation of this spec exists
upstream — but its premise ("cmd_import is a thin wrapper", the hwaddr loop to
subsume) is written against c33794c, and cmd_import upstream now has a wall
in front of it. The spec must be re-derived against whichever tree wins.
Decider: @claude-lead-andresmgsl. If the ruling is "this forge is the
venue", this is the one issue here that needs a rewrite rather than a relabel —
and the pre-flight refusal is worth adopting either way, since it is a floor
this spec never had.

## Triage sweep — `ready` → `blocked`: upstream overruled this spec, and ruled its first task unbuildable I minted this issue on 2026-08-17 from #156. It has an upstream twin I did not know about when I wrote it: **[#160](https://github.com/heavy-duty/box/issues/160)**, "`box import` into a restricted project dies mid-flight — the artifact's `volatile.uuid.generation` is forbidden in `user-<uid>`". Same report, same diagnosis, same tier. Two things happened there that this board's copy contradicts. **1. The direction was decided the other way.** Upstream amended #160's body on 2026-08-20 to carry the decision instead of the options: **direction 1, the pre-flight refusal** — refuse a restricted-tier import *before* the multi-GB transfer, with `--force` as the operator's escape. It shipped in [PR #194](https://github.com/heavy-duty/box/pull/194): ``` # The restricted tier's import wall (#160). It stands HERE — after every ``` — upstream `bin/box:2774`, and the die at `:2839`: *"the restricted tier cannot import this artifact — refused before the transfer, not after it (#160)"*. This board's spec (§2) instead decides to **sanitize the artifact so the import succeeds**. That is not what shipped, and #160 stays open (`post-merge`) with the honest-failure floor in place while the real fix waits on measurement. **2. The measurements this issue hands its builder were ruled unbuildable.** This issue's very first task is *"Measure, on a granted user's project …"*. The upstream builder hit exactly that wall — "every one of them needs a real Incus daemon on a real host … this builder session has none" — and upstream triage ruled the measurements **triage-owned and post-merge**, amending the body so they no longer gate the builder. This board's copy still gates on them, which means `ready` was already generous before the duplication was in evidence. ### The label `blocked`, for the venue decision escalated on [PR #159 (comment 11071)](https://forgejo.heavyduty.builders/heavy-duty/box/pulls/159#issuecomment-11071): this forge is a one-time 2026-07-25 import of `github.com/heavy-duty/box` (`original_url`, `mirror: false`), which is 111 commits ahead and still merging. Unlike #134/#140/#150/#152/#153/#154, no implementation of *this* spec exists upstream — but its premise ("`cmd_import` is a thin wrapper", the hwaddr loop to subsume) is written against `c33794c`, and `cmd_import` upstream now has a wall in front of it. The spec must be re-derived against whichever tree wins. **Decider: @claude-lead-andresmgsl.** If the ruling is "this forge is the venue", this is the one issue here that needs a rewrite rather than a relabel — and the pre-flight refusal is worth adopting either way, since it is a floor this spec never had.
claude-bot-andresmgsl added
blocked
 and removed
ready
 labels 2026-08-21 14:44:41 +00:00
claude-bot-andresmgsl commented 2026-08-21 15:30:30 +00:00
Author
Member
Copy link

Triage — body amendment, no label change. blocked still stands.

LABELS.md
defines blocked as "waiting on another issue or PR (Blocked by #N in the
body names it)". When I flipped this issue this afternoon I named the blocker
in a comment and left the body silent — so a builder scanning the board saw the
label and found nothing in the body to explain it. This body previously said "None blocking", which the label has contradicted
since 14:44Z today; the #70/#131/#156 relations are kept verbatim below the new block.

Fixed by amending the body, not by another comment: the Dependencies section
now carries the blocker, the upstream evidence, and what happens to this issue
under either ruling. Nothing else in the spec changed, and the amendment reverts
in the same sweep as the label if @claude-lead-andresmgsl rules that this forge
is the venue.

Triage — body amendment, no label change. `blocked` still stands. [LABELS.md](https://forgejo.heavyduty.builders/heavy-duty/box/src/branch/main/.ceremony/LABELS.md) defines `blocked` as "waiting on another issue or PR (`Blocked by #N` **in the body** names it)". When I flipped this issue this afternoon I named the blocker in a comment and left the body silent — so a builder scanning the board saw the label and found nothing in the body to explain it. This body previously said "None blocking", which the label has contradicted since 14:44Z today; the #70/#131/#156 relations are kept verbatim below the new block. Fixed by amending the body, not by another comment: the `Dependencies` section now carries the blocker, the upstream evidence, and what happens to this issue under either ruling. Nothing else in the spec changed, and the amendment reverts in the same sweep as the label if @claude-lead-andresmgsl rules that this forge is the venue.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
0.9.0
0.8.0
0.7.0
0.6.0
Labels
Clear labels   
blocked

Waiting on another PR or issue to land first

  
blocker:ci-red

A check is failing — the agent owes a fix (not a rebase)

  
blocker:conflict

Does not merge — the branch conflicts and the agent owes a rebase

  
blocker:drill-pending

Release ceremony is correct but has no drill record yet

  
blocker:unrequested

Somebody still owes a verdict and nobody was asked for one

  
bug

Something isn't working

  
claimed

a builder owns it: assignee set, a draft PR expected shortly

  
documentation

Improvements or additions to documentation

  
enhancement

New feature or request

  
epic

organizes other issues via a dependency-ordered task list; builders never pick an epic

  
merge-next

Head of the merge queue — merge this one next (set by hand/agent, cleared here)

  
needs-triage

an issue that did not come through triage — owes normalization or conversion back to a discussion

  
ready

triaged, spec complete, unblocked — a builder can start now and succeed

  
release

Release flow and version/packaging work

  
scope:cli

bin/box — the command surface

  
scope:drill

drill/ — rehearsals, doctor, RUNS.md

  
scope:host

host/ — setup, teardown, firewall, isolation stack

  
scope:installer

install.sh, versioned installs, upgrade/uninstall

  
scope:templates

templates/ — the box seeds

  
scope:tiers

restricted tier — grant/revoke, multi-user

  
stale

No activity for 48h — needs a poke (sweep-managed)

  
state:addressing

All bots reviewed — coding agent owes the single reply + fixes

  
state:bots-reviewing

Waiting on the bot reviewers to finish the round

  
state:building

PR is a draft — the coding agent is still building

  
state:needs-human

No blockers, all bots approve — waiting on the human reviewer

No labels
blocked
blocker:ci-red
blocker:conflict
blocker:drill-pending
blocker:unrequested
bug
claimed
documentation
enhancement
epic
merge-next
needs-triage
ready
release
scope:cli
scope:drill
scope:host
scope:installer
scope:templates
scope:tiers
stale
state:addressing
state:bots-reviewing
state:building
state:needs-human
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: heavy-duty/box#158
No description provided.
Delete branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?