Fix preset hooks dropped on load; presets remember hook enablement

[dngrtech]

Summary

• Instance create (POST /instances/) and instance config update (PUT /instances/<id>/config) never copied a draft's user-hooks/ directory when applying a preset — hook .so files from an imported/loaded preset were silently discarded, even though scripts/configs/factories copied correctly.
• Presets never recorded which hooks were enabled, so hooks that did land on an instance always showed up disabled in the Hooks tab.
• Adds enabled_hooks.json per preset (mirrors the existing checked_plugins.json/checked_factories.json pattern): captured on Save from the source instance's currently-enabled hooks, included in export/import, and applied to ld_preload_hooks on instance create/update — fully replacing the target instance's hook selection to match the preset, filtered to hooks that actually exist on disk after the copy.
• Frontend (EditInstanceConfigModal.jsx, AddInstanceForm.jsx) carries enabled_hooks through load → save/create.
• Docs updated: architecture.md, technical.md, api_reference.md, user-facing preset/hooks pages.
• Version bumped to 1.13.6 (VERSION, docs/user/version.json, docs/user/releases.md).

Test plan

• pytest tests/ — 991/991 passing, including 18 new regression tests covering: draft user-hooks copy on instance create/update, enabled_hooks validation/round-trip through preset create/update/import/export, filtering to hooks present on disk, and replace-on-load semantics.
• pnpm lint in frontend-react/ — no new lint errors in touched files.
• Manual browser verification of the full flow (import preset → Load Preset onto an instance → Save → Hooks tab shows the hook enabled) — not yet done in this session.

[github-actions]

PR Review — v1.13.6: Preset Hook Files & Enabled-Hooks Selection

Summary

Adds enabled_hooks support to presets: saves which LD_PRELOAD hook files were enabled on an instance when a preset is captured, and replays that selection when the preset is loaded onto another instance. Mirrors the existing checked_plugins/checked_factories replace-on-load pattern. Also fixes the prior bug where user-hooks/ was dropped when a draft was committed to an instance.

---

Strengths

• Pattern consistency (ui/routes/preset_api_routes.py:825–852): _read_preset_enabled_hooks / _write_preset_enabled_hooks follow the exact same shape as _read_preset_checked_plugins / _write_preset_checked_plugins. No new patterns introduced.
• Correct filter-before-apply semantics (ui/routes/instance_routes.py:677–683): _filtered_enabled_hooks uses os.listdir() after the draft copy step, so only hooks that actually landed on disk can be enabled. Non-existent filenames are silently dropped rather than causing an error, which is the right call for preset portability.
• Path traversal resistance (incidental but real): os.listdir() returns bare filenames, so a client submitting ['../../etc/evil.so'] would find no match in on_disk and the entry would be dropped. The filter makes traversal impossible in practice.
• Backwards compatibility (docs/api_reference.md:71): null enabled_hooks on a legacy preset correctly leaves the target instance's hook enablement untouched — documented clearly and tested.
• Best-effort hook capture on preset save (frontend-react/src/components/instances/EditInstanceConfigModal.jsx:253–257): wrapping fetchInstanceHooks in a try/catch so a failed hooks fetch doesn't block the preset save is the right call. Silent omission is better than a failed save.
• Test breadth: Covers create-with-draft, create-with-enabled_hooks filter, update-with-draft, update-with-enabled_hooks-replace, preset CRUD, import with/without enabled_hooks.json, validation rejection, and normalization filtering.

---

Issues

Critical (Must Fix)

None.

Important (Should Fix)

1. _validate_enabled_hooks_payload is duplicated with inconsistent signatures

• ui/routes/instance_routes.py:667–674 — returns (error_message, status_code) tuple
• ui/routes/preset_api_routes.py:806–815 — returns error_string | None

Both validate the same rule (list of .so filenames). This will diverge over time (e.g., if path separators are added to one but not the other). The instance-routes version should call through to a shared helper, or the preset-routes version should be extracted to a shared module.

2. _read_preset_enabled_hooks doesn't type-check the decoded JSON

ui/routes/preset_api_routes.py:825–838: if enabled_hooks.json contains a non-list value (e.g., a string or dict — possible from a manually edited or corrupted file), the function returns it unvalidated. Downstream callers that call .join() on it would raise a TypeError at runtime. A simple if not isinstance(data, list): return None guard after json.load would prevent this.

3. Missing test: enabled_hooks applied to an update without a draft_id

The path in manage_instance_config_api where enabled_hooks is provided but draft_id is absent isn't covered by any test. In that case the filter runs against whatever already exists in instance_user_hooks_dir — any filenames that don't already live there are silently dropped, effectively clearing the selection. This is arguably the correct replace-on-load behaviour, but without a test it's easy to accidentally break.

Suggested test name: test_update_config_enabled_hooks_without_draft_filters_to_existing_files

Minor (Nice to Have)

4. Explicit path-separator rejection in _validate_enabled_hooks_payload

ui/routes/instance_routes.py:669–672 / ui/routes/preset_api_routes.py:808–814: The .so suffix check doesn't reject names containing / or ... In practice this is safe because _filtered_enabled_hooks uses exact-match against os.listdir() bare filenames, but the intent is clearer and the code is more defensible with an explicit check:

not any('/' in h or h.startswith('..') for h in enabled_hooks)

5. enabled_hooks.json not mentioned in the ZIP export manifest field list

ui/routes/preset_api_routes.py:95–99: the _preset_export_manifest already sets 'enabled_hooks': True in the contents dict, so it will appear in the exported manifest. This is correct. However the API reference section for export/import (docs/api_reference.md) doesn't explicitly state that enabled_hooks.json is included in the archive. Minor omission compared to how checked_plugins.json and checked_factories.json are mentioned.

6. instance_user_hooks_dir assigned but unused in the no-draft branch of add_instance_api

ui/routes/instance_routes.py:407: instance_user_hooks_dir is declared before the if draft_id: block. In the else branch (no draft), no hooks are copied, os.makedirs runs (line ~432), then _filtered_enabled_hooks is called against the freshly-created empty directory. This is correct, but a comment explaining why enabled_hooks filtering still runs after the else-branch (answer: to honour an enabled_hooks payload even when there's no draft) would help readers.

---

Assessment

Ready to merge? Yes, with the caveat that issues 1–3 above are worth fixing soon (they're not blocking bugs but will cause technical debt and one plausible runtime error on a corrupted file).

Reasoning: The core logic is correct — the filtering model is safe against path traversal, the backwards-compatibility semantics are sound, and test coverage is solid for the main flows. Issues 1 and 2 are quality/robustness gaps rather than correctness bugs, and issue 3 is a missing test on a path that the code handles correctly.