Namespace the appdata path, and pin the unusable-port case - #83
Merged
Conversation
Three things this file and its neighbours got wrong, none of which shows up as a failure until it matters. docker-compose.yml interpolated a bare APPDATA_DIR into the /config bind mount. Compose prefers a shell-exported variable over the project's .env, so running docker compose up by hand from a shell that had sourced a sibling project's .env would mount that project's directory here. The wrong path then freezes into the container until it is recreated, with nothing logged. deploy.sh is immune because ssh does not forward the environment, so this only ever bit an interactive run. The variable is SMARTTHINGS_LOCAL_APPDATA_DIR now, in .env.example, docker-compose.yml and deploy.sh together, which is a name no other project can shadow. deploy.sh keeps its own short local name; nothing exports it, so Compose never sees it. DEBUG_BRIDGE was read by bridge.py and documented nowhere, so the only way to find it was to grep the source. tests/test_ocf_discovery.py had no case for a policy port it cannot use. Zero is the one the hardware produces: RT-OCF binds the DTLS socket with port 0 and learns the assignment through getsockname, so a directory serialised before that bind advertises sec true with port 0. A plaintext read on the reference dryer here has returned exactly that while a later read of the same device gave a real port. Absence is the right answer, because it surfaces as no_secure_ports and leaves the caller free to retry, and a bool has to be rejected too or True passes for port 1. The deploy.sh header also named two stale convention paths and a bare source .env that no longer resolves the key it goes on to use. Claude-Session: https://claude.ai/code/session_01UJBUFo8zZWrebGqf6gUzcL
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three things that only show up as a failure once it is too late to see them.
docker-compose.ymlinterpolated a bareAPPDATA_DIRinto the/configbind mount. Compose gives a shell-exported variable precedence over the project's.env, so adocker compose uprun by hand from a shell that had sourced a sibling project's.envwould mount that project's directory here. The wrong path then freezes into the container until it is recreated, and nothing logs it.deploy.shis immune, since ssh does not forward the environment, so this could only ever bite an interactive run.The variable is
SMARTTHINGS_LOCAL_APPDATA_DIRnow, renamed in.env.example,docker-compose.ymlanddeploy.shtogether so the:?guard fails loudly rather than falling through to the./certsdefault.deploy.shkeeps a short local name internally; nothing exports it, so Compose never sees it.DEBUG_BRIDGEwas read bybridge.pyand documented nowhere. The only way to find it was to grep the source..env.exampledescribes it now.tests/test_ocf_discovery.pyhad no case for a policy port the library cannot use. Zero is the one the hardware produces: RT-OCF binds the DTLS socket with port0and learns the assignment throughgetsockname, so a directory serialised before that bind advertisessec: true, port: 0. A plaintext read on my reference dryer has returned exactly that, while a later read of the same device gave a real port. Absence is the right answer, because it surfaces asno_secure_portsand leaves the caller free to retry rather than sending a handshake at port 0. A bool needs rejecting in the same breath, orTruepasses for port 1. A second case covers one unusable port alongside a usable one, so a single bad link cannot discard the rest of a reduced directory.While in the
deploy.shheader: two stale convention paths, and a bootstrap snippet whosesource .envno longer resolved the key it went on to use.Tested: 769 tests pass,
check_share_safety.py --changed-since mainclean. Deployed to my Unraid bridge and recreated the container:/configresolves to/mnt/user/appdata/smartthings-localwith both certs visible, no./certsfallback directory got created, and both appliances reconnected and seeded clean.https://claude.ai/code/session_01UJBUFo8zZWrebGqf6gUzcL