Save boat password access and protect boat storage

[jneb802]

Players currently enter the boat password every time they take the helm, while boat storage has no password check. This change saves access per character on the boat and uses the same access for its helm and storage. Access remains through reconnects, restarts, and password changes until the boat is destroyed. Setting a password also grants the creator access.

Storage prompts for the password and opens after authorization. Owner-side checks protect open, stack, and take-all requests and reject requests using another character's identity. Existing ward and container checks remain in effect. Install this version on the server and participating clients because owner grant payloads use version 2 messages.

Removes ShipRudderOwnershipPatch.cs completely. NetworkPerformanceSystem handles ownership through its existing helm patches. Password authorization is saved on the current boat owner before control is granted; NPS transfers that boat data with ownership. No replacement ownership patch or direct NPS dependency is added.

Validation on September 30, final commit b6d9d39:

• Release build passed with 84 existing warnings and no errors. git diff --check passed.
• Tested Karves with fresh development characters on Valdev and both Valnet clients, aligned to deployed Season 8 version 8.0.30 and NPS 1.11.1. Production shared DLL hashes matched except for the candidate. Both clients connected and the server accepted their mod lists.
• Runtime patch inspection confirmed the PraetorisClient ownership patch was absent and NPS ship ownership was enabled. NPS handoff counters increased, and the clients confirmed ownership moved between them.
• Saved access survived ownership transfers in both directions, password changes, both clients logging out, and a full Valdev restart. Helm and storage use did not require another password entry.
• Incorrect passwords and direct/spoofed storage requests were denied. A correct helm password also authorized storage. Authorized take-all moved the expected items.
• After destroying the original boat, its replacement had no saved access for the second character. A wrong chest password failed. A correct chest password opened storage automatically and also granted helm access without another prompt.
• The earlier feature run also covered clearing/resetting passwords, occupied helms, stack operations, and cargo persistence. That run preceded removal of the ownership patch.

Known compatibility limitation: an open passenger chest panel can disappear when NPS transfers boat ownership to the player steering. This also reproduced after clearing the boat password. Releasing the helm and reopening storage worked without re-entering a password. Saved authorization remains intact, but this PR does not resolve simultaneous steering/storage behavior.

Log review found no boat-password exceptions. Other issues remain: PieceManager snapshot null-reference errors, a location-unload ZoneSystem.UnsetLoadingInZone error during client setup, a DiscordConnector shutdown null-reference error, rendering/configuration warnings, and candidate hash/audit warnings from ValheimEnforcer. Older duplicate mod DLLs were skipped in favor of the loaded production versions. The server logged the candidate audit without enforcement and both clients remained connected. Setup also needed retries for teleport loading and joining before server readiness.

Test boats and helpers were removed. Original profile contents and metadata were restored and verified against backup hashes. Original client profile selections were restored. Both Valnet machines and Valdev were returned to their initial stopped state after checking the restored Valdev profile loads.

Evidence: /Users/benjmarston/Develop/valheim-validation-evidence/boat-nps-20260930/ contains command output, inspected screenshots, server/client logs, candidate file hashes, and restoration results. Earlier evidence remains in boat-access-20260930/. No test or documentation files are included in this PR.

[github-actions]

No blocking issues found.

Access persistence: data model and protocol

ShipPasswordData.GrantAccess appends |<id>| to an existing delimited string. A new entry starts with a trailing | from the previous entry, producing |12||34| instead of |12|34|. HasAccess searches for |<id>| so the double pipe is not a correctness problem (the substring is still found and partial IDs like 2 still cannot match 12), and the tests confirm it. Slightly wasteful storage, not a bug.

The RPC rename to SetGrantV2 / ControlGrantV2 is a correct versioning strategy: old clients cannot receive or handle the new payload (which now carries playerId for set and verifier for control), and the PR description already requires a coordinated install.

Empty-password fast path (BeginEnterPassword)

When the local ZDO shows HasAccess, the client skips the dialog and sends RequestControl(""). The server's unified OnAccessRequest guard is:

(!HasAccess(shipZdo, playerId) && !Verify(shipZdo!, password))

An empty password fails Verify, so the request only survives if the server-side ZDO also shows HasAccess. There is a narrow ZDO sync window right after the initial grant where the server's copy may lag behind. In that window the server rejects the request with "Incorrect ship password." – but since the dialog was never shown the player must re-interact with the helm to retry. After the ZDO sync cycle completes (typically well under a second) the retry succeeds. This is a transient UX hiccup, not a permanent failure, and the ZDO system's eventual-consistency guarantees make it self-resolving.

ShipPasswordStorageSenderPatch – sender identity check

The anti-spoofing check (character.GetOwner() == uid) iterates Player.GetAllPlayers(). If the ship ZDO is owned by a client (the normal case when any player is nearby), the patch runs on that client and all nearby players' Player instances are present. If the server owns a dormant ship ZDO, the patch runs on the server; Valheim dedicated servers do instantiate Player components for connected clients, so the check should work there too. The comment acknowledges that remote callers are routed through the server and the ownership comparison handles that correctly.

OnAccessGrant null safety for storage path

controls is only checked for null inside takeControl && (...), so controls may be null when takeControl is false. The method returns early at if (!takeControl) { ... return; } before controls!.GetUser() is reached, so no NullReferenceException is possible. Correct.

Storage retry loop

_storageRetryUntil is not explicitly cleared on a server rejection – only on timeout (10 s), on range exit, or when a new BeginStoragePassword call resets it to 0. After a wrong-password response the loop polls harmlessly until the timer expires. The player can re-interact immediately (the Interact patch calls BeginStoragePassword, which resets the timer) so there is no lockout. Not a bug.

ShipRudderOwnershipPatch proactive grant

The patch calls GrantAccess when the local player receives helm ownership to cover the race where the control grant arrives before the ZDO update. There is no corresponding ForceSendZDO here, but after ClaimOwnership() the local client is the ZDO owner and normal sync propagates the update to the server on the next cycle. The window between ownership claim and sync is the same race that the patch is already documenting; no new exposure is introduced.

Live risks (acknowledged, not introduced by the diff)

World-save persistence, Harmony patch interaction with the full Season 8 mod set, and ownership-transfer edge cases are called out in the PR body and the test README. None of these are regressions introduced by this diff; they were present before and are left for the live test procedure described in Tests/ShipPassword/README.md.

[github-actions]

No blocking issues found.

Overview: The PR unifies helm and storage access under a single persistent per-character access record stored in the ship ZDO, adds a new ShipPasswordStorage.cs with four Harmony patches, and bumps the grant RPC names to V2 to account for the new payload field (playerId in the set-grant, verifier in the control/storage grant).

---

Correctness observations

ShipPasswordData.GrantAccess (ShipPasswordData.cs:47–54) appends "|playerId|" each time, which produces a double-pipe separator between entries: the second player yields "|id1||id2|". The HasAccess Contains check for "|id2|" still matches correctly, so this is not a functional bug.

ShipPasswordInteraction.BeginEnterPassword (ShipPasswordInteraction.cs:82–87) calls _controls.m_ship.GetComponent<ZNetView>() before reaching any null guard on m_ship. If m_ship is ever null at that point (unlikely given game setup, but possible during unusual destruction/reload order), this will throw a NullReferenceException. The original code below the insertion also assumes a valid ship, so the risk profile is unchanged—but it is now triggered one step earlier.

InStorageRange (ShipPasswordInteraction.cs:127–131) uses container.m_hoverOffset as a range extension addend. m_hoverOffset is a display-positioning float for hover text, not an interaction-range value. For typical chest/cargo containers it happens to be small and positive, so the range check is slightly generous rather than wrong, but this is semantically incorrect. If a future container type uses a large or negative m_hoverOffset, the retry loop could cut out at the wrong distance.

ShipPasswordStorageSenderPatch (ShipPasswordStorage.cs:395–426) gates RPC_RequestOpen, RPC_RequestStack, and RPC_RequestTakeAll by verifying that character.GetOwner() == uid (the ZDO owner of the acting player equals the RPC sender). This correctly closes the impersonation vector the PR description calls out. However, if Player.GetAllPlayers() does not yet include a recently-connected player whose ZDO has arrived ahead of their Player instance, the patch returns false and silently blocks the operation. This is an edge case at login time and resolves itself within one or two ticks.

OnAccessGrant for the storage path (ShipPasswordRpc.cs) skips ship.IsPlayerInBoat(playerId). A player can therefore receive permanent storage access after sailing away before the grant arrives. The Update retry loop still guards the actual container open via InStorageRange, so the interaction itself does not fire—but the access persists in the ZDO. This is consistent with the PR goal ("access remains until the boat is destroyed") and appears intentional.

The verifier echo (shipZdo.GetString(VerifierHash, "") written into the grant, compared on the owner side) correctly invalidates stale grants when the password changes between server validation and owner processing.

The ShipRudderOwnershipPatch addition (ShipRudderOwnershipPatch.cs:456–459) defensively re-applies GrantAccess locally after ClaimOwnership(). Because the ForceSendZDO from OnAccessGrant carries a revision that predates the ownership claim, the incoming sync will lose to the now-higher local revision, making this write the authoritative record. The logic is correct.

---

Protocol compatibility

The V2 rename of ShipPasswordSetGrant and ShipPasswordControlGrant is a hard break: mixed-version sessions silently drop all grants. The PR description acknowledges this and requires simultaneous client+server updates. No additional mitigation is needed for a mod of this type, but callers that relied on the old constant names (e.g., external tooling or tests) will need updating.

[github-actions]

No blocking issues found.

The implementation correctly persists boat access via a pipe-delimited AuthorizedPlayersHash ZDO field and unifies the control and storage grant flows through OnAccessRequest/OnAccessGrant. The review below covers every meaningful behavior concern I identified.

---

Access persistence and ZDO lifecycle

ShipPasswordData.GrantAccess appends |playerId| entries that are never revoked, which is the stated design. HasAccess uses exact |id| sentinel delimiters so a player ID that is a prefix of another cannot falsely match. ForceSendZDO is correctly issued to the requesting peer after every grant so the local HasAccess check converges quickly. When the password is cleared, ForceSendZDO is intentionally skipped (no access change, verifier replication via normal sync is sufficient).

Already-authorized helm bypass

BeginEnterPassword sends RequestControl("") with an empty password when HasAccess is true on the client. The server accepts this via (!HasAccess && !Verify) short-circuit. If the client ZDO is stale (access not yet synced), the empty-password falls through to the normal prompt, which is safe degraded behavior.

ShipPasswordStorageSenderPatch identity check

The comment correctly explains that ZRoutedRpc preserves the original sender ID through server routing, so uid in RPC_Request* handlers is the actual client peer, not the server. character.GetOwner() == uid therefore correctly validates that the claim playerID is owned by the actual sender. The CheckAccess postfix is the primary authorization gate; SenderPatch is a defense-in-depth anti-spoofing layer.

OnAccessGrant null-safety

controls! at ShipPasswordRpc.cs:316 is used only when takeControl is true, and the guard at line 299 rejects when takeControl && controls == null, so the null-forgiving operator is safe.

Storage retry loop

ShipPasswordInteraction.Update retries _container.Interact for up to 10 seconds after a storage grant is sent, which bridges the gap between the success OnResponse and the ZDO update arriving. _storageRetryUntil = 0f is set before the Interact call to prevent repeated attempts. The _container == null and InStorageRange guards prevent stale opens after the player leaves. This is sound.

Verifier freshness check in OnAccessGrant

nview.GetZDO().GetString(VerifierHash, "") != verifier compares the grant-time verifier (from the server) against the owner's current ZDO. If the password changes in the window between server processing and owner processing, the grant is rejected and the player retries. Authorized players re-request with HasAccess still true, so a new grant with the updated verifier is issued immediately.

Minor observations (non-blocking)

• ShipPasswordStorageHoverPatch appends "\nPassword protected" for all players including those who already have access. UX-only; no behavior impact.
• InStorageRange adds container.m_hoverOffset to m_maxInteractDistance. This is a client-side proximity guard for the prompt, not a security gate; the server authorization is unaffected.
• RPC_RequestStack and RPC_RequestTakeAll parameter name assumptions in ShipPasswordStorageSenderPatch (uid, playerID) are unverifiable without Valheim source, but the PR description explicitly covers authorized stack and take-all testing.