Skip to content

[P1/mid] fix(server_remote): replace print() with logging to clear CodeQL py/clear-text-logging-sensitive-data - #281

Merged
cipher813 merged 7 commits into
mainfrom
groom/security-sweep-4356-clear-text-logging
Aug 2, 2026
Merged

[P1/mid] fix(server_remote): replace print() with logging to clear CodeQL py/clear-text-logging-sensitive-data#281
cipher813 merged 7 commits into
mainfrom
groom/security-sweep-4356-clear-text-logging

Conversation

@ne-groomer

@ne-groomer ne-groomer Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Priority: P1 · Complexity: mid

Closes nousergon/alpha-engine-config#4356

Problem

CodeQL alert py/clear-text-logging-sensitive-data on src/mnemon/server_remote.py — the file used print(file=sys.stderr) for all operational logging. Exception messages ({e} in f-strings) could theoretically propagate sensitive data through the log stream, and the pattern prevented structured log level control.

Root-cause fix

Replaced all 11 print(file=sys.stderr, ...) calls with appropriate logging.info() / logging.warning() / logging.error() calls via Python's standard logging module, consistent with every other source file in the project (server.py, auth.py, store.py, etc.).

Updated test assertions from capsys (stderr capture) to caplog (log capture).

Validation

  • ✅ All 13 test_server_remote.py tests pass
  • ✅ Full suite: 574 passed, 1 skipped, 1 pre-existing env-only failure (test_mnemon_ops.py: mnemon_ops.sh script path not found on this runner)

…lear-text-logging-sensitive-data

All 13 server_remote tests pass (converted from capsys to caplog). Full
suite: 574 passed, 1 pre-existing env-only failure (mnemon_ops.sh
script path).

Closes nousergon/alpha-engine-config#4356

Co-Authored-By: Claude <noreply@anthropic.com>
@ne-groomer
ne-groomer Bot requested a review from cipher813 July 27, 2026 12:33
@ne-groomer ne-groomer Bot added the source:groom Opened by the backlog groomer label Jul 27, 2026
Comment thread src/mnemon/server_remote.py Fixed
ne-groomer Bot and others added 3 commits July 27, 2026 20:21
Refactor AuthorizationServerConfig.validate() to not access self.passphrase
— moves passphrase validation into a separate @staticmethod that takes only
non-sensitive metadata (bool + int), so CodeQL's py/clear-text-logging
data-flow analysis can see no password-classified data reaches the
logger.error() sink.

Also removes passphrase-length disclosure from the error message ("minimum
16 chars" instead of "X chars, minimum 16").

Closes nousergon/alpha-engine-config#4356

Co-Authored-By: Claude <noreply@anthropic.com>
@ne-groomer
ne-groomer Bot marked this pull request as draft July 28, 2026 00:32
@ne-groomer ne-groomer Bot added the gate:dependency Blocked on another issue/PR merging first label Jul 28, 2026
@ne-groomer

ne-groomer Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

🚫
The end-of-SF sweep's fix pass could not fix this PR (CI red on the current head) — escalated per alpha-engine-config-I2714: converted to draft, labeled gate:dependency, and tracked as alpha-engine-config-I4943.

Blocked-by: nousergon/alpha-engine-config#4943

gate_dependency_sweep.py auto-clears the gate (re-verifying CI/conflicts first) when that issue closes.

groom-bot and others added 2 commits July 28, 2026 23:28
The PR split validate() into validate() (non-passphrase checks) and
validate_passphrase() (passphrase-only, signature avoids value flowing
through log paths). The tests were still calling validate() for
passphrase assertions — updated to call validate_passphrase() instead.

- test_enabled_requires_public_url_and_passphrase: checks both methods
- test_short_passphrase_rejected: uses validate_passphrase()
- test_exactly_16_char_passphrase_accepted: uses validate_passphrase()

Co-Authored-By: Claude <noreply@anthropic.com>
mcp>=1.27 resolved to mcp-2.0.0 which removed mcp.server.fastmcp,
breaking server.py and all tests that import from it (test_server.py,
test_server_proxy.py, test_tools_integration.py). Pin to <2.0.0 until
the migration to mcp v2 APIs is done.

Co-Authored-By: Claude <noreply@anthropic.com>
@ne-groomer ne-groomer Bot removed the gate:dependency Blocked on another issue/PR merging first label Jul 29, 2026
@ne-groomer

ne-groomer Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

🔓 Dependency resolved — every referenced dependency (nousergon/alpha-engine-config#4943) is now closed/merged, so gate:dependency has been removed and this PR re-enters the daily groom queue (config#1891 deterministic sweep; gate exclusion per config#1805).

@ne-groomer
ne-groomer Bot marked this pull request as ready for review July 29, 2026 15:31
@cipher813 cipher813 added the automerge:armed On an automated merge lane — no human action needed label Jul 29, 2026
@ne-groomer

ne-groomer Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

⏳ This PR has had merge conflicts in 13 consecutive sweep cycles. After 3 cycles it will be escalated (draft + gate:dependency + tracked issue).

@ne-groomer ne-groomer Bot added blocked:conflict Merge conflicts — sweep will attempt a fix and removed automerge:armed On an automated merge lane — no human action needed labels Jul 29, 2026
@cipher813
cipher813 marked this pull request as draft July 30, 2026 01:29
@cipher813 cipher813 added the gate:dependency Blocked on another issue/PR merging first label Jul 30, 2026
@cipher813

Copy link
Copy Markdown
Collaborator

🚫
The end-of-SF sweep's fix pass could not fix this PR (real merge conflicts with main) — escalated per alpha-engine-config-I2714: converted to draft, labeled gate:dependency, and tracked as alpha-engine-config-I5616.

Blocked-by: nousergon/alpha-engine-config#5616

gate_dependency_sweep.py auto-clears the gate (re-verifying CI/conflicts first) when that issue closes.

@cipher813 cipher813 added blocked:gate Blocked by a gate:* label and removed blocked:conflict Merge conflicts — sweep will attempt a fix labels Jul 30, 2026
@cipher813
cipher813 removed their request for review July 30, 2026 15:08
…4356-clear-text-logging

# Conflicts:
#	pyproject.toml
@cipher813

Copy link
Copy Markdown
Collaborator

Merge conflict resolved + gate verification (from alpha-engine-config#5616).

Resolved the merge conflict by merging origin/main into this branch (merge commit 979386a). The only conflict was a single hunk in pyproject.toml — the mcp pin. Resolved it to main's version "mcp>=1.27,<2", # <2: mcp 2.0.0 (2026-07-28) removed mcp.server.fastmcp — mnemon-ops-I34, which is semantically identical to the branch's <2.0.0 and carries the better comment. No branch changes reverted.

Verification that the gate:dependency blocker (#5616) is resolved:

  • mergeable = MERGEABLE (conflict-free) — verified live on the current head 979386a
  • All CI checks green: test (3.10/3.12/3.13 × ubuntu/macos), built-wheel ×3, CodeQL, Analyze (actions+python), audit, secrets, test-server-extras-only — all pass

Flipping ready for review and removing the now-satisfied gate:dependency label together per config#2002.

@cipher813
cipher813 marked this pull request as ready for review August 2, 2026 06:33
@cipher813
cipher813 self-requested a review as a code owner August 2, 2026 06:33
@cipher813 cipher813 removed gate:dependency Blocked on another issue/PR merging first conflict-chronic blocked:gate Blocked by a gate:* label labels Aug 2, 2026
@ne-groomer ne-groomer Bot added the needs:human-review Awaiting Brian — no auto-merge lane took it, and self-authorship blocks a native review request label Aug 2, 2026
@cipher813
cipher813 merged commit 515bee6 into main Aug 2, 2026
15 checks passed
@cipher813
cipher813 deleted the groom/security-sweep-4356-clear-text-logging branch August 2, 2026 14:39
@ne-groomer ne-groomer Bot removed the needs:human-review Awaiting Brian — no auto-merge lane took it, and self-authorship blocks a native review request label Aug 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

source:groom Opened by the backlog groomer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants