"Audit of tooling and host-facing contracts — SDK (Studio snapshots, typed commands, StorageUtil compat), debug server + protocol + byro-dbg, debug-ui panels, launcher/boot-request/settings-io handoff, install detection, byro-detect, texture-upscale"
Instal·la
npx skillscat add matiaszanolli/byroredux/claude-commands-audit-tooling Instal·la des del registre de SkillsCat.
Tooling and Host-Contract Audit
Read _audit-common.md and _audit-severity.md for shared protocol.
Surfaces a modder, player or developer touches instead of the engine: a TCP command port evaluated against the live World, the renderer-independent SDK, the launcher's boot/settings handoff and the tool CLIs. Not here: crates/mod-runtime (/audit-safety Dim 8); debug-server locks/threads (/audit-concurrency Dim 7); the egui pass (/audit-renderer); VDF/ACF parsing (/audit-parsers); save schema (/audit-save).
| Area | Code (docs) |
|---|---|
| sdk | crates/sdk/src/ (identity, manifest, service.rs, studio.rs, compatibility/); adapters byroredux/src/studio_host.rs, byroredux/src/commands/studio.rs, byroredux/src/extensions/ (docs/engine/sdk-v0.1-development-plan.md, docs/engine/sdk-v0.1-next-action-plan.md) |
| debug | crates/debug-server/src/{listener,system,evaluator,registration}.rs, crates/debug-protocol/src/{lib,wire,registry}.rs, byroredux/src/debug_load.rs, tools/byro-dbg/src/, crates/debug-ui/src/ (panels also host the player menu, inventory, HUD) (docs/engine/debug-cli.md) |
| launcher | crates/boot-request/src/, crates/settings-io/src/, crates/core/src/atomic_file.rs, tools/byro-launcher/src/, byroredux/src/boot/cli.rs (expand_boot_request) (docs/engine/launcher.md) |
| detect | crates/game-detect/src/{catalog,steam,profiles,validate,overrides}.rs, crates/core/src/ecs/game_profiles.rs, assets/debug_profiles.toml, tools/byro-detect/src/main.rs (docs/engine/launcher.md §3) |
| tools | tools/texture-upscale/src/ (tools/nifskope is vendored, not first-party) |
Parameters (from $ARGUMENTS)
--focus <dimensions>: dimensions to run (default all 6).--area <name>: one area.- Delta-first: run each
First stepandgit log --since=<last AUDIT_TOOLING date> -- <Paths>; deep-audit only dimensions whose Paths changed.
Extra Per-Finding Fields
- Dimension: Debug Trust Boundary | Protocol & Registry | SDK Contract | Boot Handoff & Persistence | Install Detection | Tool CLIs
- Exposure: who can reach it (any local process / launcher user / modder / developer only). Unauthenticated local file write or command execution in a stock build = HIGH; developer-only tool defect = LOW–MEDIUM; user data loss in a crash window = MEDIUM.
Phase 1: Setup
mkdir -p /tmp/audit/tooling;gh issue list --repo matiaszanolli/ByroRedux --limit 200 --json number,title,state,labels > /tmp/audit/tooling/issues.json; dedup per_audit-common.md.- Read the newest
docs/audits/AUDIT_TOOLING_*.mdif any. cargo testfor-p byroredux-{sdk,debug-server,debug-protocol,debug-ui,boot-request,settings-io,game-detect}and-p byro-{launcher,detect,texture-upscale}; record counts. Do not launch a windowed engine to exercise the debug server (feedback_no_parallel_engine_launch).
Phase 2: Dimensions (max 3 concurrent agents)
Dimension 1: Debug-server trust boundary
Paths: crates/debug-server/src/{listener,system,evaluator}.rs, crates/debug-protocol/src/{lib,wire}.rs, byroredux/src/main.rs (start site, debug_server_allowed), byroredux/src/debug_load.rs (cell-load path confinement), byroredux/Cargo.toml.
First step: grep -n 'bind\|MAX_\|fs::write\|create_new\|from_secs' crates/debug-server/src/*.rs crates/debug-protocol/src/wire.rs.
Facts (2026-10-05): bind is the literal ("127.0.0.1", port); no authentication; debug-server is a default feature of byroredux, started from main.rs on BYRO_DEBUG_PORT (default 9876). Debug builds always bind; release builds bind only with BYRO_DEBUG_SERVER=1 (debug_server_allowed, guard release_debug_server_requires_explicit_opt_in; 63c0aee3b, #4752), so the exposure is every local process on a debug build or an opted-in release. Caps: MAX_QUEUED_COMMANDS 64, MAX_CONCURRENT_CLIENTS 8 (#3449), 300 s read timeout, 16 MB frame (MAX_MESSAGE_SIZE, enforced by both encode and decode), MAX_WALK_DEPTH 64. The timeout pair lives in crates/debug-protocol/src/lib.rs: COMMAND_RESPONSE_TIMEOUT 30 s (server wait) < CLIENT_READ_TIMEOUT 35 s (byro-dbg read), ordering pinned (cc5468813, #5140).
Guards: try_enqueue_rejects_when_queue_is_full, connections_past_the_cap_are_refused, occupied_port_is_reported_before_a_handle_is_returned (listener.rs); rejects_oversized_message, encode_rejects_response_payloads_over_the_protocol_limit (wire.rs); cancelled_screenshot_does_not_skip_same_frame_command_drain, cancelled_command_is_answered_without_evaluation, rejects_absolute_and_parent_paths (system.rs); walk_depth_is_capped_even_for_untrusted_requests (evaluator.rs); output_path_is_confined_to_the_dump_directory (commands/assets.rs); cell_load_paths_are_confined_to_startup_roots, confinement_rejects_a_symlink_out_of_the_root (debug_load.rs). No test pins the loopback address: a 0.0.0.0 bind would pass every guard (known-open #5149). Known-open #5147: default output names are not unique under create_new (a second default tex.dump → tex_dump.png, two screenshot_<secs>.png in one second) — dedup, don't re-file.
Checklist:
- Capability surface = the whole console:
eval_requestdispatches every registered dotted command;SetFieldwrites any registered component field. File reads are confined:LoadNifloose paths and theLoadInteriorCell/LoadExteriorCellESM/master/archive paths all go throughconfine_to_rootsagainststartup_asset_roots()(relative, normal components, canonicalised inside a startup--esm/--master/--bsaroot; one rejected path rejects the whole load; 554259315, #5165). File writes are confined:write_screenshot(system.rs) takes a single filename underscreenshots/withcreate_new(no overwrite), andtex.dumpgoes throughsafe_tex_dump_path(byroredux/src/commands/assets.rs,texture-dumps/). Check a new path-taking request goes through the same helpers and that no other command reads or writes a client-chosen path. Ask whether each mutating or file-writing command (tex.dump, screenshot, dumps) should be reachable unauthenticated in a player build; size by Exposure. - Evaluator bounds:
parse_exprruns on the engine thread in the Late exclusive (depth caps incrates/papyrus/src/parser/);WalkEntity.max_depthis clamped bybounded_walk_depth; other entity/Valueresponses are unbounded untilencoderefuses them. - An over-limit response is replaced by an error response in
handle_client(theInvalidDataarm) rather than a torn frame — verify the fallback itself cannot exceed the limit and that the command's side effects already ran. A decode error (even an unknown variant) drops the connection: intended? - The drain system runs
evaluateinline, so a slow query stalls a frame; is there a per-request cost bound? Lock order is/audit-concurrency(debug_evaluator_acquires_locks_in_canonical_order). Abandoned requests (PendingCommand.cancel, #1007) and theSCREENSHOT_OWNER_*claim must release on client timeout; the drain answers an already-cancelled command with an error instead of evaluating it, and keeps running while a native pause/settings/inventory page gates the scheduler (Scheduler::run_exclusive_named, 00115f7a5, #5141), naming the system through the sharedDRAIN_SYSTEM_NAMEconstant (crates/debug-server/src/system.rs) with adebug_asserton the returned bool (8a9989070, #5292; source-scan guard inbyroredux/src/scheduler_access_tests.rs) — a paused frame path that skips the drain, or a hand-typed copy of the name, re-creates the burst-on-unpause replay.
Dimension 2: Protocol and registry completeness, wire compat
Paths: crates/debug-protocol/src/{lib,registry}.rs, crates/debug-server/src/registration.rs, tools/byro-dbg/src/{main,display,tui}.rs, docs/engine/debug-cli.md.
First step: grep -c 'register_component::<' crates/debug-server/src/registration.rs; diff against inspect-derived components (grep -rn 'cfg_attr(feature = "inspect"' crates/core/src).
- Re-derive counts:
docs/engine/debug-cli.mdmust report N in both the "Currently registered (N components)" heading and thebyro> componentsexample transcript (debug_cli_component_counts_match_the_registryrequires ≥2 occurrences since f2d0b6ea0, #5417). N = the production-textregister_component::<calls plus the hand-builtregister_faction_reputationdescriptor: 71 + 1 = 72 on 2026-10-12 (67 → 71: the M42 Eat/Sleep trio plusEditorPlacement, #5391; 71 → 72:CreatureActor, 9bb8bc475 #5495). A whole-filegrep -calso prints 72, but only because the test's own needle adds a line; count production text. - Completeness guards (
registration.rs):every_inspectable_component_is_registeredwalkscrates/core/srcfor every type carrying theinspectderive and implementingComponentand fails unless each is registered (977793679, #4755, took the registry 49 → 67), plus the olderevery_ai_procedure_behavior_component_is_registered(#4063). Remaining gap class: components outsidecrates/core/src(engine-crate or other-crate components withinspectderives) the walk does not see — enumerate those and judge which hold user-visible state. - Wire: 4-byte BE length + JSON, serde-tagged
DebugRequest; no version or handshake field, so engine andbyro-dbgmust come from one workspace commit. Guards:round_trip_request,round_trip_response(wire.rs). - A new
DebugRequest/DebugResponsevariant must reach the evaluator dispatch,parse_shorthandanddisplay.rsinbyro-dbg, andhandle_response/variant_nameintui.rs; a_ =>arm in any of them hides a missing variant.byro-dbgsets its socket read timeout toCLIENT_READ_TIMEOUT(configure_read_timeout, guardread_timeout_exceeds_the_server_response_timeout); the TUI net thread treats a read timeout as "engine busy", not a disconnect. Its only other test isparses_common_shorthand_requests— variant coverage indisplay.rs/tui.rsis untested. The never-constructedDebugResponse::Screenshotvariant was deleted (6f38b335d, #5150); a dead variant still matched by the client is the same class. - Console docs vs the dispatch table (
registry.register(inbyroredux/src/commands/mod.rs): every documented command exists; every registered one is documented or deliberately omitted.set_fieldnever panics on bad JSON.
Dimension 3: SDK contract surface
Paths: crates/sdk/src/, byroredux/src/{studio_host.rs,commands/studio.rs}, crates/debug-ui/src/panels.rs, the two SDK plan docs.
First step: cargo tree -p byroredux-sdk --depth 1 and grep -rn 'byroredux_core::' crates/sdk/src.
- Facts (2026-09-19):
#![forbid(unsafe_code)]; deps arebyroredux-core(onlymath::Vec3used),semver,serde,thiserror;StudioSessionispub(crate)inbyroredux/src/studio_host.rs, not the SDK. - Plan gates: an explicit checked dependency list (no renderer/UI/platform/archive/Wasmtime), "Rust/WIT schema drift fails CI" (
crates/mod-runtime/wit/host.wit), warning-free docs, and "no supported SDK value exposes an ECS ID or engine pointer". Find the test or CI step for each (none found for the dependency list, 2026-09-19). The plan isIn progress: report only gates the docs call delivered. - API leak: a
pubSDK item exposing an engine type (Entity,World,FormId, glam beyondVec3) instead of a stable id (ObjectId,AssetId,EntityRef,FormRef);pubvalue-type fields are unvalidated by construction. - Actor-value projection:
ActorValueStatehas noset_overrideslot, soextensions/capture.rsfolds the #5412 layer intobase(base + set_override) and a mod reads back its own routedSetBase(627149b62, #5508); a projection that drops the layer, or re-sums the layers by hand instead ofActorValue::undamaged(), is the regression (byroredux/src/extensions/tests.rs). - Validation lives in the host:
valid_transform/valid_materialinstudio_host.rsreject non-finite values, while the clamps (sanitize_transform,sanitize_material) sit in the egui panel; every non-UI command path (extensions,commands/studio.rs) must reach the host validators. - Snapshot determinism: ids come from a sorted canonical set (plan Phase 1); check no
HashMapiteration order reaches aStudioSnapshotor serialized output. Identity guards:object_ids_reserve_zero_and_assign_one_based_ordinals,namespaced_ids_reject_path_and_whitespace_spoofing,capability_sets_are_validated_and_deduplicated. - Every
*_CAPABILITYconstant inservice.rsis checked at a real call site (count by grep); an unchecked one is a grant without a gate (enforcement is/audit-safetyDim 8). - StorageUtil / list verbs (
crates/sdk/src/compatibility/storage_util/—mod.rsscalar + codecs,list.rs,declarations.rs): bounded sizes and deterministic order per the plan's compatibility table; every provider alias maps to a route, policy and fixture; the unsupported boundary (file-backed state, JContainers JSON/Lua, menu mutation) fails explicitly, never no-ops.
Dimension 4: Launcher boot handoff and persistence
Paths: crates/boot-request/src/, crates/settings-io/src/, crates/core/src/atomic_file.rs, crates/game-detect/src/overrides.rs, tools/byro-launcher/src/, byroredux/src/boot/cli.rs, byroredux/src/cli_args.rs.
First step: grep -n 'fs::write\|atomic_write\|CONTRACT_VERSION\|SETTINGS_VERSION' crates/boot-request/src crates/settings-io/src crates/game-detect/src tools/byro-launcher/src.
Guards: version — an_unknown_contract_version_is_refused_by_version_not_by_field, a_missing_version_is_a_mismatch_not_a_default; precedence (docs/engine/launcher.md §2.4) — explicit_command_line_flags_suppress_their_request_counterparts (crates/boot-request/src/args.rs), boot_request_seam_tests in boot/cli.rs; settings — saving_a_partial_registry_preserves_keys_it_does_not_know, stale_and_invalid_values_do_not_block_valid_siblings, a_save_does_not_revert_another_frontends_newer_value, save_refuses_when_the_existing_file_does_not_parse, save_refuses_to_downgrade_a_newer_version_file, the_shipped_presets_apply_cleanly_to_the_real_builtin_registry; durability — atomic_file.rs::{failed_rename_removes_the_temp, clobber_guard_accepts_the_writer_shape_and_rejects_fallbacks}, save_has_no_nonatomic_clobber_fallback (boot-request, settings-io), merge_uses_the_shared_atomic_file_writer + merging_preserves_comments_and_formatting (overrides.rs); supervision — the_engine_is_invoked_with_the_boot_request_path_and_profiles_env, the_log_tail_is_bounded_and_keeps_the_most_recent_lines.
Checklist:
- CLI flag reads (
byroredux/src/cli_args.rs, owned here): only the space-separated--flag valueform is accepted, andparse_string_argwarns on the--flag=valuespelling instead of silently dropping it (the_equals_form_is_reported_rather_than_silently_dropped). Plain value lookups (--esm,--cell,--grid,--wrld,--radius,--kf,--rotation-mode,--bench-frames) and the repeatable--mastercollectors go throughparse_string_arg/parse_all_string_args(parse_all_string_args_collects_repeats_and_skips_flag_values) andesm_discovery_root, never a hand-rolledargs.iter().position(..)(d8a6e58bf, #5598). The raw-by-design flags (--swf,--bsa,--menu,--studio,--cmd, ...) error on a missing value, whichparse_string_arg'sNonewould weaken; a new flag reader should pick one deliberately. atomic_write(unique hidden temp sibling -> fsync -> read-back -> rename -> parent fsync, #3472) is the durability contract, used by all three writers:BootRequest::save,RootOverrides::merge_into_file, settings-iosave_to_path. The non-atomic "Windows rename"fs::writefallback is gone (2e95f0bbf, #5143) and any staging error removes the temp (69fd54fb2, #5163); each writer is pinned by the shared structural scanatomic_file::assert_no_clobber_fallbackover production text only (#5164). A fallback, a secondatomic_writecall, or anfs::write/fs::renamereappearing in a writer is the regression.merge_into_filenow edits atoml_edit::DocumentMut, touching only changed[roots]entries and skipping the write when nothing changed (7f2cee3e8, #5166).- Version policies:
boot-requestrefuses a mismatch;settings-ioload logs and "attempts compatible values", while save refuses a newer-version file (no silent downgrade), copies an unparseable file aside as<name>.badand refuses, and refuses a non-NotFoundread error (2e95f0bbf, #5144). - Lost updates: there is still no file lock, but
SettingsPersistencekeeps a per-frontend baseline (shared through anArcacross clones) andsave_to_pathwrites only ids whose value moved off it; the launcher reloads on opening Settings and on engine exit (c9f95283a, #5162). Check a new save path goes through the baseline, not a full-registry write. - Precedence: settings path is
SETTINGS_PATH_ENV, then the platform config dir, then a cwd fallback (discover_settings_path); games root is--games-root,BYROREDUX_GAMES_ROOT, thenDEFAULT_GAMES_ROOT, one developer's path (crates/game-detect/src/profiles.rs) — a player's machine silently resolves to a missing directory; does anything say so?discover_settings_path's sibling for profiles isuser_profiles_path()(crates/game-detect/src/profiles.rs, one per-user path shared by launcher,byro-detectand the engine);$BYRO_PROFILESredirects every per-user layer (profile merge and both[roots]passes) and the launcher exports it on the engine it spawns, so a custom--profilesfile stays self-consistent (322c36626, #5294) — a private~/.byroredux/profiles.tomlcopy is the regression. 61ad29da3 (#5476) finished it: the[defaults]layer (game,games_root,light_atten_*) now runs throughselected_user_path()too (ordered_config_paths_with), the launcher'sLauncherState::load/refreshandbyro-detectbuild their registry withload_with_user_path(Some(&profiles_path))(notload_default()), and a relative--profilesis made absolute before the engine spawns withcurrent_dir(engine.parent()). A path that still reads~/.byroredux/profiles.tomldirectly, or a launcher registry built from the process's own home, is the regression. boot.toml paths are absolute and never~-expanded (docs/engine/launcher.md§2.2) whileprofiles.rsexpands~/on roots — check they agree. - The launcher must run when the engine cannot (no Vulkan):
cargo tree -p byro-launchermay showash(pre-flight probe) but not the renderer,byroredux-uior anywgpu(eframe runs onglow;cargo tree -p byro-launcher -i wgpuempty after the 2026-10 egui 0.36 / wgpu 30 bump,docs/engine/launcher.md);a_directory_with_the_engine_name_is_not_the_engineguardslocate_engine.
Dimension 5: Install detection and profile registry
Paths: crates/game-detect/src/{catalog,steam,profiles,validate}.rs, crates/core/src/ecs/game_profiles.rs, byroredux/src/game_profiles.rs (re-export shim), assets/debug_profiles.toml, tools/byro-detect/src/main.rs.
First step: cargo test -p byroredux-game-detect and git log --since=<last report> -- crates/game-detect assets/debug_profiles.toml.
- Steam only today (
~/.steam/*,~/.local/share/Steam, Flatpak, macOS, WindowsProgramFiles*; the registry probe is marked "still to do"); GOG/Epic and Wine prefixes are not probed — a Wine install is reachable only via a manual[roots]entry, whichbyro-detectsays when nothing is found. A manifestinstalldirmust passis_bare_install_dir(non-empty, not./.., no/,\or:; aad5dfd75, #5016;only_a_bare_directory_name_is_a_valid_installdir) — aPath::is_absolute-style check reappearing misses Windows\X/D:Xshapes. - Alternate releases (
[[profiles.<key>.releases]], chosen by files on disk viaGameProfileEntry::for_data_dir): guardsan_alternate_release_is_validated_against_its_own_archives,the_primary_release_wins_when_its_archives_are_all_present,a_release_replaces_only_the_lists_it_names. The launcher validator, the engine'sexpand_game_profile_args(byroredux/src/boot/cli.rs) andstudio_host.rsmust all go throughfor_data_dir, not a private copy; sibling rules come frombyroredux_bsa::numeric_sibling_paths. - Env-var spelling: canonical is
BYROREDUX_<GAME>_DATA(crates/plugin/src/esm/test_paths.rs, e.g.BYROREDUX_SKYRIMSE_DATA).63c0aee3b(Session 91) unified the Skyrim SE split; the legacy BYROREDUX_SKYRIM_DATA survives only as a repo-variable fallback in.github/workflows/playable-smoke.yml(#4760 closed; its Rust/script readers are gone). A new reader of the old name is a regression of that unification (#3741 was the earlier same-class failure). validate.rs: a zero-byte or truncated.esm/ archive must fail at the rightSeverity, not pass and then abort the engine.esm_opensreads only a bounded 1 MiB TES4 prefix and fails closed on an over-declared header (d6b495f2a, #5145); a whole-file read on the launcher UI thread is the regression.byro-detect: exit failure when nothing is found, text output only,--writerecords[roots]via the formatting-preserving merge (Dim 4); arguments parse strictly (the launcher's--profilestoo, #5294) — an unknown flag, a missing/flag-shaped or repeated--profilesvalue is a usage error (exit 2, nothing written; ad6967fa9, #5167;rejects_an_unknown_flag) — check--helpmatches behaviour.
Dimension 6: Tool CLIs
Paths: tools/texture-upscale/src/.
First step: cargo test -p byro-texture-upscale.
- texture-upscale (
MANIFEST_VERSION1,Manifest::validate):output_png_pathfollowsvalidate_asset_path(rejects absolute,.,.., empty segments); overwrite refused without--overwrite;--dry-runcreates nothing (dry_run_checks_the_plan_without_creating_output_or_launching_the_model); dimensions overflow-checked (scaled_dimensions_reject_overflow); disk space checked first (insufficient_space_is_rejected_before_writes). - Trust: the upscaler is
Command::new(&manifest.upscaler.program)with.args(..)(no shell), so the manifest is executable trusted input; README/--helpmust say so. - Output collisions and per-set failure are fixed (#4761/#4762):
validateclaims each source's output PNG across every set (manifest_rejects_sources_that_share_an_output_png); a bad set is reported, not batch-fatal (one_bad_set_does_not_abort_the_batch), while an unlaunchable upscaler still aborts (an_unlaunchable_upscaler_still_aborts_the_batch). Check:SourceStack"later overrides earlier" matches the engine's last-wins order (/audit-parsersDim 6).
Phase 3: Merge and Cleanup
Write docs/audits/AUDIT_TOOLING_<TODAY>.md: Executive Summary (findings by severity, areas swept), deduplicated Findings, and an Exposure Matrix (surface x {reachable by, authenticated, size-capped, persistence atomic}). Cross-audit dedup: debug-server locks /audit-concurrency Dim 7; sandbox enforcement /audit-safety Dim 8; save schema /audit-save; VDF/archive parsing /audit-parsers. Then rm -rf /tmp/audit/tooling and suggest /audit-publish docs/audits/AUDIT_TOOLING_<TODAY>.md (labels: tech-debt for debug-server/launcher/tools, which have no label of their own; doc-rot, test-gap as fitting).