From 6ae527359e3e064d4186ac487c64082d3834b4d5 Mon Sep 17 00:00:00 2001 From: Ikalus1988 Date: Fri, 3 Jul 2026 01:01:21 +0800 Subject: [PATCH] fix: CI self-heal robustness, Windows compatibility, dashboard SQLite safety - classify-failure: heredoc + os.environ.get() (no shell injection) - retry: capture command output to log file for downstream classification - pr-checks: continue-on-error for score step, handle missing score gracefully - token_manager: Windows-compatible _restrict_plaintext_file() with warning - dashboard: explicit conn.close() in try/finally, conn.commit() after CREATE TABLE - tests: Windows bash detection, encoding params, POSIX permission skip on win32 12 passed, 2 skipped (Windows-only skips). Co-Authored-By: Claude Signed-off-by: Ikalus1988 --- .github/actions/classify-failure/action.yml | 161 ++++++++++---------- .github/actions/retry/action.yml | 11 +- .github/workflows/pr-checks.yml | 15 +- hub/master/token_manager.py | 24 ++- misakanet/tools/dashboard.py | 31 ++-- tests/test_ci_self_heal.py | 36 ++++- tests/test_dashboard.py | 6 +- tests/test_token_manager_nokeyring.py | 10 +- 8 files changed, 184 insertions(+), 110 deletions(-) diff --git a/.github/actions/classify-failure/action.yml b/.github/actions/classify-failure/action.yml index 09d69d55..ebf8b83f 100644 --- a/.github/actions/classify-failure/action.yml +++ b/.github/actions/classify-failure/action.yml @@ -28,87 +28,90 @@ runs: LOG_FILE="${{ inputs.log_file }}" EXIT_CODE="${{ inputs.exit_code }}" - CLASS=$(python3 -c " - import re, sys - - log = '' - try: - with open('$LOG_FILE', 'r') as f: - log = f.read() - except: - log = '' - - exit_code = int('$EXIT_CODE') if '$EXIT_CODE'.isdigit() else 1 - - # Network timeout patterns - if any(p in log.lower() for p in [ - 'timeout', 'timed out', 'connection refused', 'name resolution', - 'ssl', 'tls', 'network', 'dns', 'errno', 'etimedout', - 'econnrefused', 'econnreset', 'httperror', '502', '503', '504' - ]): - print('network_timeout') - sys.exit(0) - - # Permission denied patterns - if any(p in log.lower() for p in [ - 'permission denied', 'forbidden', '401', '403', - 'authentication failed', 'token expired', 'unauthorized', - 'secret', 'not allowed' - ]): - print('permission_denied') - sys.exit(0) - - # Race condition patterns - if any(p in log.lower() for p in [ - 'conflict', '409', 'race', 'concurrent', - 'another process', 'locked', 'busy', - 'already exists', 'duplicate' - ]): - print('race_condition') - sys.exit(0) - - # Flaky test patterns - if any(p in log.lower() for p in [ - 'flaky', 'intermittent', 'sometimes fails', - 'passed on retry', 'random', 'unstable' - ]): - print('flaky_test') - sys.exit(0) - - # Test failures (likely flaky if retry helps) - if any(p in log.lower() for p in [ - 'assertionerror', 'assert', 'test failed', - 'pytest', 'unittest', 'coverage' - ]): - print('flaky_test') - sys.exit(0) - - # Real bug patterns - if any(p in log.lower() for p in [ - 'syntaxerror', 'typeerror', 'nameerror', 'attributeerror', - 'importerror', 'modulenotfounderror', 'keyerror', 'indexerror', - 'valueerror', 'zerodivisionerror', 'segmentation fault', - 'core dumped', 'fatal', 'panic' - ]): - print('real_bug') - sys.exit(0) - - print('unknown') - ") + export LOG_FILE EXIT_CODE + + CLASS=$(python3 <<'PY' +import os +import sys + +log = "" +try: + with open(os.environ.get("LOG_FILE", ""), encoding="utf-8", errors="replace") as f: + log = f.read() +except OSError: + log = "" + +exit_code_raw = os.environ.get("EXIT_CODE", "1") +exit_code = int(exit_code_raw) if exit_code_raw.isdigit() else 1 +log_lower = log.lower() + +if any(p in log_lower for p in [ + "timeout", "timed out", "connection refused", "name resolution", + "ssl", "tls", "network", "dns", "errno", "etimedout", + "econnrefused", "econnreset", "httperror", "502", "503", "504", +]): + print("network_timeout") + sys.exit(0) + +if any(p in log_lower for p in [ + "permission denied", "forbidden", "401", "403", + "authentication failed", "token expired", "unauthorized", + "secret", "not allowed", +]): + print("permission_denied") + sys.exit(0) + +if any(p in log_lower for p in [ + "conflict", "409", "race", "concurrent", + "another process", "locked", "busy", + "already exists", "duplicate", +]): + print("race_condition") + sys.exit(0) + +if any(p in log_lower for p in [ + "flaky", "intermittent", "sometimes fails", + "passed on retry", "random", "unstable", +]): + print("flaky_test") + sys.exit(0) + +if any(p in log_lower for p in [ + "assertionerror", "assert", "test failed", + "pytest", "unittest", "coverage", +]): + print("flaky_test") + sys.exit(0) + +if any(p in log_lower for p in [ + "syntaxerror", "typeerror", "nameerror", "attributeerror", + "importerror", "modulenotfounderror", "keyerror", "indexerror", + "valueerror", "zerodivisionerror", "segmentation fault", + "core dumped", "fatal", "panic", +]): + print("real_bug") + sys.exit(0) + +print("unknown") +PY + ) + # Generate summary - SUMMARY=$(python3 -c " - log = '' - try: - with open('$LOG_FILE', 'r') as f: - lines = f.readlines() - # Get last 3 non-empty lines - relevant = [l.strip() for l in lines if l.strip()][-3:] - summary = ' | '.join(relevant)[:200] - print(summary if summary else 'No log content available') - except: - print('Could not read log file') - ") + SUMMARY=$(python3 <<'PY' +import os + +try: + with open(os.environ.get("LOG_FILE", ""), encoding="utf-8", errors="replace") as f: + lines = f.readlines() + relevant = [line.strip() for line in lines if line.strip()][-3:] + summary = " | ".join(relevant)[:200] + print(summary if summary else "No log content available") +except OSError: + print("Could not read log file") +PY + ) + # Determine should_retry if [ "$CLASS" = "real_bug" ]; then diff --git a/.github/actions/retry/action.yml b/.github/actions/retry/action.yml index d6cb6a99..17547706 100644 --- a/.github/actions/retry/action.yml +++ b/.github/actions/retry/action.yml @@ -44,6 +44,8 @@ runs: BASE_SECONDS="${{ inputs.backoff_base_seconds }}" RETRY_CODES="${{ inputs.retry_on_exit_code }}" SHELL_CMD="${{ inputs.shell_override }}" + LOG_FILE="${GITHUB_WORKSPACE:-$PWD}/.retry-output.log" + : > "$LOG_FILE" ATTEMPT=0 FINAL_CODE=0 @@ -51,12 +53,13 @@ runs: ATTEMPT=$((ATTEMPT + 1)) echo "::group::Attempt $ATTEMPT/$MAX_ATTEMPTS" - # Run the command - $SHELL_CMD -c "${{ inputs.run }}" - EXIT_CODE=$? + # Run the command and preserve its output for downstream classification. + echo "Command: ${{ inputs.run }}" | tee -a "$LOG_FILE" + $SHELL_CMD -c "${{ inputs.run }}" 2>&1 | tee -a "$LOG_FILE" + EXIT_CODE=${PIPESTATUS[0]} FINAL_CODE=$EXIT_CODE - echo "Exit code: $EXIT_CODE" + echo "Exit code: $EXIT_CODE" | tee -a "$LOG_FILE" echo "::endgroup::" if [ $EXIT_CODE -eq 0 ]; then diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index ea1bc540..f8fa655d 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -82,6 +82,7 @@ jobs: - name: Agent Quality Score if: steps.scope.outputs.scope == 'full' id: score + continue-on-error: true uses: ./.github/actions/score-agent with: pr-number: ${{ github.event.pull_request.number || github.event.inputs.pr_number }} @@ -90,7 +91,7 @@ jobs: gh-token: ${{ secrets.SHELDON_PAT || secrets.GITHUB_TOKEN }} - name: Reject Low-Quality PR - if: steps.scope.outputs.scope == 'full' && steps.score.outputs.verdict == 'fail' + if: steps.scope.outputs.scope == 'full' && steps.score.outcome == 'success' && steps.score.outputs.verdict == 'fail' env: GH_TOKEN: ${{ secrets.SHELDON_PAT || secrets.GITHUB_TOKEN }} run: | @@ -290,16 +291,20 @@ jobs: # 1. Quality Score (full scope only) if [ "$SCOPE" = "full" ]; then - REPORT+="### 📊 Quality Score: ${SCORE:-?}/100" + REPORT+="### 📊 Quality Score" REPORT+=$'\n\n' - if [ -n "$REASONS" ]; then + if [ -z "$SCORE" ]; then + REPORT+="⚠️ Quality score unavailable; continuing with hard gates." + elif [ -n "$REASONS" ]; then + REPORT+="Score: ${SCORE}/100" + REPORT+=$'\n\n' REPORT+="**Deductions:**" REPORT+=$'\n' echo "$REASONS" | while IFS= read -r line; do [ -n "$line" ] && REPORT+="- ${line}"$'\n' done else - REPORT+="No deductions." + REPORT+="Score: ${SCORE}/100 — no deductions." fi REPORT+=$'\n\n' fi @@ -398,7 +403,7 @@ jobs: fi if [ "$SCOPE" = "full" ]; then - if [ "$SCORE" -lt 40 ] 2>/dev/null; then + if [ -n "$SCORE" ] && [ "$SCORE" -lt 40 ] 2>/dev/null; then REPORT+="❌ Quality Score ${SCORE}/40 below threshold."$'\n' VERDICT_PASS=false fi diff --git a/hub/master/token_manager.py b/hub/master/token_manager.py index 47a40296..ecedbe2d 100644 --- a/hub/master/token_manager.py +++ b/hub/master/token_manager.py @@ -7,6 +7,7 @@ from typing import Optional import json import os +import warnings class TokenManager: @@ -45,7 +46,6 @@ def _load_tokens(self): # Fallback: plaintext JSON file with owner-only permissions (chmod 600) # WARNING: This is NOT encrypted. Do NOT rely on this for high-security environments. # On shared hosts or compromised machines, any process running as this user can read the file. - import warnings warnings.warn( "Master token fallback: storing tokens as plaintext JSON in ~/.hermes-tokens. " "This is NOT encrypted. For production, ensure keyring is available or use a secrets manager." @@ -55,11 +55,27 @@ def _load_tokens(self): with open(token_path, 'r') as f: self._tokens = json.load(f) self._cleanup_expired() - # Ensure file is only readable by owner - os.chmod(token_path, 0o600) + self._restrict_plaintext_file(token_path) except FileNotFoundError: self._tokens = {} + def _restrict_plaintext_file(self, token_path: str) -> bool: + """Best-effort owner-only permissions for the plaintext fallback file. + + Returns True when POSIX mode bits confirm 0600. On Windows, Python's + chmod/stat mode bits do not express owner-only ACLs, so callers must + treat this fallback as weaker than keyring-backed storage. + """ + if os.name == "nt": + warnings.warn( + "Master token fallback on Windows cannot guarantee POSIX 0600 " + "owner-only permissions. Use keyring or a secrets manager for production." + ) + return False + + os.chmod(token_path, 0o600) + return (os.stat(token_path).st_mode & 0o777) == 0o600 + def _save_tokens(self): """Save tokens — use keyring if available, else plaintext JSON fallback (NOT encrypted)""" if self._keyring_available: @@ -76,7 +92,7 @@ def _save_tokens(self): os.makedirs(os.path.dirname(token_path), exist_ok=True) with open(token_path, 'w') as f: json.dump(self._tokens, f, default=str) - os.chmod(token_path, 0o600) + self._restrict_plaintext_file(token_path) def _cleanup_expired(self): """Remove expired tokens""" diff --git a/misakanet/tools/dashboard.py b/misakanet/tools/dashboard.py index 96eceaf9..77c63422 100644 --- a/misakanet/tools/dashboard.py +++ b/misakanet/tools/dashboard.py @@ -20,22 +20,28 @@ def _connect(telemetry_path: str | Path) -> sqlite3.Connection: path = Path(telemetry_path) path.parent.mkdir(parents=True, exist_ok=True) conn = sqlite3.connect(str(path)) - conn.execute( - """ - CREATE TABLE IF NOT EXISTS search_telemetry ( - query TEXT, - timestamp REAL, - latency_ms REAL, - cache_hit INTEGER + try: + conn.execute( + """ + CREATE TABLE IF NOT EXISTS search_telemetry ( + query TEXT, + timestamp REAL, + latency_ms REAL, + cache_hit INTEGER + ) + """ ) - """ - ) - return conn + conn.commit() + return conn + except Exception: + conn.close() + raise def read_dashboard_data(telemetry_path: str | Path = DEFAULT_TELEMETRY_PATH) -> dict[str, Any]: """Read summary metrics and recent rows from the telemetry database.""" - with _connect(telemetry_path) as conn: + conn = _connect(telemetry_path) + try: total_searches = int( conn.execute("SELECT COUNT(*) FROM search_telemetry").fetchone()[0] ) @@ -62,6 +68,8 @@ def read_dashboard_data(telemetry_path: str | Path = DEFAULT_TELEMETRY_PATH) -> LIMIT 20 """ ).fetchall() + finally: + conn.close() saved_time_ms = 0.0 if hit_count and avg_hit_latency is not None and avg_miss_latency is not None: @@ -84,7 +92,6 @@ def read_dashboard_data(telemetry_path: str | Path = DEFAULT_TELEMETRY_PATH) -> ], } - def _format_timestamp(timestamp: float) -> str: if timestamp <= 0: return "-" diff --git a/tests/test_ci_self_heal.py b/tests/test_ci_self_heal.py index 89bd20d0..e38c2fda 100644 --- a/tests/test_ci_self_heal.py +++ b/tests/test_ci_self_heal.py @@ -4,7 +4,9 @@ """ import json import os +import shutil import subprocess +import sys import tempfile import unittest @@ -12,6 +14,25 @@ class TestRetryBackoff(unittest.TestCase): """Test retry action behavior via subprocess.""" + @classmethod + def setUpClass(cls): + cls.bash = shutil.which("bash") + if cls.bash: + probe = subprocess.run( + [cls.bash, "-c", "echo ok"], + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=10, + ) + if probe.returncode != 0 or "ok" not in (probe.stdout or ""): + cls.bash = None + + def setUp(self): + if not self.bash: + self.skipTest("requires a usable POSIX bash") + def _run_retry(self, command, max_attempts=3, backoff="exponential", base_seconds=1, retry_codes=""): """Helper: run a command through retry logic.""" script = ''' @@ -57,7 +78,14 @@ def _run_retry(self, command, max_attempts=3, backoff="exponential", base_second echo "attempts=$ATTEMPT exit=$FINAL_CODE" exit $FINAL_CODE ''' % (max_attempts, backoff, base_seconds, retry_codes, __import__('shlex').quote(command)) - result = subprocess.run(["bash", "-c", script], capture_output=True, text=True, timeout=30) + result = subprocess.run( + [self.bash, "-c", script], + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=30, + ) return result def test_succeeds_after_2_failures(self): @@ -97,14 +125,14 @@ class TestFailureClassification(unittest.TestCase): def _classify(self, log_content, exit_code="1"): """Helper: write log to temp file and run classification.""" - with tempfile.NamedTemporaryFile(mode="w", suffix=".log", delete=False) as f: + with tempfile.NamedTemporaryFile(mode="w", suffix=".log", encoding="utf-8", delete=False) as f: f.write(log_content) log_file = f.name try: py_code = ( "import sys\n" - f"with open('{log_file}') as f:\n" + f"with open({json.dumps(log_file)}, encoding='utf-8') as f:\n" " log = f.read().lower()\n" "patterns = {\n" " 'network_timeout': ['timeout', 'timed out', 'connection refused', 'ssl', 'dns'],\n" @@ -120,7 +148,7 @@ def _classify(self, log_content, exit_code="1"): " break\n" "print(result)\n" ) - result = subprocess.run(["python3", "-c", py_code], capture_output=True, text=True) + result = subprocess.run([sys.executable, "-c", py_code], capture_output=True, text=True) return result.stdout.strip() finally: os.unlink(log_file) diff --git a/tests/test_dashboard.py b/tests/test_dashboard.py index 7b342e6f..6d9a863d 100644 --- a/tests/test_dashboard.py +++ b/tests/test_dashboard.py @@ -14,7 +14,8 @@ class TestTelemetryDashboard(unittest.TestCase): def test_dashboard_serves_telemetry_html(self): with tempfile.TemporaryDirectory() as tmp: telemetry_path = Path(tmp) / "telemetry.db" - with sqlite3.connect(telemetry_path) as conn: + conn = sqlite3.connect(telemetry_path) + try: conn.execute( """ CREATE TABLE search_telemetry ( @@ -37,6 +38,9 @@ def test_dashboard_serves_telemetry_html(self): ("", time.time(), 30.0, 1), ], ) + conn.commit() + finally: + conn.close() server = create_server(port=0, telemetry_path=telemetry_path) self.assertIsInstance(server, ThreadingHTTPServer) diff --git a/tests/test_token_manager_nokeyring.py b/tests/test_token_manager_nokeyring.py index 7498d072..6d9a94fa 100644 --- a/tests/test_token_manager_nokeyring.py +++ b/tests/test_token_manager_nokeyring.py @@ -90,7 +90,15 @@ def mock_import(name, *args, **kwargs): self.assertTrue(os.path.exists(self.token_path)) mode = os.stat(self.token_path).st_mode & 0o777 - self.assertEqual(mode, 0o600, f"Expected 0600, got {oct(mode)}") + if sys.platform == "win32": + self.assertWarnsRegex( + UserWarning, + "cannot guarantee POSIX 0600", + tm._restrict_plaintext_file, + self.token_path, + ) + else: + self.assertEqual(mode, 0o600, f"Expected 0600, got {oct(mode)}") with open(self.token_path) as f: saved = json.load(f)