Skip to content

Redact SmbConfig password and centralize default SMB port - #117

Merged
in-jun merged 1 commit into
mainfrom
fix/remoteconfig
Jul 18, 2026
Merged

Redact SmbConfig password and centralize default SMB port#117
in-jun merged 1 commit into
mainfrom
fix/remoteconfig

Conversation

@in-jun

@in-jun in-jun commented Jul 18, 2026

Copy link
Copy Markdown
Owner

Two small hardening/hygiene fixes to SmbConfig and its call sites.

  • Override toString() on SmbConfig so the plaintext password is rendered as *** instead of the raw value. The data-class default would dump the secret verbatim, and since withoutSecrets() and a redacting SyncPair both stringify configs, a future debug log or interpolated exception message could leak the password to logcat without anyone noticing in review.
  • Add const val DEFAULT_PORT = 445 to SmbConfig's companion and reference it from the four fallback sites (SetupScreen seed + build, ConfigRepository JSON parse + legacy migration) so the default lives in one place instead of five copies.

Fixes #77
Fixes #83

@in-jun in-jun left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both fixes look correct and complete.

#77 — password redaction: The custom toString() covers exactly the constructor properties in declaration order and matches the default data-class format, so the only behavioral change is masking password. Since SmbConfig is the sole RemoteConfig implementor holding a secret and SyncPair's auto-generated toString() delegates to remote.toString(), both the direct-config and enclosing-pair leak paths described in the issue are now closed. equals/hashCode are untouched, so the withoutSecrets() equality semantics still hold. The empty-password case rendering password= (rather than ***) is fine — there's no secret to leak there.

#83 — centralized default: DEFAULT_PORT = 445 is referenced from all four production fallback sites (SetupScreen seed + build, ConfigRepository JSON parse + legacy migration) plus the constructor default. The remaining literal in SmbRemoteStorageIntegrationTest is test-only configuration and correctly out of scope.

No regressions or scope creep. LGTM.

@in-jun
in-jun merged commit 7688de1 into main Jul 18, 2026
1 check passed
@in-jun
in-jun deleted the fix/remoteconfig branch July 18, 2026 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Default SMB port 445 is repeated as a bare literal in five places SmbConfig's auto-generated toString() would print the plaintext password

1 participant