docs(06): research phase — wire icon into .intunewin export
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,356 @@
|
|||||||
|
# Phase 6: Wire Icon into .intunewin Export — Research
|
||||||
|
|
||||||
|
**Researched:** 2026-04-10
|
||||||
|
**Domain:** Python file I/O, Peewee ORM query, .intunewin staging pipeline
|
||||||
|
**Confidence:** HIGH
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
<phase_requirements>
|
||||||
|
## Phase Requirements
|
||||||
|
|
||||||
|
| ID | Description | Research Support |
|
||||||
|
|----|-------------|-----------------|
|
||||||
|
| PKG-04 | User can upload a custom PNG icon for Intune app display (256x256, max 750KB) | Icon upload (icons.py + Icon model) is already complete. This phase wires the stored icon into the .intunewin build by copying it into the tmpdir before `build_intunewin()` is called. |
|
||||||
|
</phase_requirements>
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
|
||||||
|
Phase 5 implemented icon upload and storage (`icons.py`, `Icon` model, SHA256-addressed
|
||||||
|
file under `DATA_DIR/icons/`). The `get_intunewin_package()` endpoint in `packages.py`
|
||||||
|
already builds a `.intunewin` file from a `TemporaryDirectory` staging area. The gap is
|
||||||
|
that the endpoint never queries the `Icon` model, so the PNG never lands in the tmpdir and
|
||||||
|
is never encrypted into the inner ZIP.
|
||||||
|
|
||||||
|
The work is minimal and mechanical: one `Icon.get_or_none()` lookup, one `shutil.copy2()`
|
||||||
|
(or equivalent `open/write`) into `tmpdir`, and an integration test that verifies the icon
|
||||||
|
is present inside the exported package. No schema changes, no new dependencies.
|
||||||
|
|
||||||
|
**Primary recommendation:** Query `Icon.get_or_none(Icon.printer == printer_id)` in
|
||||||
|
`get_intunewin_package()`, and if an icon exists copy it into `tmpdir` as `icon.png`
|
||||||
|
before calling `build_intunewin()`. Add one integration test that uploads an icon and
|
||||||
|
verifies `icon.png` appears inside the `.intunewin` inner ZIP.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Standard Stack
|
||||||
|
|
||||||
|
### Core (already in place — no new installs)
|
||||||
|
|
||||||
|
| Library | Version | Purpose | Why Standard |
|
||||||
|
|---------|---------|---------|--------------|
|
||||||
|
| peewee | 3.x | ORM query for Icon record | Already used throughout; `Icon.get_or_none()` follows established pattern |
|
||||||
|
| Python shutil / pathlib | stdlib | Copy icon file from ICONS_DIR into tmpdir | Zero-dep, already used in codebase |
|
||||||
|
| Pillow | current | Not needed here (already validated on upload) | Upload already guarantees PNG 256x256 |
|
||||||
|
| zipfile | stdlib | Read inner ZIP of .intunewin to assert icon presence in tests | Already used in test_packages.py |
|
||||||
|
|
||||||
|
**Installation:** None required. All dependencies exist.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Architecture Patterns
|
||||||
|
|
||||||
|
### Existing tmpdir staging pattern (packages.py lines 132-149)
|
||||||
|
|
||||||
|
```python
|
||||||
|
with tempfile.TemporaryDirectory(prefix="imptune_") as tmpdir:
|
||||||
|
# Write scripts
|
||||||
|
with open(os.path.join(tmpdir, "install.ps1"), "w", encoding="utf-8") as f:
|
||||||
|
f.write(install_script)
|
||||||
|
# ... uninstall.ps1, detect.ps1 ...
|
||||||
|
|
||||||
|
# Extract driver ZIP into tmpdir/drivers/
|
||||||
|
drivers_subdir = os.path.join(tmpdir, "drivers")
|
||||||
|
os.makedirs(drivers_subdir, exist_ok=True)
|
||||||
|
with zipfile.ZipFile(driver_zip_path, "r") as driver_zf:
|
||||||
|
driver_zf.extractall(drivers_subdir)
|
||||||
|
|
||||||
|
# Build .intunewin
|
||||||
|
build_intunewin(tmpdir, "install.ps1", output_path)
|
||||||
|
```
|
||||||
|
|
||||||
|
The icon copy slots in immediately after driver extraction, before `build_intunewin()`.
|
||||||
|
|
||||||
|
### Pattern: icon lookup + conditional copy
|
||||||
|
|
||||||
|
```python
|
||||||
|
import shutil
|
||||||
|
from imptune.db.models import Icon
|
||||||
|
|
||||||
|
# Inside get_intunewin_package(), after driver validation:
|
||||||
|
icon_record = Icon.get_or_none(Icon.printer == printer_id)
|
||||||
|
if icon_record is not None:
|
||||||
|
icon_src = os.path.join(cfg.ICONS_DIR, icon_record.sha256)
|
||||||
|
icon_dst = os.path.join(tmpdir, "icon.png")
|
||||||
|
shutil.copy2(icon_src, icon_dst)
|
||||||
|
```
|
||||||
|
|
||||||
|
**Key decision:** Copy is conditional — printers without an uploaded icon still export
|
||||||
|
successfully. `shutil.copy2` preserves metadata and is the idiomatic stdlib copy call.
|
||||||
|
|
||||||
|
### Icon filename in tmpdir
|
||||||
|
|
||||||
|
Use `"icon.png"` as the fixed destination name regardless of `original_filename`. This is
|
||||||
|
predictable for the test assertion and matches Intune's expectation for app icons (a
|
||||||
|
well-known filename in the package root).
|
||||||
|
|
||||||
|
### Established ORM access pattern (from project decisions)
|
||||||
|
|
||||||
|
- Use `Model.get_or_none()` (not `Model.get()`) — avoids `DoesNotExist` exception
|
||||||
|
- Test assertions use `list(Model.select().where(...))` not `Model.get()` — avoids Peewee
|
||||||
|
cursor-caching issue with re-init'd DBs across tests (Phase 03 decision)
|
||||||
|
|
||||||
|
### Anti-Patterns to Avoid
|
||||||
|
|
||||||
|
- **Raising 422 when icon is absent:** Icon is optional. The export must succeed without one.
|
||||||
|
- **Storing icon in a subdirectory:** `build_intunewin` walks `source_dir` recursively; any
|
||||||
|
placement works, but root-level `icon.png` is simplest and most predictable.
|
||||||
|
- **Re-validating the PNG in packages.py:** Validation already happened at upload time. Do
|
||||||
|
not import Pillow into packages.py; just copy the bytes.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Don't Hand-Roll
|
||||||
|
|
||||||
|
| Problem | Don't Build | Use Instead | Why |
|
||||||
|
|---------|-------------|-------------|-----|
|
||||||
|
| File copy into tmpdir | Custom open/read/write loop | `shutil.copy2()` | Handles edge cases, one line |
|
||||||
|
| Icon existence check | Filesystem path probe | `Icon.get_or_none()` | Source of truth is the DB record, not filesystem |
|
||||||
|
| Verify icon in .intunewin | Manual ZIP decryption | Read inner ZIP via `zipfile` after decoding (see test pattern below) | The test can open the outer ZIP and then the inner ZIP without decryption using stored bytes |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Common Pitfalls
|
||||||
|
|
||||||
|
### Pitfall 1: ICONS_DIR not `cfg.ICONS_DIR`
|
||||||
|
|
||||||
|
**What goes wrong:** Hardcoding `DATA_DIR + "/icons"` instead of reading `cfg.ICONS_DIR`.
|
||||||
|
**Why it happens:** `cfg.ICONS_DIR` was added in Phase 5 (config.py line 13); easy to miss.
|
||||||
|
**How to avoid:** Always use `cfg.ICONS_DIR` — monkeypatched by `tmp_data_dir` fixture in tests.
|
||||||
|
**Warning signs:** Integration test writes icon to `cfg.ICONS_DIR` but packages.py reads
|
||||||
|
from a hardcoded path — icon silently missing in package.
|
||||||
|
|
||||||
|
### Pitfall 2: Icon copy placed after `build_intunewin()` call
|
||||||
|
|
||||||
|
**What goes wrong:** Icon is written to tmpdir but after `build_intunewin()` already ran;
|
||||||
|
not included in the inner ZIP.
|
||||||
|
**How to avoid:** Icon copy must come BEFORE `build_intunewin(tmpdir, ...)` call.
|
||||||
|
|
||||||
|
### Pitfall 3: Test verifying icon in outer ZIP instead of inner ZIP
|
||||||
|
|
||||||
|
**What goes wrong:** The `.intunewin` outer ZIP contains only `IntuneWinPackage/Contents/
|
||||||
|
IntunePackage.intunewin` (encrypted blob) and `Detection.xml`. `icon.png` is NOT a member
|
||||||
|
of the outer ZIP; it is packed into the inner ZIP (the plaintext content ZIP). A test that
|
||||||
|
opens the outer ZIP and checks `namelist()` for `icon.png` will always fail.
|
||||||
|
**How to avoid:** The integration test must decrypt (or use a pre-decryption approach — see
|
||||||
|
Code Examples below) to verify icon presence. The simplest approach: use a test fixture
|
||||||
|
that calls `build_intunewin` directly against a tmpdir so the inner ZIP can be inspected
|
||||||
|
before encryption, OR read the Detection.xml and re-derive key material from a
|
||||||
|
test-seeded build. Given current architecture uses random keys, the cleanest test strategy
|
||||||
|
is:
|
||||||
|
a) Make a real HTTP request through the test client to get the `.intunewin` bytes
|
||||||
|
b) Use `build_intunewin`'s known structure: the inner ZIP is the AES-encrypted payload
|
||||||
|
— cannot open it directly without the key.
|
||||||
|
c) **Best approach:** Test that the icon file is present in `tmpdir` staging BEFORE
|
||||||
|
`build_intunewin()` is called by exposing a helper, OR introduce a
|
||||||
|
`build_intunewin` that accepts a pre-built inner ZIP for testability.
|
||||||
|
d) **Pragmatic approach that avoids refactoring:** In the integration test, directly
|
||||||
|
call the packages.py function with a monkeypatched `build_intunewin` that records
|
||||||
|
what was in `tmpdir` instead of encrypting. This is the pattern to use.
|
||||||
|
|
||||||
|
**Recommended test strategy:** Monkeypatch `build_intunewin` to capture the staging
|
||||||
|
directory contents, then assert `icon.png` is present among the staged files. This is
|
||||||
|
simpler than decrypting the output and avoids coupling the test to the encryption
|
||||||
|
implementation.
|
||||||
|
|
||||||
|
### Pitfall 4: `printer_id` vs ORM instance in Icon query
|
||||||
|
|
||||||
|
**What goes wrong:** `Icon.get_or_none(Icon.printer == printer)` (ORM instance) vs
|
||||||
|
`Icon.get_or_none(Icon.printer == printer_id)` (integer). Both work in Peewee, but the
|
||||||
|
integer form is more explicit and consistent with how icons.py deletes records
|
||||||
|
(`Icon.delete().where(Icon.printer == printer_id)`).
|
||||||
|
**How to avoid:** Use integer `printer_id` (the local variable from `_get_printer_and_driver`
|
||||||
|
returns the printer ORM object; extract `.id` from it or use it directly — Peewee resolves
|
||||||
|
FK equality either way, but be consistent with existing codebase style).
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Code Examples
|
||||||
|
|
||||||
|
### Icon lookup and conditional copy (insert inside `get_intunewin_package`)
|
||||||
|
|
||||||
|
```python
|
||||||
|
# Source: derived from icons.py pattern + packages.py TemporaryDirectory pattern
|
||||||
|
import shutil
|
||||||
|
from imptune.db.models import Icon
|
||||||
|
|
||||||
|
# After extracting driver ZIP, before build_intunewin():
|
||||||
|
icon_record = Icon.get_or_none(Icon.printer == printer.id)
|
||||||
|
if icon_record is not None:
|
||||||
|
icon_src = os.path.join(cfg.ICONS_DIR, icon_record.sha256)
|
||||||
|
if os.path.isfile(icon_src):
|
||||||
|
shutil.copy2(icon_src, os.path.join(tmpdir, "icon.png"))
|
||||||
|
```
|
||||||
|
|
||||||
|
### Integration test — icon presence in staged files (monkeypatch approach)
|
||||||
|
|
||||||
|
```python
|
||||||
|
# Source: pattern established by test_packages.py + test_icon_upload.py
|
||||||
|
|
||||||
|
def test_intunewin_includes_icon(client, setup_printer_with_driver, tmp_data_dir, monkeypatch):
|
||||||
|
"""Icon PNG is included in .intunewin staging directory."""
|
||||||
|
import io
|
||||||
|
import os
|
||||||
|
from PIL import Image
|
||||||
|
from imptune.db.models import Icon
|
||||||
|
import imptune.config as cfg
|
||||||
|
import imptune.api.packages as pkg_module
|
||||||
|
|
||||||
|
printer, _ = setup_printer_with_driver
|
||||||
|
|
||||||
|
# Create and upload icon
|
||||||
|
img = Image.new("RGBA", (256, 256), color="red")
|
||||||
|
buf = io.BytesIO()
|
||||||
|
img.save(buf, format="PNG")
|
||||||
|
png_data = buf.getvalue()
|
||||||
|
|
||||||
|
resp = client.post(
|
||||||
|
f"/printers/{printer.id}/icon",
|
||||||
|
files={"file": ("icon.png", io.BytesIO(png_data), "image/png")},
|
||||||
|
)
|
||||||
|
assert resp.status_code == 200
|
||||||
|
|
||||||
|
# Capture staging contents via monkeypatched build_intunewin
|
||||||
|
staged_files = []
|
||||||
|
|
||||||
|
def fake_build(source_dir, setup_file, output_path):
|
||||||
|
staged_files.extend(os.listdir(source_dir))
|
||||||
|
# Write a minimal valid file so endpoint can read it
|
||||||
|
with open(output_path, "wb") as f:
|
||||||
|
f.write(b"FAKE")
|
||||||
|
|
||||||
|
monkeypatch.setattr(pkg_module, "build_intunewin", fake_build)
|
||||||
|
|
||||||
|
resp = client.get(f"/printers/{printer.id}/packages/intunewin")
|
||||||
|
assert resp.status_code == 200
|
||||||
|
assert "icon.png" in staged_files
|
||||||
|
```
|
||||||
|
|
||||||
|
### Test: export succeeds with no icon (icon-free baseline)
|
||||||
|
|
||||||
|
```python
|
||||||
|
def test_intunewin_without_icon_succeeds(client, setup_printer_with_driver, monkeypatch):
|
||||||
|
"""Export succeeds even when no icon has been uploaded."""
|
||||||
|
import imptune.api.packages as pkg_module
|
||||||
|
import os
|
||||||
|
|
||||||
|
printer, _ = setup_printer_with_driver
|
||||||
|
|
||||||
|
def fake_build(source_dir, setup_file, output_path):
|
||||||
|
with open(output_path, "wb") as f:
|
||||||
|
f.write(b"FAKE")
|
||||||
|
|
||||||
|
monkeypatch.setattr(pkg_module, "build_intunewin", fake_build)
|
||||||
|
|
||||||
|
resp = client.get(f"/printers/{printer.id}/packages/intunewin")
|
||||||
|
assert resp.status_code == 200
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## State of the Art
|
||||||
|
|
||||||
|
| Old Approach | Current Approach | When Changed | Impact |
|
||||||
|
|--------------|------------------|--------------|--------|
|
||||||
|
| icon stored as column in Printer | Separate `Icon` model with FK, SHA256-addressed on disk | Phase 5 | Clean separation; icon is optional; no schema migration needed here |
|
||||||
|
| build_intunewin reads arbitrary files | build_intunewin walks entire tmpdir | Phase 5 | Any file placed in tmpdir automatically included in inner ZIP |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Open Questions
|
||||||
|
|
||||||
|
1. **Should missing icon file on disk (orphaned DB record) be silently skipped or 422?**
|
||||||
|
- What we know: `icon_record` can exist in DB but file might be absent (disk corruption,
|
||||||
|
manual cleanup). Current `icons.py` does not handle this case either.
|
||||||
|
- Recommendation: Silent skip (don't copy if `os.path.isfile(icon_src)` is False) —
|
||||||
|
consistent with the optional nature of icons and avoids breaking exports for stale records.
|
||||||
|
|
||||||
|
2. **Is there any Intune-specific requirement for the icon filename or location inside .intunewin?**
|
||||||
|
- What we know: The `.intunewin` package is a deployment container; Intune does NOT
|
||||||
|
read the icon from within the .intunewin file. The icon is set separately in the
|
||||||
|
Intune portal during app creation. The PKG-04 requirement says "included in the
|
||||||
|
.intunewin package" as a convenience/archive — not as an Intune-interpreted artifact.
|
||||||
|
- Confidence: MEDIUM — based on reverse-engineering of .intunewin format. The
|
||||||
|
requirement text "Intune displays it as the app icon" describes the end-state in
|
||||||
|
Intune, not a technical requirement for the .intunewin binary.
|
||||||
|
- Recommendation: Include the icon in the package root as `icon.png` for user
|
||||||
|
convenience (they can extract it when uploading manually to Intune portal).
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Validation Architecture
|
||||||
|
|
||||||
|
### Test Framework
|
||||||
|
|
||||||
|
| Property | Value |
|
||||||
|
|----------|-------|
|
||||||
|
| Framework | pytest (detected: pytest.ini or pyproject.toml implicit) |
|
||||||
|
| Config file | none detected — runs via `python -m pytest` |
|
||||||
|
| Quick run command | `python -m pytest tests/test_packages.py -x -q` |
|
||||||
|
| Full suite command | `python -m pytest tests/ -q` |
|
||||||
|
|
||||||
|
### Phase Requirements -> Test Map
|
||||||
|
|
||||||
|
| Req ID | Behavior | Test Type | Automated Command | File Exists? |
|
||||||
|
|--------|----------|-----------|-------------------|-------------|
|
||||||
|
| PKG-04 | Icon PNG is copied into .intunewin staging dir when icon exists | integration | `python -m pytest tests/test_packages.py::TestIntunewinDownload -x -q` | Partial — test_packages.py exists but no icon-in-package test yet |
|
||||||
|
| PKG-04 | Export succeeds when no icon uploaded | integration | `python -m pytest tests/test_packages.py::TestIntunewinDownload -x -q` | Partial — baseline export tests exist |
|
||||||
|
|
||||||
|
### Sampling Rate
|
||||||
|
|
||||||
|
- **Per task commit:** `python -m pytest tests/test_packages.py -x -q`
|
||||||
|
- **Per wave merge:** `python -m pytest tests/ -q`
|
||||||
|
- **Phase gate:** Full suite green before `/gsd:verify-work`
|
||||||
|
|
||||||
|
### Wave 0 Gaps
|
||||||
|
|
||||||
|
- [ ] `tests/test_packages.py` — add `test_intunewin_includes_icon` test (monkeypatch `build_intunewin`)
|
||||||
|
- [ ] `tests/test_packages.py` — add `test_intunewin_without_icon_succeeds` test
|
||||||
|
|
||||||
|
*(Existing test infrastructure covers the rest — no new files or fixtures needed)*
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Sources
|
||||||
|
|
||||||
|
### Primary (HIGH confidence)
|
||||||
|
|
||||||
|
- Direct code reading: `imptune/api/packages.py` — full `get_intunewin_package()` implementation
|
||||||
|
- Direct code reading: `imptune/api/icons.py` — Icon upload, SHA256 storage path, `cfg.ICONS_DIR`
|
||||||
|
- Direct code reading: `imptune/db/models.py` — `Icon` model, FK to `Printer`, `sha256` field
|
||||||
|
- Direct code reading: `imptune/config.py` — `ICONS_DIR = str(Path(DATA_DIR) / "icons")`
|
||||||
|
- Direct code reading: `imptune/generators/intunewin_builder.py` — `build_intunewin` walks entire `source_dir`
|
||||||
|
- Direct code reading: `tests/conftest.py` — `tmp_data_dir` monkeypatches `cfg.ICONS_DIR`
|
||||||
|
- Direct code reading: `tests/test_packages.py` — existing test patterns (fixtures, monkeypatch style)
|
||||||
|
- Direct code reading: `.planning/STATE.md` — Phase 5 decisions on ICONS_DIR, Icon model, shutil pattern
|
||||||
|
|
||||||
|
### Secondary (MEDIUM confidence)
|
||||||
|
|
||||||
|
- Intune .intunewin format: icon is NOT read from within the package by Intune — icons are
|
||||||
|
set in the Intune portal. Including it in the package is for user convenience only.
|
||||||
|
(Based on reverse-engineering notes in `intunewin_builder.py` comments + svrooij.io reference)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Metadata
|
||||||
|
|
||||||
|
**Confidence breakdown:**
|
||||||
|
- Standard stack: HIGH — all libraries already in codebase, no new dependencies
|
||||||
|
- Architecture: HIGH — single code location change (packages.py), clear insertion point
|
||||||
|
- Pitfalls: HIGH — inner-ZIP test trap verified by reading intunewin_builder.py structure
|
||||||
|
- Test strategy: HIGH — monkeypatch pattern already established in existing test suite
|
||||||
|
|
||||||
|
**Research date:** 2026-04-10
|
||||||
|
**Valid until:** Stable — no moving parts (stdlib + existing codebase only)
|
||||||
Reference in New Issue
Block a user