refactor(skills): export install safety primitives

[asto18089]

Summary

Pinvou's plugin/skill importer currently keeps a re-implementation of the skill-install safety checks whose header says it is kept ``aligned with the engine's install.rs by comment'' (checked_zip_entry_path and friends in `pinvou3-app/src-tauri/src/features/marketplace/plugin_import.rs`). Nothing enforces that alignment — and the label is partly inaccurate: the zip sanitizer's verdicts were never point-identical with the engine's (it normalizes and accepts where the engine's `is_safe_path` rejects), and the app's name validators are deliberately stricter allowlists. Pinvou's follow-up (draft PR Pinvou/pinvou-agent#653) adopts the point-equivalent checks (`add_entry_size`, `INSTALLED_FROM_MARKER`) and registers the deliberately divergent judgments (traversal, symlink face, name allowlist, legacy marker format) with pin tests instead of restating them as "aligned" copies.

This PR makes the minimal engine surface public so embedders can reuse the real checks — the same reuse path the in-tree plugin installer (plugins::install) already took via pub(crate) for is_safe_path, validate_skill_name_segment, and the marker pair. Follow-up commits on this branch route the plugin installer's last inline link predicate through the shared entry_type_is_link and calibrate the contract docs. The plugin tarball's size gate keeps its own accumulation because it must raise PluginInstallError::OversizedBundle rather than InstallError::OversizedTarball — a differently-typed error, not a divergent judgment.

Exported surface (codewhale_tui::skills::install)

• is_safe_path(&Path) -> bool — entry-path safety predicate: rejects path traversal (..), absolute paths, and Windows prefix/root components. Was pub(crate).
• entry_type_is_link(tar::EntryType) -> bool — symlink/hard-link entry rejection judgment, extracted unchanged from being inline in both scan_tarball and extract_into (new; both passes now call it, and the plugin installer's pass two as well).
• add_entry_size(total, added, max_size) -> Result<u64> — running uncompressed-size cap (saturating, >-strict), extracted unchanged from being inline in both passes: scan measures header-declared sizes, extract re-measures bytes actually read so a forged header cannot skip the bound (new).
• validate_skill_name_segment(&str) -> Result<&str> — single path-safe segment validation for user- or archive-supplied names. Was pub(crate).
• INSTALLED_FROM_MARKER — already pub; doc now states the embedder contract.
• InstalledFromMarker — the .installed-from JSON schema struct, now pub with pub fields (and source_checksum()), so marker readers deserialize through the shared schema instead of redefining it.

The module itself stays at its existing visibility; only these items are public. The contract names are additionally reachable through the short skills::* re-export list.

Verification

• cargo fmt --all — clean.
• CARGO_TARGET_DIR=/tmp/cw-up/target RUST_MIN_STACK=32212254 cargo test -p codewhale-tui --lib skills::install:: -- --test-threads=4 — test result: ok. 22 passed; 0 failed (includes 2 new tests: entry_type_is_link_accepts_only_link_entries — now pinning every non-link tar::EntryType variant — and add_entry_size_rejects_entries_past_the_cap; traversal/absolute-path and name-segment vectors already existed and still pass).
• CARGO_TARGET_DIR=/tmp/cw-up/target RUST_MIN_STACK=32212254 cargo test -p codewhale-tui --lib plugins::install:: -- --test-threads=4 — test result: ok. 24 passed; 0 failed (in-tree consumer of the widened surface).
• CARGO_TARGET_DIR=/tmp/cw-up/target cargo clippy -p codewhale-tui --lib — clean.

No-Issue: visibility-only export of existing install-safety checks for embedder reuse (app-side chop tracked as the parent repo PR Pinvou/pinvou-agent 653).

[asto18089]

Stacked consumer: Pinvou/pinvou-agent#653 pins upstream/integration (= pinjou3-clean + these upstream contributions) and chops the app-side duplicates onto the new APIs. That parent PR is draft until this PR merges — its verify-public-submodule gate requires the parent gitlink to equal the public pinjou3-clean head. After merge, the pin re-points to the branch head and the chop lands.

[asto18089]

Review (fresh, from scratch)

Verdict: approve-grade content — no blockers, no behavior change (proven), no smuggled changes. One MAJOR wording fix requested in the contract docs/PR body (no code change required); minors listed below. After the wording fix this is merge-ready. All required gates green (link was red for a missing No-Issue: line — appended to the body).

What was verified

• Diff integrity: exactly one file (crates/tui/src/skills/install.rs, +110/−22); every hunk serves the export (visibility, contract docs, two helper extractions, two tests). No logic drift anywhere: error texts, OversizedTarball construction, strict-> boundary, saturating_add, if let Ok(size) guard, and evaluation order in scan_tarball/extract_into are byte-identical to base; serde attributes untouched; the two #[allow(dead_code)] removals are warning-free (pub fields in a pub module; verified under -D warnings).
• Tests are real: 22 (skills::install) + 24 (plugins::install) pass, fmt/clippy clean — reproduced. Mutation testing: dropping the hard-link arm of entry_type_is_link, >→>= in add_entry_size, and saturating_add→wrapping_add were each killed by exactly the intended new test, and only that test.
• Premise is real: the in-tree plugins::install reuse via pub(crate) pre-exists (this PR only widens visibility); the stacked consumer refactor: adopt engine skill and safety primitives pinvou-agent#653 exists in draft and genuinely adopts add_entry_size (with equivalence pin tests) and INSTALLED_FROM_MARKER.

MAJOR — contract wording overclaims adoption/unifiability (docs + body, no code change)

1. The module doc says embedders should call these "instead of maintaining comment-aligned copies of the checks (the in-tree plugin installer already does ...)". True for is_safe_path / validate_skill_name_segment / the marker pair, but false for the two new helpers: plugins/install/tarball.rs:158 still inlines the link predicate (adoptable), and tarball.rs:79-81/184-186 cannot adopt add_entry_size as-is because it hardcodes InstallError::OversizedTarball while the plugin path must raise PluginInstallError::OversizedBundle. Also crate::plugins::install is a private module — a dead-end pointer for external rustdoc readers.
2. The body's "Pinvou will switch its aligned copy to consume this exported API instead" overstates what the follow-through can be. The app copy was never verdict-aligned with the engine for 4 of the 6 exports — verified in zip 8.6.0 source: enclosed_name is normalize-and-accept (drive/root components skipped at depth 0, mid-path .. pops), while is_safe_path strict-rejects; the symlink judgment's zip face is unix-mode bits (a tar::EntryType predicate is unusable there); the app's name validators are deliberate allowlists ([a-zA-Z0-9-_]{1,64}) far stricter than validate_skill_name_segment; marker formats already diverged (app legacy plain-text, never writes; engine JSON). feat(execpolicy): layered permission rulesets — defaults+agent+user (closes #415) codewhale-hq/Codewhale#653 does the right thing — adopts the two point-equivalent items and registers the divergences with pin tests — but this PR's contract should describe that shape honestly rather than implying full unification of an "aligned" copy.

Suggested fix (wording only): in the module doc, replace the parenthetical with something like "(the in-tree plugin installer consumes is_safe_path, validate_skill_name_segment, and the marker pair; archives with different verdict semantics — e.g. zip's normalize-and-accept sanitizer — register their divergence instead of restating the checks)"; in the body, one sentence noting that codewhale-hq#653 adopts the point-equivalent subset and pins the intentionally divergent judgments. Optionally (separate decision) convert tarball.rs:158 to call entry_type_is_link — note that alone doesn't make the "already does" claim true for add_entry_size, so the doc softening is the real fix.

Minors (non-blocking)

• skills/mod.rs has a re-export list "kept for documentation parity and downstream consumers"; none of the six newly-public names were added, and the app-side habit is the short skills:: path. Add them or amend the comment.
• InstallError's doc still says it "is used internally so tests can pattern-match" — it is now part of the embedder contract (the new test demonstrates the downcast_ref pattern); consider refreshing that sentence and adding it to the module-doc contract bullet.
• InstalledFromMarker schema-history doc: v1 also wrote url (see the audit fixture), v2 additionally writes registry_version: null; and since the doc tells embedders to deserialize through this struct, add that installed_name is intentionally not modeled (unknown-field skipping drops it).
• source_checksum()'s v1 fallback leg is not empty-filtered (can return ""); fail-safe in both callers (spurious re-extract only), but one doc clause would help.
• "single source of truth" / "cannot drift" overpromise — nothing (lint/test) compels embedders to call these; convention-level wording is more accurate.
• entry_type_is_link is the one export whose named consumer (zip-based) cannot call it; fine as in-tree dedup, but consider whether pub is warranted before a tar-based embedder exists (or pair it with a mode-bits predicate later).
• Test polish: add negative asserts for the remaining EntryType variants (e.g. GNULongLink) to lock the contract against a future tar-crate reclassification.

Clean bills

Performance (allocation-free, branch-only helpers on pre-existing passes), panic paths (none outside cfg(test)), feature gates (none hide the surface; serde/tar non-optional), deny-warnings build, DCO/commit-message convention.

[asto18089]

Fix wave pushed (49c8b733b, 97fee5d1a)

Addressing this review — all findings I judged worth taking, with two deliberate non-changes noted at the end.

MAJOR (contract wording):

• Module doc no longer claims the in-tree plugin installer "already does" for the two new helpers: it now says the installer consumes the judgments verbatim, and that importers with deliberately different verdict semantics (zip's normalize-and-accept traversal) keep the judgment local and register divergence with tests. Dropped the crate::plugins::install pointer external rustdoc readers cannot resolve.
• 49c8b733b makes the claim true for the link predicate: plugins/install/tarball.rs now calls entry_type_is_link instead of its inline is_symlink() || is_hard_link() (same expression, pure predicate — plugins suite still 24 passed). The plugin size gate keeps its own accumulation, and the docs now say why: it must raise PluginInstallError::OversizedBundle, not InstallError::OversizedTarball.
• PR body rewritten with the honest framing: the "aligned" label was partly inaccurate (zip sanitizer verdicts never matched the engine's), feat(execpolicy): layered permission rulesets — defaults+agent+user (closes #415) codewhale-hq/Codewhale#653 adopts the point-equivalent subset and pins the divergent ones.

Minors fixed:

• skills/mod.rs: the five new contract names joined the downstream-consumer re-export set (short skills::* path reaches them).
• InstallError doc: dropped the "used internally" audience statement; it now names the embedder downcast pattern.
• InstalledFromMarker: schema history corrected (v1 wrote spec/url/checksum; v2 adds schema_version/source_checksum/content_digest/installed_name/registry_version) and url/installed_name/registry_version marked intentionally-not-modeled.
• source_checksum(): doc states the v1 leg is returned verbatim (can be empty; empty reads as "changed" — fail-safe).
• "single source of truth" / "cannot drift" replaced with reviewable-home wording; stricter embedder allowlists documented as policy, not drift.
• entry_type_is_link doc: predicate separated from caller policy (subtree skipping); zip keeps the unix-mode face, pinned by tests.
• entry_type_is_link_accepts_only_link_entries now pins every non-link EntryType variant (11, including GNULongLink — a metadata header, not a link entry) so a tar-crate reclassification cannot silently widen the judgment.

Deliberate non-changes:

• entry_type_is_link stays pub: it is part of the documented contract face (the follow-up chop references it as the tar-side judgment), and its in-tree consumer now exists. A mode-bits predicate for zip can be added when a second zip consumer needs it — today feat(execpolicy): layered permission rulesets — defaults+agent+user (closes #415) codewhale-hq/Codewhale#653's single site is correctly registered as local.
• The plugin size gate is not generalized behind an error-parameter (or similar): one caller pair with differently-typed errors doesn't justify the indirection yet; the docs now state the boundary explicitly.

Verification after the wave: cargo fmt --all -- --check clean; cargo clippy -p codewhale-tui --lib zero warnings; skills::install 22 passed (the expanded link test included), plugins::install 24 passed.