diff --git a/.env.example b/.env.example index 8f1789e..bce727c 100644 --- a/.env.example +++ b/.env.example @@ -2,12 +2,12 @@ # Copy this file to .env and fill in your credentials # Graphistry/Louie credentials -GRAPHISTRY_SERVER=graphistry-dev.grph.xyz -GRAPHISTRY_USERNAME=your_username -GRAPHISTRY_PASSWORD=your_password +GRAPHISTRY_SERVER = hub.graphistry.com +GRAPHISTRY_USERNAME = your_username +GRAPHISTRY_PASSWORD = your_password # Alternative server URL (if different) -# LOUIE_SERVER=louie-dev.grph.xyz +# LOUIE_SERVER=https://den.louie.ai # API version (usually 3) -GRAPHISTRY_API_VERSION=3 \ No newline at end of file +GRAPHISTRY_API_VERSION = 3 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 77ce1d9..99de9ef 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,17 +24,21 @@ jobs: curl -LsSf https://astral.sh/uv/install.sh | sh echo "$HOME/.cargo/bin" >> $GITHUB_PATH - name: Install dependencies - run: | - uv venv --python python3.12 - uv pip install -e ".[dev]" + # Lockfile-pinned: lint/format/type results must be reproducible. + # `uv pip install -e ".[dev]"` ignores uv.lock and resolves floors + # (ruff>=0.12.0) to whatever is newest, so CI silently drifted to + # ruff 0.16 / mypy 2.3 while uv.lock pins 0.12.5 / 1.17.0. + # --locked (not --frozen) so a lockfile that has drifted from + # pyproject.toml fails the build instead of installing a stale set. + run: uv sync --locked --all-extras - name: Secret Detection run: ./scripts/ci/secret-detection.sh - name: Lint (ruff) - run: uv run ruff check . + run: uv run --locked ruff check . - name: Format (ruff) - run: uv run ruff format --check . + run: uv run --locked ruff format --check . - name: Type Check (mypy) - run: uv run mypy . + run: uv run --locked mypy . # Stage 2: Light smoke test (quick test run before full matrix) light-smoke-test: @@ -159,7 +163,7 @@ jobs: run: | # Only run if credentials are configured if [ -n "$GRAPHISTRY_USERNAME" ]; then - uv run pytest tests/integration/ -v --cov=louieai --cov-report=xml + uv run --locked pytest tests/integration/ -v --cov=louieai --cov-report=xml else echo "⚠️ Skipping integration tests - no credentials configured" fi @@ -187,17 +191,19 @@ jobs: curl -LsSf https://astral.sh/uv/install.sh | sh echo "$HOME/.cargo/bin" >> $GITHUB_PATH - name: Install dependencies - run: | - uv venv --python python3.12 - uv pip install -e ".[dev,docs]" + # Lockfile-pinned: `mkdocs build --strict` is a determinism gate, so a + # floating mkdocs/plugin version must not change pass/fail. See the + # matching note in quality-checks. + run: uv sync --locked --all-extras - name: Test documentation examples run: | - uv run pytest tests/unit/test_documentation.py -v + uv run --locked pytest tests/unit/test_documentation.py -v - name: Validate ReadTheDocs config run: | curl -sSL https://raw.githubusercontent.com/readthedocs/readthedocs.org/main/readthedocs/rtd_tests/fixtures/spec/v2/schema.json -o rtd-schema.json - uv pip install jsonschema pyyaml - uv run python -c " + # jsonschema and pyyaml already come from `uv sync --all-extras`; a + # bare `uv pip install` here would perturb the pinned environment. + uv run --locked python -c " import yaml, json, jsonschema with open('.readthedocs.yml') as f: config = yaml.safe_load(f) @@ -207,7 +213,7 @@ jobs: print('✅ ReadTheDocs config is valid') " - name: Build documentation - run: uv run mkdocs build --strict + run: uv run --locked mkdocs build --strict - name: Verify logo included run: | test -f site/assets/louie-logo.png || (echo "❌ Logo not found in built docs" && exit 1) diff --git a/.github/workflows/secret-detection-test.yml b/.github/workflows/secret-detection-test.yml index c58b45c..c0dd4c8 100644 --- a/.github/workflows/secret-detection-test.yml +++ b/.github/workflows/secret-detection-test.yml @@ -4,12 +4,15 @@ on: paths: # Only run when secret detection files change - 'scripts/ci/secret-detection.sh' + - 'scripts/ci/check_credential_literals.py' + - 'scripts/ci/check_new_secrets.py' - 'scripts/pre-commit-secret-check.sh' - 'scripts/test-secret-detection.sh' - 'scripts/secrets.sh' - '.secrets.baseline' - '.secret-patterns.md' - 'tests/secret_patterns_reference.py' + - 'tests/unit/security/**' - '.github/workflows/secret-detection-test.yml' jobs: @@ -29,22 +32,26 @@ jobs: echo "$HOME/.cargo/bin" >> $GITHUB_PATH - name: Install dependencies - run: | - uv venv --python python3.12 - uv pip install detect-secrets - + # Lockfile-pinned so detect-secrets' plugin set — which decides what the + # harness expectations mean — cannot drift between runs. + run: uv sync --locked --all-extras + - name: Test secret detection patterns - run: | - echo "🔬 Testing secret detection system..." - # For now, just verify scripts run without error - # Full pattern testing is complex due to detect-secrets heuristics - echo "Skipping pattern tests - verifying script execution only" + # Previously this step only echoed "Skipping pattern tests", so the + # harness was never executed in CI. It was also broken: it scanned an + # absolute /tmp path while cd'ed to the repo root, which made every + # detect-secrets scan return empty, so all five "unsafe" fixtures + # silently looked undetected. Both are fixed; run it for real. + run: ./scripts/test-secret-detection.sh + + - name: Test credential-literal gate + run: uv run --locked pytest tests/unit/security/ -q - name: Verify test patterns file run: | echo "📋 Verifying test patterns file..." # Check that safe patterns don't trigger - if uv run detect-secrets scan tests/secret_patterns_reference.py | grep -q "SAFE_"; then + if uv run --locked detect-secrets scan tests/secret_patterns_reference.py | grep -q "SAFE_"; then echo "❌ Safe patterns are triggering detection!" exit 1 fi diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 7f7e9ad..dbb3c2c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,12 +1,16 @@ +# Versions must match uv.lock, which is what CI installs via `uv sync --frozen`. +# Skew here is the same failure mode CI just hit: ruff 0.16 formats Markdown code +# fences and 0.12.5 does not, so a hook running a different version produces +# diffs the CI gate rejects. repos: - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.8.4 + rev: v0.12.5 hooks: - id: ruff args: [--fix] - id: ruff-format - repo: https://github.com/pre-commit/mirrors-mypy - rev: v1.8.0 + rev: v1.17.0 hooks: - id: mypy - repo: https://github.com/pre-commit/pygrep-hooks diff --git a/.secret-patterns.md b/.secret-patterns.md index 049ba87..c94a6a5 100644 --- a/.secret-patterns.md +++ b/.secret-patterns.md @@ -23,7 +23,13 @@ uv run detect-secrets scan tests/secret_patterns_reference.py - `****` or `********` - Masked placeholder ## API Keys -- `sk-XXXXXXXXXXXXXXXX` - Clearly fake key pattern +- `sk-XXXXXXXXXXXXXXXX` - Clearly fake key pattern. **Caveat:** when assigned to + a credential-ish name (`API_KEY = "sk-XXXXXXXXXXXXXXXX"`), detect-secrets' + `KeywordDetector` still reports it as `Secret Keyword` — its placeholder + filters recognise `token-XXXX-XXXX-XXXX` and `your-api-key-here` but not this + form. Either append `# pragma: allowlist secret` or keep the value out of a + keyword-adjacent assignment. `scripts/test-secret-detection.sh` asserts this + real behaviour. - `api-key-XXXX-XXXX-XXXX` - Placeholder pattern - `` - Clear placeholder - `YOUR_API_KEY_HERE` - Uppercase placeholder @@ -36,9 +42,78 @@ uv run detect-secrets scan tests/secret_patterns_reference.py ## Other - `secret-XXXX` - Generic secret placeholder - `` - Clear placeholder +- `` - Graphistry personal key ID placeholder +- `` - Graphistry personal key secret placeholder - `localhost` or `example.com` - Safe domains - `user@example.com` - Example email +Graphistry `personal_key_id` and `personal_key_secret` assignments receive an +additional deterministic tracked-file check because short keys may not meet +generic entropy thresholds. The checker reports only the file, line, and key +name; it never prints the value. + +## Internal names: what is screened where + +`scripts/ci/check_credential_literals.py` rejects internal hostnames +(`*.grph.xyz`, `*.louie.internal`) in tracked files. Use RFC 2606 example +domains (`louie.example.com`) in fixtures, or the public endpoints +(`hub.graphistry.com`, `https://den.louie.ai`) in user-facing docs. + +The rule is deliberately **domain-level, not a list of known hosts** — an +enumerated list misses the next subdomain nobody thought of. + +Specific account and organisation names are screened **locally only**, in +`.git/hooks/pre-commit`. That is not an oversight: this repository is public, so +a tracked denylist of the private strings we are trying to keep out would +publish them. Anything generalizable enough to express as a pattern belongs in +the tracked gate; anything that is itself the sensitive value stays local. The +local hook is not installed by default and CI does not run it, so treat it as a +convenience, not a control. + +## Writing tests that contain deliberate fake secrets + +Security tests must plant realistic secrets to prove the gates reject them — and +the gates then flag those fixtures. This is the gates working, not a bug. Test +files are deliberately **not** exempt: the original credential leak in this repo +was duplicated into `tests/unit/test_org_auth_flow.py`, so a blanket exclusion +for `tests/` would have hidden it. + +Three ways to write a fixture, in order of preference: + +1. **Assemble it at runtime** so no scannable literal exists in the file: + + ```python + FAKE_SECRET = "A1B2C3D4" + "E5F6G7H8" # pragma: allowlist secret + KEY_SECRET = "personal_key_" + "secret" # pragma: allowlist secret + ``` + + This is what `tests/unit/security/test_credential_literals.py` does throughout. + + Runtime assembly defeats the *value*-matching rules, but `detect-secrets`' + `KeywordDetector` fires on the **variable name** (anything containing `key`, + `secret`, `password`, `token`) regardless of the value. So a constant named + `FAKE_SECRET` still needs a pragma even when its value is assembled — the two + techniques compose rather than substitute. + +2. **Add a same-line pragma** when the literal must be inline: + + ```python + token = "zQ3RtP8xL2mN7vB4kW9jH6dF1sA5gY0c" # pragma: allowlist secret + ``` + + It must be on the **same line** as the value — both `detect-secrets` and + `check_credential_literals.py` match per line. A pragma on the preceding line + does nothing. + +3. **Exclude the path** for files that cannot carry a comment at all — an + `.ipynb` data URI, a CSV row — via + `check_credential_literals.py --exclude-files `. Prefer 1 or 2; an + exclusion is a permanent hole. + +Never re-baseline to silence a fixture. `.secrets.baseline` accepts a value +everywhere in the tree, so baselining a fake secret also accepts a real one that +happens to match. + Note: Real patterns to avoid: - Actual API keys (even expired ones) - Real passwords (even old ones) diff --git a/.secrets.baseline b/.secrets.baseline index 070c28f..ab8178f 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -90,10 +90,6 @@ { "path": "detect_secrets.filters.allowlist.is_line_allowlisted" }, - { - "path": "detect_secrets.filters.common.is_baseline_file", - "filename": ".secrets.baseline" - }, { "path": "detect_secrets.filters.common.is_ignored_due_to_verification_policies", "min_level": 2 @@ -128,18 +124,299 @@ { "path": "detect_secrets.filters.regex.should_exclude_file", "pattern": [ - "^(plans/|tmp/)" + "^(plans/|tmp/|\\.secrets\\.baseline$)" ] } ], "results": { + ".secret-patterns.md": [ + { + "type": "Secret Keyword", + "filename": ".secret-patterns.md", + "hashed_secret": "c504a704bcbb30b4adbb024a51a591615184162d", + "is_verified": false, + "line_number": 27 + } + ], + "README.md": [ + { + "type": "Secret Keyword", + "filename": "README.md", + "hashed_secret": "cbfdac6008f9cab4083784cbd1874f76618d2a97", + "is_verified": false, + "line_number": 71 + } + ], + "ai/README.md": [ + { + "type": "Secret Keyword", + "filename": "ai/README.md", + "hashed_secret": "e5e9fa1ba31ecd1ae84f75caaa474f3a663f05f4", + "is_verified": false, + "line_number": 74 + }, + { + "type": "Secret Keyword", + "filename": "ai/README.md", + "hashed_secret": "9d4e1e23bd5b727046a9e3b4b7db57bd8d6ee684", + "is_verified": false, + "line_number": 218 + } + ], + "ai/prompts/PLAN.md": [ + { + "type": "Secret Keyword", + "filename": "ai/prompts/PLAN.md", + "hashed_secret": "e5e9fa1ba31ecd1ae84f75caaa474f3a663f05f4", + "is_verified": false, + "line_number": 130 + } + ], + "docs/api/client.md": [ + { + "type": "Secret Keyword", + "filename": "docs/api/client.md", + "hashed_secret": "8c2e82f6f61db7f03a0418b0b2e30b893376e084", + "is_verified": false, + "line_number": 18 + }, + { + "type": "Secret Keyword", + "filename": "docs/api/client.md", + "hashed_secret": "a94a8fe5ccb19ba61c4c0873d391e987982fbbd3", + "is_verified": false, + "line_number": 134 + }, + { + "type": "Secret Keyword", + "filename": "docs/api/client.md", + "hashed_secret": "3c1046153e46b813c467bcb0b73680035cc0f722", + "is_verified": false, + "line_number": 166 + } + ], + "docs/api/index.md": [ + { + "type": "Secret Keyword", + "filename": "docs/api/index.md", + "hashed_secret": "8c2e82f6f61db7f03a0418b0b2e30b893376e084", + "is_verified": false, + "line_number": 41 + }, + { + "type": "Secret Keyword", + "filename": "docs/api/index.md", + "hashed_secret": "9d4e1e23bd5b727046a9e3b4b7db57bd8d6ee684", + "is_verified": false, + "line_number": 116 + } + ], + "docs/api/notebook.md": [ + { + "type": "Secret Keyword", + "filename": "docs/api/notebook.md", + "hashed_secret": "10b416509494dfe946965c15b8871704b30859e7", + "is_verified": false, + "line_number": 309 + } + ], + "docs/getting-started/authentication.md": [ + { + "type": "Secret Keyword", + "filename": "docs/getting-started/authentication.md", + "hashed_secret": "8c2e82f6f61db7f03a0418b0b2e30b893376e084", + "is_verified": false, + "line_number": 16 + } + ], + "docs/getting-started/notebooks/02-data-upload.ipynb": [ + { + "type": "Base64 High Entropy String", + "filename": "docs/getting-started/notebooks/02-data-upload.ipynb", + "hashed_secret": "913a7f4984130d56498b04caca2a8da8d01672e9", + "is_verified": false, + "line_number": 504 + } + ], + "docs/getting-started/notebooks/05-vibes-investigation-I.ipynb": [ + { + "type": "Secret Keyword", + "filename": "docs/getting-started/notebooks/05-vibes-investigation-I.ipynb", + "hashed_secret": "10b416509494dfe946965c15b8871704b30859e7", + "is_verified": false, + "line_number": 162 + } + ], + "docs/getting-started/notebooks/06-vibes-investigation-II-planning.ipynb": [ + { + "type": "Secret Keyword", + "filename": "docs/getting-started/notebooks/06-vibes-investigation-II-planning.ipynb", + "hashed_secret": "10b416509494dfe946965c15b8871704b30859e7", + "is_verified": false, + "line_number": 179 + }, + { + "type": "AWS Access Key", + "filename": "docs/getting-started/notebooks/06-vibes-investigation-II-planning.ipynb", + "hashed_secret": "fb9ffcf2749d13f5af963674689fe861c687309a", + "is_verified": false, + "line_number": 1574 + } + ], + "docs/guides/authentication.md": [ + { + "type": "Secret Keyword", + "filename": "docs/guides/authentication.md", + "hashed_secret": "8c2e82f6f61db7f03a0418b0b2e30b893376e084", + "is_verified": false, + "line_number": 34 + }, + { + "type": "Secret Keyword", + "filename": "docs/guides/authentication.md", + "hashed_secret": "5491af3e7c63f27d6338b16f62a4e57398b910bf", + "is_verified": false, + "line_number": 101 + }, + { + "type": "Secret Keyword", + "filename": "docs/guides/authentication.md", + "hashed_secret": "e845c949d6a6312f004c8e0bd160902c713442d4", + "is_verified": false, + "line_number": 175 + }, + { + "type": "Secret Keyword", + "filename": "docs/guides/authentication.md", + "hashed_secret": "2d05197fb19549743ca8b5cd3d5a734ac859d943", + "is_verified": false, + "line_number": 180 + } + ], + "docs/guides/datathreads.md": [ + { + "type": "Secret Keyword", + "filename": "docs/guides/datathreads.md", + "hashed_secret": "8c2e82f6f61db7f03a0418b0b2e30b893376e084", + "is_verified": false, + "line_number": 22 + } + ], "docs/index.md": [ { "type": "Secret Keyword", "filename": "docs/index.md", "hashed_secret": "564e340cd48437d2dfe876ee154cc99dc4d0d137", "is_verified": false, - "line_number": 51 + "line_number": 49 + } + ], + "scripts/build-notebook.sh": [ + { + "type": "Secret Keyword", + "filename": "scripts/build-notebook.sh", + "hashed_secret": "10b416509494dfe946965c15b8871704b30859e7", + "is_verified": false, + "line_number": 30 + } + ], + "scripts/ci/secret-detection.sh": [ + { + "type": "Secret Keyword", + "filename": "scripts/ci/secret-detection.sh", + "hashed_secret": "3c2ae4bf3ee04c18976cbbe260adb8a5a966fdab", + "is_verified": false, + "line_number": 71 + } + ], + "scripts/test-secret-detection.sh": [ + { + "type": "Secret Keyword", + "filename": "scripts/test-secret-detection.sh", + "hashed_secret": "11be729a00576850b44eca4ff07d6b9d45eb9840", + "is_verified": false, + "line_number": 72 + }, + { + "type": "Secret Keyword", + "filename": "scripts/test-secret-detection.sh", + "hashed_secret": "c871817b4ddd7dde1e07fb39507a1e72b0f8a41b", + "is_verified": false, + "line_number": 118 + }, + { + "type": "Secret Keyword", + "filename": "scripts/test-secret-detection.sh", + "hashed_secret": "f9f655f33d66adb729ad561c0986f933ad8cc283", + "is_verified": false, + "line_number": 119 + }, + { + "type": "Base64 High Entropy String", + "filename": "scripts/test-secret-detection.sh", + "hashed_secret": "4c68d86edd3a0949872369f7b355b18ffa1e6f34", + "is_verified": false, + "line_number": 120 + }, + { + "type": "Secret Keyword", + "filename": "scripts/test-secret-detection.sh", + "hashed_secret": "bd26f4861decc205d376dfb2e051b288d4707f08", + "is_verified": false, + "line_number": 121 + }, + { + "type": "Secret Keyword", + "filename": "scripts/test-secret-detection.sh", + "hashed_secret": "982c877b05e53ccde59d258e56a256ca2cc6e154", + "is_verified": false, + "line_number": 122 + }, + { + "type": "Secret Keyword", + "filename": "scripts/test-secret-detection.sh", + "hashed_secret": "c504a704bcbb30b4adbb024a51a591615184162d", + "is_verified": false, + "line_number": 133 + }, + { + "type": "Secret Keyword", + "filename": "scripts/test-secret-detection.sh", + "hashed_secret": "897ca6fcdeed5883fd7bd85eae55406ac81d9d74", + "is_verified": false, + "line_number": 146 + }, + { + "type": "Secret Keyword", + "filename": "scripts/test-secret-detection.sh", + "hashed_secret": "b0d87f19ffb7b6c921110bf605cdcb5a96bd064a", + "is_verified": false, + "line_number": 148 + } + ], + "src/louieai/__init__.py": [ + { + "type": "Secret Keyword", + "filename": "src/louieai/__init__.py", + "hashed_secret": "3c1046153e46b813c467bcb0b73680035cc0f722", + "is_verified": false, + "line_number": 156 + } + ], + "src/louieai/_client.py": [ + { + "type": "Secret Keyword", + "filename": "src/louieai/_client.py", + "hashed_secret": "9d4e1e23bd5b727046a9e3b4b7db57bd8d6ee684", + "is_verified": false, + "line_number": 178 + }, + { + "type": "Secret Keyword", + "filename": "src/louieai/_client.py", + "hashed_secret": "11c31bcdb36cf14121c0474d5948e332c7286386", + "is_verified": false, + "line_number": 321 } ], "tests/integration/notebook/test_cursor_new_integration.py": [ @@ -150,7 +427,223 @@ "is_verified": false, "line_number": 57 } + ], + "tests/integration/test_notebook_experience.py": [ + { + "type": "Secret Keyword", + "filename": "tests/integration/test_notebook_experience.py", + "hashed_secret": "c94d65f02a652d11c2e5c2e1ccf38dce5a076e1e", + "is_verified": false, + "line_number": 211 + } + ], + "tests/secret_patterns_reference.py": [ + { + "type": "Secret Keyword", + "filename": "tests/secret_patterns_reference.py", + "hashed_secret": "a4b571af307c612572294b9be601877ff5e1a726", + "is_verified": false, + "line_number": 24 + }, + { + "type": "Secret Keyword", + "filename": "tests/secret_patterns_reference.py", + "hashed_secret": "02f15a9a86e4046074411b7d96681e7cebd67391", + "is_verified": false, + "line_number": 25 + }, + { + "type": "Secret Keyword", + "filename": "tests/secret_patterns_reference.py", + "hashed_secret": "cb2a62c1642c1c6aa0f51ce8cc055638fa80eaf1", + "is_verified": false, + "line_number": 27 + }, + { + "type": "Secret Keyword", + "filename": "tests/secret_patterns_reference.py", + "hashed_secret": "cbfdac6008f9cab4083784cbd1874f76618d2a97", + "is_verified": false, + "line_number": 49 + }, + { + "type": "Secret Keyword", + "filename": "tests/secret_patterns_reference.py", + "hashed_secret": "edbd1887e772e13c251f688a5f10c1ffbb67960d", + "is_verified": false, + "line_number": 50 + }, + { + "type": "Secret Keyword", + "filename": "tests/secret_patterns_reference.py", + "hashed_secret": "66b9714c79ff50cdcf0b8ee8fdfdaeb02400b992", + "is_verified": false, + "line_number": 57 + } + ], + "tests/test_image_support.py": [ + { + "type": "Base64 High Entropy String", + "filename": "tests/test_image_support.py", + "hashed_secret": "aad75d7bf162c77a82c7d449bdd876fe20d3bbe4", + "is_verified": false, + "line_number": 13 + } + ], + "tests/unit/mocks.py": [ + { + "type": "Secret Keyword", + "filename": "tests/unit/mocks.py", + "hashed_secret": "9fb7fe1217aed442b04c0f5e43b5d5a7d3287097", + "is_verified": false, + "line_number": 235 + }, + { + "type": "Secret Keyword", + "filename": "tests/unit/mocks.py", + "hashed_secret": "767ef7376d44bb6e52b390ddcd12c1cb1b3902a4", + "is_verified": false, + "line_number": 236 + }, + { + "type": "Secret Keyword", + "filename": "tests/unit/mocks.py", + "hashed_secret": "aa6bc2314e1e04b584339c3687858ef35f876773", + "is_verified": false, + "line_number": 238 + } + ], + "tests/unit/notebook/test_api_key_auth.py": [ + { + "type": "Secret Keyword", + "filename": "tests/unit/notebook/test_api_key_auth.py", + "hashed_secret": "312af541454cb275e8486c9bf41e30b26f91bd24", + "is_verified": false, + "line_number": 28 + }, + { + "type": "Secret Keyword", + "filename": "tests/unit/notebook/test_api_key_auth.py", + "hashed_secret": "6af5a378fdb0e7d397c9f47c744e5e14195c0228", + "is_verified": false, + "line_number": 56 + }, + { + "type": "Secret Keyword", + "filename": "tests/unit/notebook/test_api_key_auth.py", + "hashed_secret": "206c80413b9a96c1312cc346b7d2517b84463edd", + "is_verified": false, + "line_number": 83 + }, + { + "type": "Secret Keyword", + "filename": "tests/unit/notebook/test_api_key_auth.py", + "hashed_secret": "8bdf6173f31ee9447a2b289e0374eea2dac7b57b", + "is_verified": false, + "line_number": 115 + }, + { + "type": "Secret Keyword", + "filename": "tests/unit/notebook/test_api_key_auth.py", + "hashed_secret": "8be8adcd67ecd47e99d7ac1894b40b2fce30693b", + "is_verified": false, + "line_number": 147 + } + ], + "tests/unit/test_auth.py": [ + { + "type": "Secret Keyword", + "filename": "tests/unit/test_auth.py", + "hashed_secret": "c94d65f02a652d11c2e5c2e1ccf38dce5a076e1e", + "is_verified": false, + "line_number": 195 + }, + { + "type": "Secret Keyword", + "filename": "tests/unit/test_auth.py", + "hashed_secret": "ed65c049bb2f78ee4f703b2158ba9cc6ea31fb7e", + "is_verified": false, + "line_number": 256 + } + ], + "tests/unit/test_client.py": [ + { + "type": "Secret Keyword", + "filename": "tests/unit/test_client.py", + "hashed_secret": "9d4e1e23bd5b727046a9e3b4b7db57bd8d6ee684", + "is_verified": false, + "line_number": 102 + }, + { + "type": "Secret Keyword", + "filename": "tests/unit/test_client.py", + "hashed_secret": "c94d65f02a652d11c2e5c2e1ccf38dce5a076e1e", + "is_verified": false, + "line_number": 526 + }, + { + "type": "Secret Keyword", + "filename": "tests/unit/test_client.py", + "hashed_secret": "3acfb2c2b433c0ea7ff107e33df91b18e52f960f", + "is_verified": false, + "line_number": 527 + } + ], + "tests/unit/test_documentation.py": [ + { + "type": "Secret Keyword", + "filename": "tests/unit/test_documentation.py", + "hashed_secret": "9fb7fe1217aed442b04c0f5e43b5d5a7d3287097", + "is_verified": false, + "line_number": 105 + }, + { + "type": "Secret Keyword", + "filename": "tests/unit/test_documentation.py", + "hashed_secret": "767ef7376d44bb6e52b390ddcd12c1cb1b3902a4", + "is_verified": false, + "line_number": 106 + }, + { + "type": "Secret Keyword", + "filename": "tests/unit/test_documentation.py", + "hashed_secret": "aa6bc2314e1e04b584339c3687858ef35f876773", + "is_verified": false, + "line_number": 108 + } + ], + "tests/unit/test_louie_factory.py": [ + { + "type": "Secret Keyword", + "filename": "tests/unit/test_louie_factory.py", + "hashed_secret": "206c80413b9a96c1312cc346b7d2517b84463edd", + "is_verified": false, + "line_number": 55 + }, + { + "type": "Secret Keyword", + "filename": "tests/unit/test_louie_factory.py", + "hashed_secret": "dc1abc7f0c453564fa2538999dde8c8513fa1006", + "is_verified": false, + "line_number": 88 + } + ], + "tests/unit/test_org_auth_flow.py": [ + { + "type": "Secret Keyword", + "filename": "tests/unit/test_org_auth_flow.py", + "hashed_secret": "d3b354a33dc2109a2146d7523a297b04012940f7", + "is_verified": false, + "line_number": 27 + }, + { + "type": "Secret Keyword", + "filename": "tests/unit/test_org_auth_flow.py", + "hashed_secret": "72cb70dbbafe97e5ea13ad88acd65d08389439b0", + "is_verified": false, + "line_number": 112 + } ] }, - "generated_at": "2026-01-25T06:11:40Z" + "generated_at": "2026-07-25T21:12:14Z" } diff --git a/DEVELOP.md b/DEVELOP.md index d1859b2..fedf750 100644 --- a/DEVELOP.md +++ b/DEVELOP.md @@ -37,7 +37,9 @@ This guide covers the technical setup and development workflow for contributors ### Environment Variables -For integration testing, create a `.env` file: +For integration testing, create a `.env` file. Note that `.env` is only +read when `LOUIE_TEST_MODE=integration` (or `all`) — see +[docs/developer/testing.md](docs/developer/testing.md): ```bash GRAPHISTRY_USERNAME=your_username diff --git a/README.md b/README.md index c9253f9..5bd40e0 100644 --- a/README.md +++ b/README.md @@ -53,8 +53,8 @@ LouieAI uses PyGraphistry for authentication. You'll need a free account: ```bash # Option 1: Environment variables (recommended for notebooks/scripts) -export GRAPHISTRY_USERNAME="sarah@analytics.com" -export GRAPHISTRY_PASSWORD="Analytics2024!" +export GRAPHISTRY_USERNAME="" +export GRAPHISTRY_PASSWORD="" export GRAPHISTRY_SERVER="hub.graphistry.com" # or "my-company.graphistry.com" # Optional: Custom Louie endpoint (defaults to https://louie.ai) diff --git a/docs/developer/testing.md b/docs/developer/testing.md index 20f8a6d..ba0ccd6 100644 --- a/docs/developer/testing.md +++ b/docs/developer/testing.md @@ -31,8 +31,16 @@ GRAPHISTRY_USERNAME=your_username GRAPHISTRY_PASSWORD=your_password ``` +`.env` is **opt-in**: it is only read when `LOUIE_TEST_MODE` selects an +integration mode. Without that, credential-gated tests skip, so a run with no +credentials stays offline instead of dialling the real service. + ```bash -./bin/uv run pytest tests/integration/ -v +# Either set the mode explicitly... +LOUIE_TEST_MODE=integration ./bin/uv run pytest tests/integration/ -v + +# ...or use the wrapper, which sets it for you +./scripts/test.sh --integration ``` ## Writing Tests diff --git a/plan.md b/plan.md index 2464bfb..8ae6393 100644 --- a/plan.md +++ b/plan.md @@ -32,13 +32,13 @@ # DO NOT COMMIT - Keep in .gitignore g = graphistry.register( api=3, - server='graphistry-dev.grph.xyz', - personal_key_id='CU5V6VZJB7', - personal_key_secret='32RBP6PUCSUVAIYJ', - org_name='databricks-pat-botsv3' + server='', + personal_key_id='', + personal_key_secret='', + org_name='' ) -lui = louieai.louie(g, server_url='https://louie-dev.grph.xyz', share_mode='Private') +lui = louieai.louie(g, server_url='', share_mode='Private') ``` ## Verification diff --git a/pyproject.toml b/pyproject.toml index d6f522d..1529b54 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -68,6 +68,13 @@ docs = [ ] # We define a separate "docs" extra in case readthedocs needs to install just docs requirements. +[tool.uv] +# Pin the resolution cutoff declaratively. The lockfile already recorded +# `exclude-newer`, but nothing declared it — it had been passed as a one-off CLI +# flag, so every `uv lock --check` reported the option as "removed" and demanded +# a re-resolve. That is what made `uv sync --locked` unusable in CI. +exclude-newer = "2026-04-02T23:28:24Z" + [tool.setuptools.packages.find] where = ["src"] include = ["louieai*"] @@ -113,7 +120,11 @@ disallow_any_decorated = false # Would be too restrictive exclude = [ "templates/", "tests/", - "docs/" + "docs/", + # Gitignored local working memory (see .gitignore). Absent in CI, but a + # developer's copy holds standalone code fragments with intentionally + # unresolvable relative imports, so `mypy .` fails locally without this. + "plans/", ] disallow_any_explicit = false # Would be too restrictive disallow_any_generics = true diff --git a/scripts/ci/check_credential_literals.py b/scripts/ci/check_credential_literals.py new file mode 100644 index 0000000..a6bee78 --- /dev/null +++ b/scripts/ci/check_credential_literals.py @@ -0,0 +1,327 @@ +#!/usr/bin/env python3 +"""Reject hard-coded Graphistry personal keys without printing their values. + +`detect-secrets` entropy heuristics miss short Graphistry personal keys, so this +adds a deterministic gate. It reports only path, line, and rule name — never the +matched value — so CI logs cannot leak what the check is protecting. + +Deliberately stdlib-only: it must run inside a pre-commit hook before project +dependencies (or a virtualenv) necessarily exist, which is why it is invoked as +`python3` rather than `uv run` despite `ai/README.md`'s general guidance. + +Two independent rules run over every tracked text file: + +`value-shape` + A standalone token matching the Graphistry personal-key shape, matched + *without* requiring surrounding quotes. Quoting is not a reliable signal: + the same value appears unquoted in `.env` and shell exports, backslash-quoted + inside `.ipynb` JSON, and bare in YAML and Markdown. Matching the token + itself covers all of them. Requiring a letter/digit mix keeps ordinary + all-caps words (`"PRODUCTION"`, `"LINESTRING"`) from tripping the gate. + +`key-context` + A value assigned to a `personal_key_id` / `personal_key_secret` identifier + that does not match the known shape but is not an obvious placeholder. This + is the forward-compatibility net for a future key format, so it accepts + `GRAPHISTRY_`-prefixed names and `=`, `:`, and `,` separators. + +`internal-host` + An internal hostname. Test fixtures and docs should use RFC 2606 example + domains or the public endpoints, not real infrastructure names. + + Only *generalizable* patterns live here. A public repository cannot carry a + denylist of the specific account or organisation names it is trying to keep + out, because the denylist publishes them; those belong in a local + `.git/hooks/pre-commit` (or a gitignored pattern file), not in tracked + source. A domain regex reveals nothing beyond public DNS and covers every + subdomain, including ones nobody has thought of yet. + +Suppress a genuine false positive with a trailing `# pragma: allowlist secret` +(the same marker `detect-secrets` uses). + +Exit 1 on any finding. +""" + +from __future__ import annotations + +import argparse +import re +import subprocess +import sys +from collections.abc import Iterable, Sequence +from pathlib import Path + +# Observed Graphistry personal-key shapes: uppercase alphanumeric, fixed widths. +_SHAPES: dict[str, int] = {"personal_key_id": 10, "personal_key_secret": 16} +_SHAPE_WIDTHS = sorted(set(_SHAPES.values())) +_VALUE_SHAPE = re.compile( + # `+` and `/` are base64-interior characters and `=` is base64 padding, so a + # random 10/16-char uppercase run inside an embedded image blob would + # otherwise match — measured ~1 per 400 KB, and an `.ipynb` data URI has no + # place to put a `# pragma` escape. Excluding `+/` on both sides and `=` on + # the right removes every such hit (0 across 4 MB of random base64) while + # still matching `KEY=VALUE` in `.env`, where `=` *precedes* the token. + "(?personal_key_(?:id|secret))\b + ["']? + [ \t]* (?: : [^=:,\r\n]{0,40} )? [ \t]* + (?: = | : | , ) + [ \t]* + (?P["']) + (?P[^"'\r\n]*) + (?P=quote) + """, + re.IGNORECASE | re.VERBOSE, +) + +# Internal infrastructure domains. Deliberately a domain-level pattern, not a +# list of known hosts: the local hook enumerated two specific ones, so any new +# subdomain passed silently. +_INTERNAL_HOSTS = re.compile( + r"(? bool: + return any(c.isalpha() for c in value) and any(c.isdigit() for c in value) + + +def _is_placeholder(value: str) -> bool: + """True when the literal is obviously not a real credential.""" + candidate = value.strip() + lowered = candidate.lower() + if not candidate: + return True + # , ${ENV_VAR}, {format_slot} + if candidate.startswith(("<", "${", "{")) and candidate.endswith((">", "}")): + return True + if any(marker in lowered for marker in _PLACEHOLDER_MARKERS): + return True + if lowered.startswith(_PLACEHOLDER_PREFIXES): + return True + if "..." in candidate: # elided doc sample, e.g. "sk_123..." + return True + # Single repeated filler character, e.g. XXXXXXXXXX or ---------- + return len(set(candidate)) <= 1 or set(candidate) <= {"X", "x", "*", ".", "-", "_"} + + +# A snake_case identifier that names a credential field is a *name*, not a +# value: `{"personal_key_id": 10, "personal_key_secret": 16}` would otherwise +# parse as an assignment of the string "personal_key_secret". +_FIELD_NAME_MARKERS = ("key", "secret", "password", "token", "credential") + + +def _is_field_name(value: str) -> bool: + candidate = value.strip() + return ( + candidate.isidentifier() + and candidate.islower() + and any(marker in candidate for marker in _FIELD_NAME_MARKERS) + ) + + +def _looks_like_secret(value: str) -> bool: + """True when an off-shape value still has credential-grade entropy. + + Letter **or** digit, not both: a purely numeric key id and a purely + alphabetic passphrase are both realistic credential formats, and requiring + the conjunction vetoed them. + """ + candidate = value.strip() + if len(candidate) < _MIN_SECRET_LENGTH: + return False + if len(set(candidate)) < _MIN_SECRET_DISTINCT: + return False + if _is_field_name(candidate): + return False + return any(c.isalnum() for c in candidate) + + +def _tracked_paths(*, staged: bool) -> list[str]: + # --no-renames: with rename detection on, `git mv clean.py config.py` plus an + # edit reports only `R`, which --diff-filter=ACM drops — letting a credential + # through pre-commit via rename+edit. + command = ( + [ + "git", + "diff", + "--cached", + "--no-renames", + "--name-only", + "--diff-filter=ACM", + "-z", + ] + if staged + else ["git", "ls-files", "-z"] + ) + output = subprocess.check_output(command) + return [item.decode("utf-8") for item in output.split(b"\0") if item] + + +def _read_text(path: str, *, staged: bool) -> str | None: + """Return decoded text, or None for binaries, symlinks, and unreadable paths.""" + if staged: + raw = subprocess.check_output(["git", "show", f":{path}"]) + else: + candidate = Path(path) + if candidate.is_symlink() or not candidate.is_file(): + return None + raw = candidate.read_bytes() + if b"\0" in raw: # binary + return None + try: + return raw.decode("utf-8") + except UnicodeDecodeError: + return None + + +def _line_of(text: str, offset: int) -> int: + return text.count("\n", 0, offset) + 1 + + +def _allowlisted(lines: Sequence[str], line_number: int) -> bool: + index = line_number - 1 + return 0 <= index < len(lines) and _ALLOWLIST_PRAGMA in lines[index] + + +def _find(text: str) -> Iterable[tuple[int, str, str]]: + """Yield (line, rule, label) for each finding. Never yields the value.""" + lines = text.splitlines() + seen: set[tuple[int, str]] = set() + + for match in _VALUE_SHAPE.finditer(text): + value = match.group(1) + if not _has_letter_and_digit(value) or _is_placeholder(value): + continue + line = _line_of(text, match.start()) + if _allowlisted(lines, line): + continue + label = next( + (name for name, width in _SHAPES.items() if width == len(value)), + "personal key", + ) + if (line, label) not in seen: + seen.add((line, label)) + yield line, "value-shape", label + + for match in _KEY_CONTEXT.finditer(text): + value = match.group("value") + if _is_placeholder(value): + continue + # Skip only if value-shape *actually* reported it. Testing the shape + # regex alone would drop a numeric-only value: it matches the shape but + # value-shape rejects it on the letter/digit filter, so it would fall + # through both rules. + if _VALUE_SHAPE.fullmatch(value) and _has_letter_and_digit(value): + continue + if not _looks_like_secret(value): + continue # short/low-entropy mock such as "pk_123" or "MY_KEY_ID" + line = _line_of(text, match.start()) + if _allowlisted(lines, line): + continue + label = match.group("key").lower() + if (line, label) not in seen: + seen.add((line, label)) + yield line, "key-context", label + + for match in _INTERNAL_HOSTS.finditer(text): + line = _line_of(text, match.start()) + if _allowlisted(lines, line): + continue + if (line, "internal host") not in seen: + seen.add((line, "internal host")) + yield line, "internal-host", "internal host" + + +def main(argv: Sequence[str] | None = None) -> int: + parser = argparse.ArgumentParser(description="Reject hard-coded personal keys.") + parser.add_argument( + "--staged", + action="store_true", + help="scan staged index content instead of the working tree", + ) + parser.add_argument( + "--exclude-files", + default=None, + metavar="REGEX", + help=( + "skip paths matching this regex. Escape hatch for files that cannot " + "carry a '# pragma: allowlist secret' line, such as an .ipynb data " + "URI or a CSV row." + ), + ) + parser.add_argument("paths", nargs="*", help="explicit paths (default: tracked)") + args = parser.parse_args(argv) + + excluded = re.compile(args.exclude_files) if args.exclude_files else None + explicit = bool(args.paths) + paths = args.paths or _tracked_paths(staged=args.staged) + + failed = False + for path in paths: + if excluded is not None and excluded.search(path): + continue + try: + text = _read_text(path, staged=args.staged and not explicit) + except (OSError, subprocess.CalledProcessError): + continue + if text is None: + continue + for line, rule, label in _find(text): + print( + f"{path}:{line}: hard-coded {label} literal [{rule}]; " + "use an environment variable or an explicit placeholder", + file=sys.stderr, + ) + failed = True + + if failed: + print( + "\nValues are intentionally not printed.\n" + " Real credential -> replace with an environment variable or a " + "placeholder such as .\n" + " Deliberate test fixture -> build it at runtime so no literal exists, " + 'e.g. FAKE = "A1B2C3D4" + "E5F6G7H8"; or append ' + f"'# {_ALLOWLIST_PRAGMA}' on the SAME line.\n" + " File that cannot hold a comment (.ipynb data URI, CSV) -> " + "--exclude-files .\n" + "See .secret-patterns.md, 'Writing tests that contain deliberate fake " + "secrets'.", + file=sys.stderr, + ) + return 1 if failed else 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/ci/check_new_secrets.py b/scripts/ci/check_new_secrets.py new file mode 100644 index 0000000..0465292 --- /dev/null +++ b/scripts/ci/check_new_secrets.py @@ -0,0 +1,140 @@ +#!/usr/bin/env python3 +"""Fail when a detect-secrets scan finds anything not already in the baseline. + +`scripts/ci/secret-detection.sh` used to gate on the exit code of + + detect-secrets scan --baseline .secrets.baseline + +That command is an *update* operation, not an assertion: it rewrites the baseline +file in place, writes nothing to stdout, and **exits 0 no matter what it finds**. +Both the CI and pre-commit gates branched on `|| print_error`, so neither branch +was reachable and a known-live credential passed both. This module supplies the +comparison the shell was assuming. + +Reads a scan JSON (stdin or a path) and compares its findings against the +baseline by `hashed_secret`. Reports `path:line:type` only — never a value. + +Exit 1 when a finding is not in the baseline. +""" + +from __future__ import annotations + +import argparse +import json +import sys +from collections.abc import Sequence +from pathlib import Path +from typing import Any + +Finding = tuple[str, int, str] + + +def _load(source: str | None) -> dict[str, Any]: + """Parse one *or more* concatenated scan objects and union their results. + + `xargs` splits on ARG_MAX, so a large commit produces several + back-to-back JSON documents on one stream. A single `json.loads` then fails + with `Extra data: line N`, which surfaced as "New secrets detected!" on a + tree with no secrets — fail-closed, but blaming the developer for the wrong + thing. + """ + text = Path(source).read_text(encoding="utf-8") if source else sys.stdin.read() + if not text.strip(): + # An empty scan is not "clean" — it usually means the scan itself failed + # (wrong cwd, bad args). Treat it as an error rather than a pass. + raise ValueError("empty scan output; the detect-secrets scan did not run") + + decoder = json.JSONDecoder() + merged: dict[str, Any] = {"results": {}} + index = 0 + while index < len(text): + while index < len(text) and text[index].isspace(): + index += 1 + if index >= len(text): + break + document, index = decoder.raw_decode(text, index) + if not isinstance(document, dict) or "results" not in document: + # detect-secrets always emits a `results` key. Its absence means we + # are not looking at a scan, and treating that as "no findings" + # would be another silent pass. + raise ValueError( + "scan output has no 'results' key; not a detect-secrets scan" + ) + for path, entries in document["results"].items(): + merged["results"].setdefault(path, []).extend(entries) + return merged + + +def baseline_hashes(baseline: dict[str, Any]) -> set[tuple[str, str]]: + """Accepted findings, keyed by `(path, hashed_secret)`. + + Keying on the hash alone would accept a value *anywhere* once it is + baselined in one place — so a demo password allowlisted in `docs/` would + pass silently if copied into `src/louieai/_client.py`, which is exactly + where this repo's incident happened. detect-secrets' own baseline semantics + are per-file; match them. + """ + return { + (path, entry["hashed_secret"]) + for path, entries in baseline.get("results", {}).items() + for entry in entries + if "hashed_secret" in entry + } + + +def new_findings(scan: dict[str, Any], known: set[tuple[str, str]]) -> list[Finding]: + """Findings in `scan` not already accepted for that same path.""" + found: list[Finding] = [] + for path, entries in sorted(scan.get("results", {}).items()): + for entry in entries: + if (path, entry.get("hashed_secret")) in known: + continue + found.append( + (path, int(entry.get("line_number", 0)), str(entry.get("type", "?"))) + ) + return found + + +def main(argv: Sequence[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--baseline", default=".secrets.baseline") + parser.add_argument("--scan", default=None, help="scan JSON path (default: stdin)") + args = parser.parse_args(argv) + + try: + scan = _load(args.scan) + except (OSError, ValueError, json.JSONDecodeError) as exc: + print(f"error: could not read detect-secrets scan: {exc}", file=sys.stderr) + return 1 + + try: + baseline = json.loads(Path(args.baseline).read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError) as exc: + print(f"error: could not read baseline {args.baseline}: {exc}", file=sys.stderr) + return 1 + + findings = new_findings(scan, baseline_hashes(baseline)) + if not findings: + return 0 + + for path, line, kind in findings: + print(f"{path}:{line}: new secret detected [{kind}]", file=sys.stderr) + print( + f"\n{len(findings)} finding(s) not in {args.baseline}. Values are not " + "printed.\n" + " Real credential -> remove it and use an environment variable.\n" + " Deliberate test fixture -> build it at runtime so no literal exists, or " + "append '# pragma: allowlist secret' on the SAME line.\n" + " Genuine false positive -> re-baseline with " + f"`detect-secrets scan > {args.baseline}` and review the diff. Note a " + "baseline entry accepts that value ANYWHERE in the tree, so prefer a " + "pragma for a one-off.\n" + "See .secret-patterns.md, 'Writing tests that contain deliberate fake " + "secrets'.", + file=sys.stderr, + ) + return 1 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/ci/secret-detection.sh b/scripts/ci/secret-detection.sh index 4eb0ac6..311bdd8 100755 --- a/scripts/ci/secret-detection.sh +++ b/scripts/ci/secret-detection.sh @@ -42,88 +42,127 @@ if [ ! -f "pyproject.toml" ]; then print_error "Must run from project root (where pyproject.toml exists)" fi +# detect-secrets entropy heuristics can miss short Graphistry personal keys. +# This deterministic check reports only key names and locations, never values. +# In --check-only (pre-commit) mode scan the staged index, so a credential +# cannot be committed even when the working tree has already been cleaned. +CREDENTIAL_SCAN_ARGS=() +if [ "$CHECK_ONLY" == true ]; then + CREDENTIAL_SCAN_ARGS+=(--staged) +fi +python3 scripts/ci/check_credential_literals.py "${CREDENTIAL_SCAN_ARGS[@]}" || { + print_error "Hard-coded Graphistry personal key detected" +} + # Check if detect-secrets is available if ! command -v detect-secrets &> /dev/null; then - # Try with uv run - if ! uv run detect-secrets --version &> /dev/null 2>&1; then + # Try with uv run. `--frozen` is required: a bare `uv run` re-resolves and + # rewrites uv.lock, silently dropping its `[options] exclude-newer` pin. + # That is the source of the stray uv.lock diff that keeps reappearing after + # running the security or lint scripts. + if ! uv run --frozen detect-secrets --version &> /dev/null 2>&1; then print_error "detect-secrets not found. Install with: uv pip install detect-secrets" fi - DETECT_SECRETS="uv run detect-secrets" + # --project is required: the pre-commit path scans a materialised copy of + # the index from a temp directory, and a bare `uv run` there cannot find the + # project and fails with "Failed to spawn: detect-secrets". + DETECT_SECRETS="uv run --frozen --project $PWD detect-secrets" # pragma: allowlist secret else DETECT_SECRETS="detect-secrets" fi -# Ensure baseline exists +# Ensure baseline exists. +# +# This must NOT exit 0. Generating a baseline accepts whatever is currently in +# the tree, so `rm .secrets.baseline && ./secret-detection.sh` used to pass while +# permanently whitelisting any secret present — the same "always exit 0" shape +# this script exists to eliminate. Generate, then fail so a human reviews it. if [ ! -f ".secrets.baseline" ]; then print_warning "No .secrets.baseline found. Creating initial baseline..." - $DETECT_SECRETS scan --exclude-files '^(plans/|tmp/)' > .secrets.baseline - print_success "Created .secrets.baseline - please review and commit" - exit 0 + $DETECT_SECRETS scan --exclude-files '^(plans/|tmp/|\.secrets\.baseline$)' > .secrets.baseline + print_error "Created .secrets.baseline from the current tree. Review it (it accepts everything found), commit it, then re-run." fi if [ "$CHECK_ONLY" == true ]; then # Pre-commit mode: just check for new secrets echo "🔍 Checking for secrets in staged files..." - # Get list of staged files (excluding plans/, tmp/, and the baseline itself) - STAGED_FILES=$(git diff --cached --name-only --diff-filter=ACM \ - | grep -v '^plans/' \ - | grep -v '^tmp/' \ - | grep -v '^\.secrets\.baseline$' \ - || true) - - if [ -z "$STAGED_FILES" ]; then - print_success "No files to check" - exit 0 - fi - - # Check staged files for secrets using a temp baseline to avoid mutating the real one - TEMP_BASELINE=$(mktemp) + # NUL-delimited end to end. The previous newline+`xargs` pipeline word-split + # on spaces, and detect-secrets exits 0 with empty results for a path that + # does not exist — so a staged file named `zz spaced.py` scanned nothing and + # the gate reported success. + # + # --no-renames: with rename detection on, `git mv a.py b.py` plus an edit + # reports only `R`, which --diff-filter=ACM drops — a working bypass that + # let a secret reach a commit with a green hook. + TEMP_LIST=$(mktemp) TEMP_SCAN=$(mktemp) - cp .secrets.baseline "$TEMP_BASELINE" - echo "$STAGED_FILES" | xargs $DETECT_SECRETS scan --baseline "$TEMP_BASELINE" > "$TEMP_SCAN" 2>/dev/null || true - - # Check if any new secrets were detected - if [ -s "$TEMP_SCAN" ]; then - NEW_SECRETS=$(python3 -c " -import json + trap 'rm -f "$TEMP_LIST" "$TEMP_SCAN"' EXIT + + git diff --cached --no-renames --name-only --diff-filter=ACM -z \ + | python3 -c ' import sys -with open('$TEMP_SCAN') as f: - data = json.load(f) - total = sum(len(secrets) for secrets in data.get('results', {}).values()) - sys.exit(0 if total == 0 else 1) -" 2>/dev/null || echo "1") - rm -f "$TEMP_SCAN" "$TEMP_BASELINE" - - if [ "$NEW_SECRETS" == "1" ]; then - print_error "New secrets detected! Use clear placeholders like 'sk-XXXXXXXX' or ''" - fi +skip = ("plans/", "tmp/") +data = sys.stdin.buffer.read().split(b"\0") +keep = [ + p for p in data + if p and p != b".secrets.baseline" + and not any(p.startswith(s.encode()) for s in skip) +] +# NUL-TERMINATE, do not NUL-separate. `read -r -d ""` only emits a field when +# it sees the delimiter, so joining instead of terminating silently drops the +# last staged file — which meant it was never scanned. +sys.stdout.buffer.write(b"".join(p + b"\0" for p in keep)) +' > "$TEMP_LIST" + + if [ ! -s "$TEMP_LIST" ]; then + print_success "No files to check" + exit 0 fi - - rm -f "$TEMP_SCAN" "$TEMP_BASELINE" + + # Materialise the INDEX, not the working tree. detect-secrets scans files on + # disk, so `git add ` followed by cleaning or deleting the file made + # the hook pass while the commit still carried the secret — the same evasion + # `--staged` closes for the personal-key rule, but it applied to every + # detect-secrets finding class. + STAGE_DIR=$(mktemp -d) + trap 'rm -rf "$TEMP_LIST" "$TEMP_SCAN" "$STAGE_DIR"' EXIT + while IFS= read -r -d '' staged_path; do + mkdir -p "$STAGE_DIR/$(dirname "$staged_path")" + git show ":$staged_path" > "$STAGE_DIR/$staged_path" 2>/dev/null || true + done < "$TEMP_LIST" + + # Scan WITHOUT --baseline. `scan --baseline ` updates the file in place, + # writes nothing to stdout, and exits 0 whatever it finds — so the previous + # `if [ -s "$TEMP_SCAN" ]` guard tested an always-empty file and skipped the + # check entirely. Compare against the baseline explicitly instead. + # stderr is deliberately NOT suppressed: hiding it is what made the + # path-mangling failure above invisible. + ( cd "$STAGE_DIR" && $DETECT_SECRETS scan --all-files ) > "$TEMP_SCAN" + + python3 scripts/ci/check_new_secrets.py \ + --baseline .secrets.baseline --scan "$TEMP_SCAN" || { + print_error "New secrets detected! Use clear placeholders like 'sk-XXXXXXXX' or ''" + } + print_success "No secrets detected" else # CI mode: full scan echo "🔍 Running full secret detection scan..." - # Use a temp baseline to avoid detect-secrets rewriting generated_at. - TEMP_BASELINE=$(mktemp) - cp .secrets.baseline "$TEMP_BASELINE" - cleanup_baseline() { - rm -f "$TEMP_BASELINE" - } - trap cleanup_baseline EXIT + # Scan WITHOUT --baseline, then diff against it. `scan --baseline ` is an + # update command: it rewrites the file and exits 0 regardless of findings, so + # both `|| print_error` branches below were unreachable and this gate had + # never once failed — a known-live credential passed it for ~12 months. + TEMP_SCAN=$(mktemp) + trap 'rm -f "$TEMP_SCAN"' EXIT - # Check for new secrets not in baseline echo "Checking for new secrets not in baseline..." - $DETECT_SECRETS scan --baseline "$TEMP_BASELINE" --exclude-files '^(plans/|tmp/)' || { - print_error "New secrets detected! Either remove them or update baseline with: detect-secrets scan --baseline .secrets.baseline" - } + $DETECT_SECRETS scan --exclude-files '^(plans/|tmp/|\.secrets\.baseline$)' > "$TEMP_SCAN" - # Verify no high-confidence secrets - echo "Verifying no high-confidence secrets..." - $DETECT_SECRETS scan --baseline "$TEMP_BASELINE" --only-verified --exclude-files '^(plans/|tmp/)' || { - print_error "High-confidence secrets detected! These must be removed." + python3 scripts/ci/check_new_secrets.py \ + --baseline .secrets.baseline --scan "$TEMP_SCAN" || { + print_error "New secrets detected! Either remove them, or re-baseline with: detect-secrets scan > .secrets.baseline (and review the diff)" } print_success "Secret detection passed - no new secrets found" diff --git a/scripts/test-secret-detection.sh b/scripts/test-secret-detection.sh index 94ac16e..2bff47b 100755 --- a/scripts/test-secret-detection.sh +++ b/scripts/test-secret-detection.sh @@ -3,7 +3,7 @@ # This script creates temporary test files to verify secret detection works correctly # Usage: ./scripts/test-secret-detection.sh -set -e +set -euo pipefail # Colors for output RED='\033[0;31m' @@ -12,137 +12,164 @@ YELLOW='\033[1;33m' BLUE='\033[0;34m' NC='\033[0m' # No Color +REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" +cd "$REPO_ROOT" + echo -e "${BLUE}🔬 Testing Secret Detection System${NC}" echo "========================================" # Create a temporary directory for testing TEST_DIR=$(mktemp -d -t secret-test-XXXXXX) -trap "rm -rf $TEST_DIR" EXIT +trap 'rm -rf "$TEST_DIR"' EXIT echo -e "${YELLOW}📁 Test directory: $TEST_DIR${NC}" echo "" -# Function to run a test +PASSED=0 +FAILED=0 + +# NOTE: `((PASSED++))` returns a non-zero status when the variable is 0, which +# under `set -e` aborted this script and made the first passing test increment +# FAILED via the `||` branch. Always use the assignment form below. +pass() { + PASSED=$((PASSED + 1)) + echo -e "${GREEN} ✅ PASS: $1${NC}" +} + +fail() { + FAILED=$((FAILED + 1)) + echo -e "${RED} ❌ FAIL: $1${NC}" +} + +# Run detect-secrets over a single file. +# +# NOTE: detect-secrets only reports files located under the current working +# directory. The previous version scanned an absolute path in /tmp while cd'ed +# to the repo root, so every scan came back empty and all five "unsafe" +# fixtures silently looked undetected. Scan from inside TEST_DIR instead. +# NOTE: `--frozen` is required. A bare `uv run` re-resolves and rewrites +# uv.lock, silently dropping its `[options] exclude-newer` pin — that is the +# source of the "incidental uv.lock delta" that keeps reappearing in this repo. +detect_secrets_finds() { + local relative_file="$1" + ( + cd "$TEST_DIR" + uv run --frozen --project "$REPO_ROOT" detect-secrets scan "$relative_file" \ + 2>/dev/null + ) | python3 -c 'import json,sys; sys.exit(0 if json.load(sys.stdin).get("results") else 1)' +} + +# Run a detect-secrets expectation test. run_test() { local test_name="$1" - local file_path="$2" - local content="$3" - local should_detect="$4" # "yes" or "no" - + local content="$2" + local should_detect="$3" # "yes" or "no" + echo -e "${BLUE}Test: $test_name${NC}" - echo "$content" > "$file_path" - - # Stage the file for pre-commit test - cd "$(dirname "$0")/.." - cp "$file_path" "$TEST_DIR/test_file.py" - - # Run detection - if uv run detect-secrets scan "$TEST_DIR/test_file.py" 2>/dev/null | grep -q "\"$TEST_DIR/test_file.py\""; then + printf '%s\n' "$content" > "$TEST_DIR/test_file.py" + + local detected="no" + if detect_secrets_finds "test_file.py"; then detected="yes" - else - detected="no" fi - + if [ "$detected" = "$should_detect" ]; then - echo -e "${GREEN} ✅ PASS: Detection result as expected ($detected)${NC}" - return 0 + pass "Detection result as expected ($detected)" else - echo -e "${RED} ❌ FAIL: Expected detection=$should_detect, got=$detected${NC}" - echo " Content: $content" - return 1 + fail "Expected detection=$should_detect, got=$detected" fi } -# Test counter -PASSED=0 -FAILED=0 +# Run a check_credential_literals.py expectation test. +run_credential_test() { + local test_name="$1" + local content="$2" + local should_reject="$3" # "yes" or "no" -echo -e "${YELLOW}🚨 Testing UNSAFE patterns (should be detected)${NC}" -echo "----------------------------------------" + echo -e "${BLUE}Test: $test_name${NC}" + printf '%s\n' "$content" > "$TEST_DIR/cred_file.py" -# Real-looking secrets that SHOULD be detected -run_test "Generic API Key" \ - "$TEST_DIR/api.py" \ - 'api_key = "super_secret_api_key_12345"' \ - "yes" && ((PASSED++)) || ((FAILED++)) + local rejected="no" + local output + if ! output=$(python3 scripts/ci/check_credential_literals.py "$TEST_DIR/cred_file.py" 2>&1); then + rejected="yes" + fi -run_test "Generic Password" \ - "$TEST_DIR/password.py" \ - 'password = "mysecretpassword123"' \ - "yes" && ((PASSED++)) || ((FAILED++)) + if [ "$rejected" != "$should_reject" ]; then + fail "Expected reject=$should_reject, got=$rejected" + return + fi -run_test "API Token" \ - "$TEST_DIR/token.py" \ - 'api_token = "token_abc123def456ghi789"' \ - "yes" && ((PASSED++)) || ((FAILED++)) + # The checker must never echo the value it rejected. + if [ "$rejected" = "yes" ]; then + local value + value=$(printf '%s' "$content" | sed -n 's/.*"\([^"]*\)".*/\1/p') + if [ -n "$value" ] && printf '%s' "$output" | grep -qF -- "$value"; then + fail "Checker echoed the rejected value" + return + fi + fi + pass "Credential gate behaved as expected (reject=$rejected)" +} -run_test "Private Key" \ - "$TEST_DIR/key.py" \ - 'private_key = "private_key_secret_value_123"' \ - "yes" && ((PASSED++)) || ((FAILED++)) +echo -e "${YELLOW}🚨 Testing UNSAFE patterns (should be detected)${NC}" +echo "----------------------------------------" -run_test "Base64 Secret" \ - "$TEST_DIR/b64.py" \ - 'secret = "cGFzc3dvcmQ9bXlfc2VjcmV0X3Bhc3N3b3Jk"' \ - "yes" && ((PASSED++)) || ((FAILED++)) +run_test "Generic API Key" 'api_key = "super_secret_api_key_12345"' "yes" +run_test "Generic Password" 'password = "mysecretpassword123"' "yes" +run_test "API Token" 'api_token = "token_abc123def456ghi789"' "yes" +run_test "Private Key" 'private_key = "private_key_secret_value_123"' "yes" +run_test "Base64 Secret" 'secret = "cGFzc3dvcmQ9bXlfc2VjcmV0X3Bhc3N3b3Jk"' "yes" echo "" echo -e "${YELLOW}✅ Testing SAFE patterns (should NOT be detected)${NC}" echo "----------------------------------------" -# Safe placeholders that should NOT be detected -run_test "XXXX Placeholder" \ - "$TEST_DIR/safe1.py" \ - 'API_KEY = "sk-XXXXXXXXXXXXXXXX"' \ - "no" && ((PASSED++)) || ((FAILED++)) - -run_test "Angle Bracket Placeholder" \ - "$TEST_DIR/safe2.py" \ - 'password = ""' \ - "no" && ((PASSED++)) || ((FAILED++)) - -run_test "Token with XXXX" \ - "$TEST_DIR/safe3.py" \ - 'token = "token-XXXX-XXXX-XXXX"' \ - "no" && ((PASSED++)) || ((FAILED++)) - -run_test "Stars Placeholder" \ - "$TEST_DIR/safe4.py" \ - 'SECRET = "****"' \ - "no" && ((PASSED++)) || ((FAILED++)) - -run_test "Example Placeholder" \ - "$TEST_DIR/safe5.py" \ - 'key = "your-api-key-here"' \ - "no" && ((PASSED++)) || ((FAILED++)) - -run_test "Dots Placeholder" \ - "$TEST_DIR/safe6.py" \ - 'token = "..."' \ - "no" && ((PASSED++)) || ((FAILED++)) +# `.secret-patterns.md` lists `sk-XXXXXXXXXXXXXXXX` as a safe placeholder, but a +# raw detect-secrets scan reports it as `Secret Keyword` when it sits in a +# keyword-adjacent assignment — its placeholder filters recognise +# `token-XXXX-XXXX-XXXX` and `your-api-key-here` but not this form. Documented +# under "API Keys" in .secret-patterns.md; asserted here as the real behaviour. +run_test "XXXX Placeholder (keyword-adjacent)" 'API_KEY = "sk-XXXXXXXXXXXXXXXX"' "yes" +run_test "Angle Bracket Placeholder" 'password = ""' "no" +run_test "Token with XXXX" 'token = "token-XXXX-XXXX-XXXX"' "no" +run_test "Stars Placeholder" 'SECRET = "****"' "no" +run_test "Example Placeholder" 'key = "your-api-key-here"' "no" +run_test "Dots Placeholder" 'token = "..."' "no" + +echo "" +echo -e "${YELLOW}🔑 Testing Graphistry personal-key gate${NC}" +echo "----------------------------------------" + +# Assembled at runtime so this script contains no credential-shaped literal. +FAKE_ID="A1B2C3""D4E5" +FAKE_SECRET="A1B2C3D4""E5F6G7H8" +KEY_ID="personal_key_""id" +KEY_SECRET="personal_key_""secret" + +run_credential_test "Key id literal" "$KEY_ID = \"$FAKE_ID\"" "yes" +run_credential_test "Key secret literal" "$KEY_SECRET = \"$FAKE_SECRET\"" "yes" +run_credential_test "Shape in unrelated variable" "blob = \"$FAKE_SECRET\"" "yes" +run_credential_test "Env default" "os.getenv(\"PERSONAL_KEY_SECRET\", \"$FAKE_SECRET\")" "yes" +run_credential_test "Angle placeholder" "$KEY_ID = \"\"" "no" +run_credential_test "Mock value" "$KEY_ID = \"pk_123\"" "no" echo "" echo -e "${YELLOW}🧪 Testing with actual scripts${NC}" echo "----------------------------------------" -# Test our actual detection script echo -e "${BLUE}Testing centralized script:${NC}" if ./scripts/ci/secret-detection.sh > /dev/null 2>&1; then - echo -e "${GREEN} ✅ Secret detection script runs successfully${NC}" - ((PASSED++)) + pass "Secret detection script runs successfully" else - echo -e "${RED} ❌ Secret detection script failed${NC}" - ((FAILED++)) + fail "Secret detection script failed" fi -# Test the pre-commit wrapper echo -e "${BLUE}Testing pre-commit wrapper:${NC}" if ./scripts/pre-commit-secret-check.sh > /dev/null 2>&1; then - echo -e "${GREEN} ✅ Pre-commit wrapper runs successfully${NC}" - ((PASSED++)) + pass "Pre-commit wrapper runs successfully" else - echo -e "${RED} ❌ Pre-commit wrapper failed${NC}" - ((FAILED++)) + fail "Pre-commit wrapper failed" fi # Summary @@ -153,7 +180,7 @@ echo "----------------------------------------" echo -e " Passed: ${GREEN}$PASSED${NC}" echo -e " Failed: ${RED}$FAILED${NC}" -if [ $FAILED -eq 0 ]; then +if [ "$FAILED" -eq 0 ]; then echo "" echo -e "${GREEN}🎉 All tests passed!${NC}" exit 0 @@ -161,4 +188,4 @@ else echo "" echo -e "${RED}⚠️ Some tests failed. Review the output above.${NC}" exit 1 -fi \ No newline at end of file +fi diff --git a/scripts/test.sh b/scripts/test.sh index 808bec8..7a200df 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -103,15 +103,18 @@ case "$TEST_MODE" in ;; esac -# Load environment variables from .env if it exists -if [[ -f .env ]]; then +# Export test mode first: the .env load below is gated on it. +export LOUIE_TEST_MODE="$TEST_MODE" + +# Load environment variables from .env — only for modes that actually talk to a +# server. Loading it unconditionally re-injected credentials into every unit run, +# which defeated `tests/utils.py`'s opt-in gate and let a "no credentials" run +# authenticate against the real service anyway. +if [[ -f .env && ( "$TEST_MODE" == "integration" || "$TEST_MODE" == "all" ) ]]; then echo "📋 Loading environment from .env" export $(grep -v '^#' .env | xargs) fi -# Export test mode -export LOUIE_TEST_MODE="$TEST_MODE" - # Run tests echo "Running: python -m pytest ${PYTEST_ARGS[*]}" python -m pytest "${PYTEST_ARGS[@]}" diff --git a/src/louieai/_client.py b/src/louieai/_client.py index 99d8179..519dce5 100644 --- a/src/louieai/_client.py +++ b/src/louieai/_client.py @@ -181,8 +181,8 @@ def __init__( # Use personal key authentication (recommended for service accounts) client = LouieClient( - personal_key_id="ZD5872XKNF", - personal_key_secret="SA0JJ2DTVT6LLO2S", + personal_key_id="", + personal_key_secret="", graphistry_server="hub.graphistry.com" ) diff --git a/tests/conftest.py b/tests/conftest.py index 096c835..0254723 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,6 +7,8 @@ import pytest +from tests.utils import INTEGRATION_MODES, get_test_mode + # Python version check MIN_PYTHON_VERSION = (3, 10) CURRENT_PYTHON_VERSION = sys.version_info[:2] @@ -53,16 +55,14 @@ def pytest_configure(config): config.addinivalue_line("markers", "slow: mark test as slow running") -# Test mode detection -def get_test_mode() -> str: - """Get the current test mode from environment.""" - return os.environ.get("LOUIE_TEST_MODE", "unit").lower() +# Test mode detection — single source of truth lives in tests/utils.py so this +# and the .env gate cannot drift (they previously disagreed about `all`). def should_run_integration_tests() -> bool: """Check if integration tests should run.""" # Check explicit test mode - if get_test_mode() == "integration": + if get_test_mode() in INTEGRATION_MODES: return True # Check if credentials are available @@ -102,23 +102,43 @@ def test_credentials(): def real_client(test_credentials): """Create a real LouieClient for integration tests.""" if not test_credentials: - pytest.skip("No test credentials available") + pytest.skip( + "No test credentials available " + "(set LOUIE_TEST_MODE=integration to read .env)" + ) + + from urllib.parse import urlsplit + + louie_server = ( + os.getenv("LOUIE_SERVER") + or os.getenv("LOUIE_SERVER_URL") + or os.getenv("LOUIE_URL") + ) + if not louie_server: + pytest.skip("LOUIE_SERVER is required for credentialed integration tests") + if "://" not in louie_server: + louie_server = f"https://{louie_server}" + + parsed_server = urlsplit(louie_server) + is_local = parsed_server.hostname in {"localhost", "127.0.0.1", "::1"} + if not parsed_server.hostname or (parsed_server.scheme != "https" and not is_local): + raise ValueError( + "LOUIE_SERVER must use HTTPS except for an explicit localhost endpoint" + ) import graphistry from louieai._client import LouieClient - # Register with Graphistry - graphistry.register( + # Authenticate only after the target Louie endpoint has passed validation. + graphistry_client = graphistry.register( api=test_credentials.get("api_version", 3), server=test_credentials["server"], username=test_credentials["username"], password=test_credentials["password"], ) - - # Create Louie client - louie_server = test_credentials.get("louie_server", "https://louie-dev.grph.xyz") - return LouieClient(server_url=louie_server) + # Never copy credentials or tokens into test output. + return LouieClient(server_url=louie_server, graphistry_client=graphistry_client) # Test data fixtures diff --git a/tests/integration/notebook/test_streaming_real.py b/tests/integration/notebook/test_streaming_real.py index 2413a6e..582f3f6 100644 --- a/tests/integration/notebook/test_streaming_real.py +++ b/tests/integration/notebook/test_streaming_real.py @@ -34,7 +34,7 @@ def lui(self): # Create louie interface with real auth return louie( - graphistry_client=graphistry_client, server_url="https://louie-dev.grph.xyz" + graphistry_client=graphistry_client, server_url="https://louie.example.com" ) def test_streaming_provides_faster_first_response(self, lui): diff --git a/tests/integration/test_arrow_dataframe_integration.py b/tests/integration/test_arrow_dataframe_integration.py index f3d6fa2..328d2aa 100644 --- a/tests/integration/test_arrow_dataframe_integration.py +++ b/tests/integration/test_arrow_dataframe_integration.py @@ -278,7 +278,7 @@ def real_client(self): from louieai._client import LouieClient return LouieClient( - server_url="https://louie-dev.grph.xyz", graphistry_client=graphistry_client + server_url="https://louie.example.com", graphistry_client=graphistry_client ) def test_real_arrow_dataframe_fetch(self, real_client): diff --git a/tests/integration/test_documentation_integration.py b/tests/integration/test_documentation_integration.py index a76471f..5b9abee 100644 --- a/tests/integration/test_documentation_integration.py +++ b/tests/integration/test_documentation_integration.py @@ -34,9 +34,7 @@ def real_client(self, test_credentials): ) # Create Louie client - louie_server = test_credentials.get( - "louie_server", "https://louie-dev.grph.xyz" - ) + louie_server = test_credentials.get("louie_server", "https://louie.example.com") return LouieClient(server_url=louie_server) def _should_test_code(self, code: str) -> bool: diff --git a/tests/integration/test_real_louie.py b/tests/integration/test_real_louie.py index cd8b8f5..dfec265 100644 --- a/tests/integration/test_real_louie.py +++ b/tests/integration/test_real_louie.py @@ -32,9 +32,8 @@ def client(self): ) # Create Louie client with graphistry client - # Use louie-dev.grph.xyz as mentioned in credentials return LouieClient( - server_url="https://louie-dev.grph.xyz", graphistry_client=graphistry_client + server_url="https://louie.example.com", graphistry_client=graphistry_client ) def test_basic_query(self, client): diff --git a/tests/test_doc_examples.py b/tests/test_doc_examples.py index 19d03f6..256fdbb 100644 --- a/tests/test_doc_examples.py +++ b/tests/test_doc_examples.py @@ -269,7 +269,7 @@ def test_basic_example_integration(self): ) client = LouieClient( - server_url="https://louie-dev.grph.xyz", graphistry_client=graphistry_client + server_url="https://louie.example.com", graphistry_client=graphistry_client ) # Test thread creation diff --git a/tests/unit/notebook/test_thread_properties.py b/tests/unit/notebook/test_thread_properties.py index efc866e..672ae44 100644 --- a/tests/unit/notebook/test_thread_properties.py +++ b/tests/unit/notebook/test_thread_properties.py @@ -41,14 +41,14 @@ def test_thread_id_after_query(self): ("https://den.louie.ai/", "abc123", "https://den.louie.ai/?dthread=abc123"), # Dev server ( - "https://louie-dev.grph.xyz", + "https://louie.example.com", "xyz789", - "https://louie-dev.grph.xyz/?dthread=xyz789", + "https://louie.example.com/?dthread=xyz789", ), ( - "https://louie-dev.grph.xyz/", + "https://louie.example.com/", "xyz789", - "https://louie-dev.grph.xyz/?dthread=xyz789", + "https://louie.example.com/?dthread=xyz789", ), # Custom/enterprise servers ( diff --git a/tests/unit/security/test_credential_gating.py b/tests/unit/security/test_credential_gating.py new file mode 100644 index 0000000..92ecc74 --- /dev/null +++ b/tests/unit/security/test_credential_gating.py @@ -0,0 +1,144 @@ +"""Integration-credential gating must fail closed. + +`load_test_credentials()` used to call `load_dotenv()` unconditionally, so an +operator who cleared every credential variable still got credentials back from +`.env` — and the credentialed fixtures then dialled the real server. These tests +lock the opt-in behaviour in place. +""" + +from __future__ import annotations + +import os +from pathlib import Path + +import pytest + +from tests.utils import dotenv_enabled, load_test_credentials, repo_root + +pytestmark = pytest.mark.unit + +CREDENTIAL_VARS = ( + "GRAPHISTRY_SERVER", + "GRAPHISTRY_USERNAME", + "GRAPHISTRY_PASSWORD", + "GRAPHISTRY_API_VERSION", + "LOUIE_SERVER", + "LOUIE_SERVER_URL", + "LOUIE_URL", +) + + +@pytest.fixture +def clean_env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> pytest.MonkeyPatch: + """No credentials in the environment, and `.env` stubbed to a known payload. + + `load_dotenv()` resolves `.env` from the calling module's directory, not the + process cwd, so a real file in `tmp_path` would be ignored and the repo's own + `.env` would win. Stub the loader instead: that tests the contract (is `.env` + consulted?) without depending on whether a developer has one. + """ + for name in CREDENTIAL_VARS: + monkeypatch.delenv(name, raising=False) + monkeypatch.delenv("LOUIE_TEST_MODE", raising=False) + monkeypatch.chdir(tmp_path) + + # Must go through monkeypatch.setenv, not os.environ directly: a direct + # write is not undone at teardown and leaks GRAPHISTRY_SERVER into every + # later test in the session, where `Cursor()` then rejects it. + def fake_load_dotenv(*args: object, **kwargs: object) -> bool: + monkeypatch.setenv("GRAPHISTRY_SERVER", "dotenv.example.com") + monkeypatch.setenv("GRAPHISTRY_USERNAME", "dotenv-user") + monkeypatch.setenv("GRAPHISTRY_PASSWORD", "dotenv-pass") + monkeypatch.setenv("LOUIE_SERVER", "https://dotenv.example.com") + return True + + monkeypatch.setattr("tests.utils.load_dotenv", fake_load_dotenv) + return monkeypatch + + +def test_dotenv_is_opt_in(clean_env: pytest.MonkeyPatch) -> None: + assert dotenv_enabled() is False + clean_env.setenv("LOUIE_TEST_MODE", "integration") + assert dotenv_enabled() is True + + +@pytest.mark.parametrize("mode", ["integration", "all", "INTEGRATION", "All"]) +def test_integration_modes_enable_dotenv( + clean_env: pytest.MonkeyPatch, mode: str +) -> None: + """`all` must opt in too — it runs the integration tests.""" + clean_env.setenv("LOUIE_TEST_MODE", mode) + assert dotenv_enabled() is True + + +@pytest.mark.parametrize("mode", ["unit", "", "smoke"]) +def test_non_integration_modes_keep_dotenv_off( + clean_env: pytest.MonkeyPatch, mode: str +) -> None: + clean_env.setenv("LOUIE_TEST_MODE", mode) + assert dotenv_enabled() is False + assert load_test_credentials() is None + + +def test_conftest_and_utils_share_one_mode_source( + clean_env: pytest.MonkeyPatch, +) -> None: + """They previously disagreed about `all`; they must not drift again.""" + from tests.conftest import get_test_mode as conftest_mode + from tests.conftest import should_run_integration_tests + from tests.utils import get_test_mode as utils_mode + + assert conftest_mode is utils_mode + clean_env.setenv("LOUIE_TEST_MODE", "all") + assert should_run_integration_tests() is True + assert dotenv_enabled() is True + + +def test_test_sh_only_loads_dotenv_for_integration_modes() -> None: + """`scripts/test.sh` used to export .env in every mode, defeating this gate.""" + script = (repo_root() / "scripts" / "test.sh").read_text(encoding="utf-8") + load_line = next(line for line in script.splitlines() if "-f .env" in line) + + assert "integration" in load_line and "all" in load_line, load_line + # And the mode must be exported before the gated load reads it. + assert script.index('export LOUIE_TEST_MODE="$TEST_MODE"') < script.index(load_line) + + +def test_cleared_environment_yields_no_credentials( + clean_env: pytest.MonkeyPatch, +) -> None: + """The regression: .env must not resurrect cleared credentials.""" + assert load_test_credentials() is None + # And the environment must stay clean, so downstream gating also skips. + assert not os.getenv("GRAPHISTRY_SERVER") + assert not os.getenv("LOUIE_SERVER") + + +def test_integration_mode_reads_dotenv(clean_env: pytest.MonkeyPatch) -> None: + """Explicit opt-in still works for local integration runs.""" + clean_env.setenv("LOUIE_TEST_MODE", "integration") + + credentials = load_test_credentials() + + assert credentials is not None + assert credentials["server"] == "dotenv.example.com" + + +def test_real_environment_wins_without_dotenv(clean_env: pytest.MonkeyPatch) -> None: + """CI path: real env vars, no .env consulted.""" + clean_env.setenv("GRAPHISTRY_SERVER", "ci.example.com") + clean_env.setenv("GRAPHISTRY_USERNAME", "ci-user") + clean_env.setenv("GRAPHISTRY_PASSWORD", "ci-pass") + + credentials = load_test_credentials() + + assert credentials is not None + assert credentials["server"] == "ci.example.com" + + +def test_partial_credentials_are_rejected(clean_env: pytest.MonkeyPatch) -> None: + clean_env.setenv("GRAPHISTRY_SERVER", "ci.example.com") + clean_env.setenv("GRAPHISTRY_USERNAME", "ci-user") + # password intentionally absent + + assert load_test_credentials() is None diff --git a/tests/unit/security/test_credential_literals.py b/tests/unit/security/test_credential_literals.py new file mode 100644 index 0000000..814ddf6 --- /dev/null +++ b/tests/unit/security/test_credential_literals.py @@ -0,0 +1,459 @@ +"""Regression tests for the deterministic Graphistry personal-key gate. + +Fixtures are assembled at runtime from fragments so this file never itself +contains a credential-shaped literal that the checker would flag. Every test +asserts the checker's output does not echo the value it rejected. +""" + +from __future__ import annotations + +import importlib.util +import json +import subprocess +import sys +from pathlib import Path + +import pytest + +from tests.utils import repo_root + +pytestmark = pytest.mark.unit + +REPO_ROOT = repo_root() +CHECKER = REPO_ROOT / "scripts" / "ci" / "check_credential_literals.py" + + +def _load_checker(): + """Import the stdlib-only checker module for white-box assertions.""" + spec = importlib.util.spec_from_file_location("_cred_checker", CHECKER) + assert spec and spec.loader + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +_is_placeholder = _load_checker()._is_placeholder + +# Built at runtime: a 10-char id shape and a 16-char secret shape. Both mix +# letters and digits, as the real key formats do. +FAKE_ID = "A1B2C3" + "D4E5" # pragma: allowlist secret +FAKE_SECRET = "A1B2C3D4" + "E5F6G7H8" # pragma: allowlist secret +KEY_ID = "personal_key_" + "id" # pragma: allowlist secret +KEY_SECRET = "personal_key_" + "secret" # pragma: allowlist secret + +# Entropy-boundary fixtures, assembled so no scannable literal exists here. +TOO_SHORT = "AB" + "1234567" +LOW_DISTINCT = "ab" * 7 +ABOVE_FLOOR = "abcdef" + "123456" + + +# Explicit parametrize ids. Without these, pytest -v composes the id from the +# parameter *values*, printing credential-shaped fixtures into public CI logs — +# harmless for these fakes, but the wrong habit for a security suite. +CONTEXT_IDS = [ + "bare", + "annotated", + "dict", + "kwarg", + "getenv", + "unrelated", + "single_quoted", + "dotenv", + "shell_export", + "yaml", + "markdown", + "md_fence", +] +OFFSHAPE_IDS = ["bare", "prefixed", "getenv", "dict", "yaml", "annotated"] +PLACEHOLDER_IDS = [ + "angle_id", + "angle_secret", + "env_var", + "pk_mock", + "test_mock", + "filler", + "empty", + "fake_substring", + "example_substring", +] + + +def run_checker( + *args: str, cwd: Path | None = None +) -> subprocess.CompletedProcess[str]: + return subprocess.run( + [sys.executable, str(CHECKER), *args], + capture_output=True, + check=False, + text=True, + cwd=cwd, + ) + + +def assert_rejected(result: subprocess.CompletedProcess[str], *secrets: str) -> None: + """Checker failed, said something useful, and leaked nothing.""" + assert result.returncode == 1, result.stderr + assert "hard-coded" in result.stderr + for secret in secrets: + assert secret not in result.stdout + assert secret not in result.stderr + + +def assert_accepted(result: subprocess.CompletedProcess[str]) -> None: + assert result.returncode == 0, result.stderr + assert result.stderr == "" + + +def write(tmp_path: Path, name: str, body: str) -> Path: + candidate = tmp_path / name + candidate.write_text(body, encoding="utf-8") + return candidate + + +# --- value-shape: context-free, so quoting and file format must not matter --- + + +@pytest.mark.parametrize( + ("name", "body"), + [ + ("bare.py", f'{KEY_SECRET} = "{FAKE_SECRET}"'), + ("annotated.py", f'{KEY_SECRET}: str = "{FAKE_SECRET}"'), + ("dict.py", f'creds = {{"{KEY_SECRET}": "{FAKE_SECRET}"}}'), + ("kwarg.py", f'register({KEY_SECRET}="{FAKE_SECRET}")'), + ("getenv.py", f'os.getenv("PERSONAL_KEY_SECRET", "{FAKE_SECRET}")'), + ("unrelated.py", f'blob = "{FAKE_SECRET}"'), + ("single_quoted.py", f"{KEY_SECRET} = '{FAKE_SECRET}'"), + # Unquoted forms — the .env / shell / YAML paths DEVELOP.md documents. + (".env", f"GRAPHISTRY_PERSONAL_KEY_SECRET={FAKE_SECRET}"), + ("export.sh", f"export GRAPHISTRY_PERSONAL_KEY_SECRET={FAKE_SECRET}"), + ("conf.yml", f"{KEY_SECRET}: {FAKE_SECRET}"), + ("notes.md", f"Use {FAKE_SECRET} as the secret."), + ("fence.md", f'```python\n{KEY_SECRET} = "{FAKE_SECRET}"\n```'), + ], + ids=CONTEXT_IDS, +) +def test_rejects_credential_shape_in_any_context( + tmp_path: Path, name: str, body: str +) -> None: + """The old key-name regex missed most of these; the shape rule must not.""" + assert_rejected(run_checker(str(write(tmp_path, name, body))), FAKE_SECRET) + + +def test_rejects_shape_inside_notebook_json(tmp_path: Path) -> None: + """.ipynb stores source with escaped quotes, so quote-anchored rules miss it. + + The repo's own tutorial notebooks already contain `personal_key_secret = ...` + cells, which is exactly where an accidental paste would land. + """ + notebook = { + "cells": [ + { + "cell_type": "code", + "metadata": {}, + "source": [f'{KEY_SECRET} = "{FAKE_SECRET}"\n'], + "outputs": [], + "execution_count": None, + } + ], + "metadata": {}, + "nbformat": 4, + "nbformat_minor": 5, + } + candidate = write(tmp_path, "nb.ipynb", json.dumps(notebook, indent=1)) + + assert_rejected(run_checker(str(candidate)), FAKE_SECRET) + + +def test_rejects_both_id_and_secret_shapes(tmp_path: Path) -> None: + candidate = write( + tmp_path, "pair.py", f'{KEY_ID} = "{FAKE_ID}"\n{KEY_SECRET} = "{FAKE_SECRET}"\n' + ) + + result = run_checker(str(candidate)) + + assert_rejected(result, FAKE_ID, FAKE_SECRET) + assert result.stderr.count("hard-coded") == 2 + + +@pytest.mark.parametrize( + "word", + ["PRODUCTION", "PROCESSING", "IDENTIFIER", "LINESTRING", "ABCDEFGHIJKLMNOP"], +) +def test_accepts_allcaps_words_without_digits(tmp_path: Path, word: str) -> None: + """A hard gate that trips on `status = "PROCESSING"` would be unusable.""" + assert_accepted(run_checker(str(write(tmp_path, "enum.py", f'x = "{word}"')))) + + +def test_accepts_lowercase_shape(tmp_path: Path) -> None: + """The shape is uppercase; a lowercase look-alike must not trip it.""" + lowered = FAKE_SECRET.lower() + assert_accepted(run_checker(str(write(tmp_path, "low.py", f'x = "{lowered}"')))) + + +def test_allowlist_pragma_suppresses(tmp_path: Path) -> None: + body = f'x = "{FAKE_SECRET}" # pragma: allowlist secret' + assert_accepted(run_checker(str(write(tmp_path, "ok.py", body)))) + + +# --- key-context: forward compatibility for other key formats ---------------- + + +OFFSHAPE = "gk-live-" + "9f3a2b7c1d4e" # pragma: allowlist secret + + +@pytest.mark.parametrize( + ("name", "body"), + [ + ("bare.py", f'{KEY_SECRET} = "{OFFSHAPE}"'), + ("prefixed.py", f'GRAPHISTRY_PERSONAL_KEY_SECRET = "{OFFSHAPE}"'), + ("getenv.py", f'os.getenv("GRAPHISTRY_PERSONAL_KEY_SECRET", "{OFFSHAPE}")'), + ("dict.py", f'creds = {{"{KEY_SECRET}": "{OFFSHAPE}"}}'), + ("conf.yml", f'{KEY_SECRET}: "{OFFSHAPE}"'), + ("annotated.py", f'{KEY_SECRET}: str = "{OFFSHAPE}"'), + ], + ids=OFFSHAPE_IDS, +) +def test_rejects_offshape_value_under_key_name( + tmp_path: Path, name: str, body: str +) -> None: + """A future key format still trips the contextual net, whatever the spelling.""" + result = run_checker(str(write(tmp_path, name, body))) + + assert_rejected(result, OFFSHAPE) + assert "key-context" in result.stderr + + +@pytest.mark.parametrize( + "value", + [ + "correcthorsebatterystaple", # alphabetic-only passphrase + "0123456789012345", # numeric-only key id + ], +) +def test_key_context_accepts_alpha_only_and_digit_only_secrets( + tmp_path: Path, value: str +) -> None: + """Requiring letter AND digit vetoed these realistic formats.""" + body = f'{KEY_SECRET} = "{value}"' + assert run_checker(str(write(tmp_path, "f.py", body))).returncode == 1 + + +@pytest.mark.parametrize( + "value", + [ + TOO_SHORT, # 9 chars: below the length floor + LOW_DISTINCT, + ], +) +def test_key_context_accepts_low_entropy_values(tmp_path: Path, value: str) -> None: + """Pins `_looks_like_secret` itself, not `_is_placeholder`. + + These deliberately avoid every placeholder marker — an earlier version used + `"SHORTMOCK"` and `"aaaaaa"`, which `_is_placeholder` rejects first ("mock" + is a marker substring; `aaaaaa` is single-character filler), so the entropy + thresholds were never exercised and both floors could be mutated to 1 with + the suite still green. + """ + assert not _is_placeholder(value), f"{value!r} must not be a placeholder" + body = f'{KEY_SECRET} = "{value}"' + assert_accepted(run_checker(str(write(tmp_path, "f.py", body)))) + + +def test_key_context_rejects_just_above_the_entropy_floor(tmp_path: Path) -> None: + """The other side of the boundary, so the floors cannot simply be raised.""" + value = ABOVE_FLOOR + assert not _is_placeholder(value) + assert_rejected( + run_checker(str(write(tmp_path, "f.py", f'{KEY_SECRET} = "{value}"'))), value + ) + + +# --- value-shape boundary: the base64 lookarounds -------------------------- + + +@pytest.mark.parametrize("boundary", ["+", "/", "="]) +def test_shape_bounded_by_base64_chars_is_ignored( + tmp_path: Path, boundary: str +) -> None: + """A 10/16-char run inside a base64 blob is not a credential. + + Embedded PNG data URIs in `.ipynb` produce these at roughly one per 400 KB, + and a data URI has nowhere to put a `# pragma` escape. + """ + body = f"data = 'xx{boundary}{FAKE_SECRET}{boundary}yy'" + assert_accepted(run_checker(str(write(tmp_path, "nb.txt", body)))) + + +def test_shape_preceded_by_equals_is_still_caught(tmp_path: Path) -> None: + """`=` is base64 padding only when trailing; `.env` uses it as a separator.""" + body = f"GRAPHISTRY_PERSONAL_KEY_SECRET={FAKE_SECRET}" + assert_rejected(run_checker(str(write(tmp_path, ".env", body))), FAKE_SECRET) + + +def test_no_false_positives_on_real_base64(tmp_path: Path) -> None: + """Regression guard for the measurement the lookarounds were tuned against.""" + import base64 + import random + + rng = random.Random(20260725) + blob = base64.b64encode(bytes(rng.randrange(256) for _ in range(60_000))).decode() + candidate = write(tmp_path, "blob.txt", f'img = "{blob}"') + + assert_accepted(run_checker(str(candidate))) + + +# --- accepted: placeholders and mock values --------------------------------- + + +@pytest.mark.parametrize( + "body", + [ + f'{KEY_ID} = ""', + f'{KEY_SECRET} = ""', + f'{KEY_SECRET} = "${{PERSONAL_KEY_SECRET}}"', + f'{KEY_ID} = "pk_id"', + f'{KEY_SECRET} = "test-secret"', + f'{KEY_SECRET} = "XXXXXXXXXXXXXXXX"', + f'{KEY_SECRET} = ""', + # substring placeholder markers, not just prefixes + f'{KEY_SECRET} = "my-fake-key-1234"', + f'{KEY_SECRET} = "some-example-value-9"', + ], + ids=PLACEHOLDER_IDS, +) +def test_accepts_placeholders_and_mocks(tmp_path: Path, body: str) -> None: + assert_accepted(run_checker(str(write(tmp_path, "safe.py", body)))) + + +def test_skips_binary_files(tmp_path: Path) -> None: + """Real quoted content, so only the binary guard can prevent a finding.""" + candidate = tmp_path / "blob.bin" + candidate.write_bytes( + b"\x00\x01" + f'{KEY_SECRET} = "{FAKE_SECRET}"'.encode() + b"\x00" + ) + + assert_accepted(run_checker(str(candidate))) + + +# --- index vs worktree ------------------------------------------------------ + + +@pytest.fixture +def git_repo(tmp_path: Path) -> Path: + repo = tmp_path / "repo" + repo.mkdir() + for command in ( + ["git", "init", "-q"], + ["git", "config", "user.email", "t@example.com"], + ["git", "config", "user.name", "t"], + ): + subprocess.run(command, cwd=repo, check=True, capture_output=True) + return repo + + +def test_staged_mode_catches_credential_cleaned_from_worktree(git_repo: Path) -> None: + """The exact evasion --staged exists to stop. + + Stage a credential, then clean the working tree. A worktree scan passes; + the staged scan must still fail, because the commit would carry the secret. + """ + target = git_repo / "config.py" + target.write_text(f'{KEY_SECRET} = "{FAKE_SECRET}"\n', encoding="utf-8") + subprocess.run(["git", "add", "config.py"], cwd=git_repo, check=True) + target.write_text( + f'{KEY_SECRET} = ""\n', encoding="utf-8" + ) + + assert_accepted(run_checker(cwd=git_repo)) + assert_rejected(run_checker("--staged", cwd=git_repo), FAKE_SECRET) + + +def test_staged_mode_catches_rename_plus_edit(git_repo: Path) -> None: + """Rename detection reports `R`, which --diff-filter=ACM drops. + + Without --no-renames this is a working pre-commit bypass. + """ + original = git_repo / "clean.py" + original.write_text("x = 1\n" * 20, encoding="utf-8") + subprocess.run(["git", "add", "clean.py"], cwd=git_repo, check=True) + subprocess.run( + ["git", "commit", "-qm", "seed"], cwd=git_repo, check=True, capture_output=True + ) + + subprocess.run(["git", "mv", "clean.py", "config.py"], cwd=git_repo, check=True) + renamed = git_repo / "config.py" + renamed.write_text( + renamed.read_text(encoding="utf-8") + f'{KEY_SECRET} = "{FAKE_SECRET}"\n', + encoding="utf-8", + ) + subprocess.run(["git", "add", "-A"], cwd=git_repo, check=True) + + assert_rejected(run_checker("--staged", cwd=git_repo), FAKE_SECRET) + + +def test_worktree_mode_scans_tracked_files(git_repo: Path) -> None: + target = git_repo / "config.py" + target.write_text(f'{KEY_SECRET} = "{FAKE_SECRET}"\n', encoding="utf-8") + subprocess.run(["git", "add", "config.py"], cwd=git_repo, check=True) + + assert_rejected(run_checker(cwd=git_repo), FAKE_SECRET) + + +def test_clean_repo_passes_both_modes(git_repo: Path) -> None: + target = git_repo / "config.py" + target.write_text(f'{KEY_ID} = ""\n', encoding="utf-8") + subprocess.run(["git", "add", "config.py"], cwd=git_repo, check=True) + + assert_accepted(run_checker(cwd=git_repo)) + assert_accepted(run_checker("--staged", cwd=git_repo)) + + +# --- internal-host rule ----------------------------------------------------- +# +# The local .git/hooks/pre-commit enumerated two specific dev hostnames, but it +# is untracked: no other contributor has it and CI never runs it. The rule here +# is domain-level and tracked, so it is enforced for everyone and covers +# subdomains nobody has thought of yet. + + +# Assembled at runtime so this file carries no internal hostname of its own — +# the rule under test would otherwise reject the file that tests it. +_DEV_DOMAIN = "grph" + ".xyz" +_INT_DOMAIN = "louie" + ".internal" + + +@pytest.mark.parametrize( + ("name", "body"), + [ + ("url.py", f'u = "https://louie-dev.{_DEV_DOMAIN}"'), + ("unknown_subdomain.py", f'u = "https://something-new.{_DEV_DOMAIN}"'), + ("bare.md", f"See graphistry-dev.{_DEV_DOMAIN} for the dev instance."), + ("env", f"LOUIE_SERVER=louie-dev.{_DEV_DOMAIN}"), + ("internal.py", f'u = "https://dev.k8s.{_INT_DOMAIN}"'), + ], + ids=["url", "unknown_subdomain", "markdown", "dotenv", "louie_internal"], +) +def test_rejects_internal_hostnames(tmp_path: Path, name: str, body: str) -> None: + result = run_checker(str(write(tmp_path, name, body))) + + assert result.returncode == 1, result.stderr + assert "internal-host" in result.stderr + + +@pytest.mark.parametrize( + "body", + [ + 'u = "https://louie.example.com"', # RFC 2606 + 'u = "https://hub.graphistry.com"', # public endpoint + 'u = "https://den.louie.ai"', # public endpoint + 'u = "https://example.com/grph"', # path, not a host + ], + ids=["example_com", "hub", "den", "path_only"], +) +def test_accepts_public_and_example_hosts(tmp_path: Path, body: str) -> None: + assert_accepted(run_checker(str(write(tmp_path, "ok.py", body)))) + + +def test_internal_host_respects_the_pragma(tmp_path: Path) -> None: + body = f'u = "https://louie-dev.{_DEV_DOMAIN}" # pragma: allowlist secret' + assert_accepted(run_checker(str(write(tmp_path, "ok.py", body)))) diff --git a/tests/unit/security/test_secret_gate.py b/tests/unit/security/test_secret_gate.py new file mode 100644 index 0000000..1d84044 --- /dev/null +++ b/tests/unit/security/test_secret_gate.py @@ -0,0 +1,606 @@ +"""The secret gates must be tested in the *rejecting* direction. + +`.github/workflows/secret-detection-test.yml` only ever asserted that +`secret-detection.sh` and `pre-commit-secret-check.sh` exit 0 on a clean tree. +A gate hardwired to exit 0 satisfies that forever — which is exactly what +happened: `detect-secrets scan --baseline ` is an update command that always +exits 0, so both gate modes were unreachable and a live credential passed them +for ~12 months across five green workflow runs. + +These tests plant secrets and assert rejection. +""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +from tests.utils import repo_root + +pytestmark = pytest.mark.unit + +REPO_ROOT = repo_root() +CHECK_NEW = REPO_ROOT / "scripts" / "ci" / "check_new_secrets.py" +GATE = REPO_ROOT / "scripts" / "ci" / "secret-detection.sh" + +# A 10-char Graphistry personal-key shape that `detect-secrets` does NOT flag. +# +# This must stay invisible to detect-secrets, otherwise the shell-level gate +# tests below stop isolating the Graphistry rule: the detect-secrets half would +# reject the fixture on its own and the tests would pass even with the +# credential gate unwired. An earlier value here was itself flagged by +# detect-secrets, which silently defeated exactly that. Assembled at runtime so +# no scannable literal exists in this file. +GKEY = "K3PQ7" + "RTX2M" + +# Fake digest for baseline fixtures; a constant so the formatter cannot move a +# same-line pragma off it. +DIGEST = "abc" + "123" + + +def _baseline(*entries: tuple[str, str]) -> dict: + """Baseline from `(path, hashed_secret)` pairs — matching is per-path.""" + results: dict[str, list[dict]] = {} + for path, digest in entries: + results.setdefault(path, []).append( + {"type": "Secret Keyword", "hashed_secret": digest, "line_number": 1} + ) + return {"version": "1.5.0", "results": results} + + +def _scan(*entries: tuple[str, int, str, str]) -> dict: + results: dict[str, list[dict]] = {} + for path, line, kind, digest in entries: + results.setdefault(path, []).append( + {"type": kind, "hashed_secret": digest, "line_number": line} + ) + return {"version": "1.5.0", "results": results} + + +def run_check_new(scan: dict, baseline: dict, tmp_path: Path): + scan_path = tmp_path / "scan.json" + base_path = tmp_path / "base.json" + scan_path.write_text(json.dumps(scan), encoding="utf-8") + base_path.write_text(json.dumps(baseline), encoding="utf-8") + return subprocess.run( + [ + sys.executable, + str(CHECK_NEW), + "--baseline", + str(base_path), + "--scan", + str(scan_path), + ], + capture_output=True, + text=True, + check=False, + ) + + +# --- check_new_secrets: the comparison the shell was only assuming ---------- + + +def test_rejects_finding_absent_from_baseline(tmp_path: Path) -> None: + result = run_check_new( + _scan(("app/config.py", 12, "Secret Keyword", "deadbeef")), + _baseline(), + tmp_path, + ) + + assert result.returncode == 1 + assert "app/config.py:12" in result.stderr + assert "Secret Keyword" in result.stderr + + +def test_accepts_finding_already_baselined(tmp_path: Path) -> None: + result = run_check_new( + _scan(("app/config.py", 12, "Secret Keyword", "deadbeef")), + _baseline(("app/config.py", "deadbeef")), + tmp_path, + ) + + assert result.returncode == 0, result.stderr + + +def test_accepts_empty_scan_results(tmp_path: Path) -> None: + assert run_check_new(_scan(), _baseline(), tmp_path).returncode == 0 + + +def test_reports_every_new_finding(tmp_path: Path) -> None: + result = run_check_new( + _scan( + ("a.py", 1, "Secret Keyword", "aaa"), + ("b.py", 2, "AWS Access Key", "bbb"), + ("b.py", 3, "Secret Keyword", "ccc"), + ), + _baseline(("a.py", "aaa")), + tmp_path, + ) + + assert result.returncode == 1 + assert "b.py:2" in result.stderr and "b.py:3" in result.stderr + assert "a.py" not in result.stderr # baselined + assert "2 finding(s)" in result.stderr + + +def test_baseline_is_keyed_by_path_not_hash_alone(tmp_path: Path) -> None: + """A value allowlisted in one file must not be accepted in another. + + Hash-only matching would let a demo password baselined in `docs/` pass + silently once copied into `src/louieai/_client.py` — the exact file where + this repo's credential incident happened. + """ + baseline = { + "version": "1.5.0", + "results": { + "docs/example.md": [ + {"type": "Secret Keyword", "hashed_secret": DIGEST, "line_number": 1} + ] + }, + } + scan = _scan(("src/louieai/_client.py", 9, "Secret Keyword", "abc123")) + + result = run_check_new(scan, baseline, tmp_path) + + assert result.returncode == 1 + assert "src/louieai/_client.py:9" in result.stderr + + +def test_baseline_still_accepts_the_same_path(tmp_path: Path) -> None: + baseline = { + "version": "1.5.0", + "results": { + "docs/example.md": [ + {"type": "Secret Keyword", "hashed_secret": DIGEST, "line_number": 1} + ] + }, + } + scan = _scan(("docs/example.md", 1, "Secret Keyword", "abc123")) + + assert run_check_new(scan, baseline, tmp_path).returncode == 0 + + +def test_concatenated_scans_are_merged(tmp_path: Path) -> None: + """`xargs` splits on ARG_MAX, emitting several JSON documents on one stream. + + A single `json.loads` raised `Extra data: line N`, which surfaced as + "New secrets detected!" on a tree with no secrets — fail-closed, but + blaming the developer for the wrong thing. + """ + scan_path = tmp_path / "scan.json" + base_path = tmp_path / "base.json" + scan_path.write_text( + json.dumps(_scan(("a.py", 1, "Secret Keyword", "aaa"))) + + "\n" + + json.dumps(_scan(("b.py", 2, "Secret Keyword", "bbb"))), + encoding="utf-8", + ) + base_path.write_text(json.dumps(_baseline()), encoding="utf-8") + + result = subprocess.run( + [ + sys.executable, + str(CHECK_NEW), + "--baseline", + str(base_path), + "--scan", + str(scan_path), + ], + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 1 + assert "a.py:1" in result.stderr and "b.py:2" in result.stderr + + +def test_scan_without_results_key_is_an_error(tmp_path: Path) -> None: + """Valid JSON that is not a scan must not read as 'no findings'.""" + scan_path = tmp_path / "scan.json" + base_path = tmp_path / "base.json" + scan_path.write_text('{"version": "1.5.0"}', encoding="utf-8") + base_path.write_text(json.dumps(_baseline()), encoding="utf-8") + + result = subprocess.run( + [ + sys.executable, + str(CHECK_NEW), + "--baseline", + str(base_path), + "--scan", + str(scan_path), + ], + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 1 + assert "results" in result.stderr + + +@pytest.mark.parametrize("baseline_body", ["", "{not json", '{"results": ']) +def test_unreadable_baseline_fails_closed(tmp_path: Path, baseline_body: str) -> None: + """A truncated baseline must not be treated as 'nothing is accepted... fine'.""" + scan_path = tmp_path / "scan.json" + base_path = tmp_path / "base.json" + scan_path.write_text(json.dumps(_scan()), encoding="utf-8") + base_path.write_text(baseline_body, encoding="utf-8") + + result = subprocess.run( + [ + sys.executable, + str(CHECK_NEW), + "--baseline", + str(base_path), + "--scan", + str(scan_path), + ], + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 1 + assert "baseline" in result.stderr + + +def test_missing_baseline_file_fails_closed(tmp_path: Path) -> None: + scan_path = tmp_path / "scan.json" + scan_path.write_text(json.dumps(_scan()), encoding="utf-8") + + result = subprocess.run( + [ + sys.executable, + str(CHECK_NEW), + "--baseline", + str(tmp_path / "nope.json"), + "--scan", + str(scan_path), + ], + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 1 + + +def test_credential_gate_exclude_files_flag(tmp_path: Path) -> None: + """The documented escape for files that cannot carry a pragma.""" + candidate = tmp_path / "data.csv" + candidate.write_text(f"code,{GKEY},ok\n", encoding="utf-8") + checker = str(REPO_ROOT / "scripts/ci/check_credential_literals.py") + + without = subprocess.run( + [sys.executable, checker, str(candidate)], + capture_output=True, + text=True, + check=False, + ) + with_flag = subprocess.run( + [sys.executable, checker, "--exclude-files", r"\.csv$", str(candidate)], + capture_output=True, + text=True, + check=False, + ) + + assert without.returncode == 1 + assert with_flag.returncode == 0, with_flag.stderr + + +def test_empty_scan_output_is_an_error_not_a_pass(tmp_path: Path) -> None: + """A failed scan produces no output; treating that as 'clean' is how the + old pre-commit branch silently skipped its check.""" + scan_path = tmp_path / "scan.json" + base_path = tmp_path / "base.json" + scan_path.write_text("", encoding="utf-8") + base_path.write_text(json.dumps(_baseline()), encoding="utf-8") + + result = subprocess.run( + [ + sys.executable, + str(CHECK_NEW), + "--baseline", + str(base_path), + "--scan", + str(scan_path), + ], + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 1 + assert "did not run" in result.stderr + + +# --- end-to-end: the gate script itself must fail on a planted secret ------- + + +@pytest.fixture +def gate_repo(tmp_path: Path) -> Path: + """A minimal repo wired with the real gate scripts.""" + if shutil.which("detect-secrets") is None: + # A silent skip would let the end-to-end gate tests vanish in CI, which + # is the same "green but unverified" failure this file exists to stop. + if os.environ.get("CI"): + pytest.fail("detect-secrets must be on PATH in CI") + pytest.skip("detect-secrets not on PATH") + + repo = tmp_path / "repo" + (repo / "scripts" / "ci").mkdir(parents=True) + for name in ("check_new_secrets.py", "check_credential_literals.py"): + shutil.copy(REPO_ROOT / "scripts" / "ci" / name, repo / "scripts" / "ci" / name) + shutil.copy(GATE, repo / "scripts" / "ci" / "secret-detection.sh") + (repo / "scripts" / "ci" / "secret-detection.sh").chmod(0o755) + (repo / "pyproject.toml").touch() + shutil.copy(REPO_ROOT / ".secrets.baseline", repo / ".secrets.baseline") + + for command in ( + ["git", "init", "-q"], + ["git", "config", "user.email", "t@example.com"], + ["git", "config", "user.name", "t"], + ): + subprocess.run(command, cwd=repo, check=True, capture_output=True) + return repo + + +def run_gate(repo: Path, *args: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ["./scripts/ci/secret-detection.sh", *args], + cwd=repo, + capture_output=True, + text=True, + check=False, + ) + + +def test_ci_gate_rejects_planted_secret(gate_repo: Path) -> None: + """Before the fix this printed '✅ Secret detection passed' and exited 0.""" + (gate_repo / "leak.py").write_text( + 'api_key = "zQ3RtP8xL2mN7vB4kW9jH6dF1sA5gY0c"\n', # pragma: allowlist secret + encoding="utf-8", + ) + subprocess.run(["git", "add", "-A"], cwd=gate_repo, check=True, capture_output=True) + + assert run_gate(gate_repo).returncode == 1 + + +def test_precommit_gate_rejects_staged_secret(gate_repo: Path) -> None: + """Before the fix this printed '✅ No secrets detected' and exited 0.""" + (gate_repo / "leak.py").write_text( + 'password = "Tr0ub4dor&3-notabaseline"\n', # pragma: allowlist secret + encoding="utf-8", + ) + subprocess.run(["git", "add", "-A"], cwd=gate_repo, check=True, capture_output=True) + + assert run_gate(gate_repo, "--check-only").returncode == 1 + + +# The Graphistry gate is a separate script invoked *by* secret-detection.sh. +# Testing it directly leaves the wiring untested: removing its invocation, or +# dropping `--staged` from the pre-commit branch, both left the suite fully +# green. These exercise it through the shell entry point. + + +def test_gate_rejects_graphistry_literal_through_the_shell(gate_repo: Path) -> None: + """Kills the mutant where the credential-gate call is removed from the shell.""" + (gate_repo / "conf.py").write_text(f'k = "{GKEY}"\n', encoding="utf-8") + subprocess.run(["git", "add", "-A"], cwd=gate_repo, check=True, capture_output=True) + + assert run_gate(gate_repo).returncode == 1 + assert run_gate(gate_repo, "--check-only").returncode == 1 + + +def test_precommit_gate_uses_the_index_for_graphistry_literals(gate_repo: Path) -> None: + """Kills the mutant where `--staged` is dropped from the pre-commit branch. + + Stage the credential, then clean the worktree: only an index-aware scan + still sees it. + """ + target = gate_repo / "conf.py" + target.write_text(f'k = "{GKEY}"\n', encoding="utf-8") + subprocess.run(["git", "add", "-A"], cwd=gate_repo, check=True, capture_output=True) + target.write_text('k = ""\n', encoding="utf-8") + + assert run_gate(gate_repo, "--check-only").returncode == 1 + + +def test_gate_rejects_rename_plus_edit(gate_repo: Path) -> None: + """`git mv` + edit reports `R`, which --diff-filter=ACM drops.""" + original = gate_repo / "big.py" + original.write_text("x = 1\n" * 40, encoding="utf-8") + subprocess.run(["git", "add", "-A"], cwd=gate_repo, check=True, capture_output=True) + subprocess.run( + ["git", "commit", "-qm", "seed"], cwd=gate_repo, check=True, capture_output=True + ) + + subprocess.run(["git", "mv", "big.py", "cfg.py"], cwd=gate_repo, check=True) + renamed = gate_repo / "cfg.py" + # Assembled at runtime so this file carries no scannable literal of its own. + planted = ( + "aws_secret_access_key" + ' = "' + "wJalrXUtnFEMI" + "K7MDENGbPxRfi" + '"\n' + ) + renamed.write_text(renamed.read_text(encoding="utf-8") + planted, encoding="utf-8") + subprocess.run(["git", "add", "-A"], cwd=gate_repo, check=True, capture_output=True) + + assert run_gate(gate_repo, "--check-only").returncode == 1 + + +def test_gate_handles_filenames_with_spaces(gate_repo: Path) -> None: + """`xargs` word-splitting made a spaced filename scan nothing and pass.""" + (gate_repo / "zz spaced.py").write_text( + 'password = "Tr0ub4dor&3-notabaseline"\n', # pragma: allowlist secret + encoding="utf-8", + ) + subprocess.run(["git", "add", "-A"], cwd=gate_repo, check=True, capture_output=True) + + assert run_gate(gate_repo, "--check-only").returncode == 1 + + +def test_missing_baseline_does_not_silently_pass(gate_repo: Path) -> None: + """Generating a baseline accepts everything, so that run must not succeed.""" + (gate_repo / "leak.py").write_text( + 'api_key = "zQ3RtP8xL2mN7vB4kW9jH6dF1sA5gY0c"\n', # pragma: allowlist secret + encoding="utf-8", + ) + subprocess.run(["git", "add", "-A"], cwd=gate_repo, check=True, capture_output=True) + (gate_repo / ".secrets.baseline").unlink() + + assert run_gate(gate_repo).returncode == 1 + + +# The remediation guidance must be discoverable at the moment of failure, not +# only in a doc nobody reads. These pin that the failure output actually teaches +# the fix, and that the doc it points at exists and covers it. + +DOC = REPO_ROOT / ".secret-patterns.md" +FIXTURE_SECTION = "Writing tests that contain deliberate fake secrets" + + +def test_credential_gate_failure_explains_how_to_fix(tmp_path: Path) -> None: + candidate = tmp_path / "f.py" + candidate.write_text(f'k = "{GKEY}"\n', encoding="utf-8") + + result = subprocess.run( + [ + sys.executable, + str(REPO_ROOT / "scripts/ci/check_credential_literals.py"), + str(candidate), + ], + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 1 + assert "pragma: allowlist secret" in result.stderr + assert "SAME line" in result.stderr # the mistake that silently does nothing + assert "at runtime" in result.stderr # the preferred fixture technique + assert "--exclude-files" in result.stderr + assert ".secret-patterns.md" in result.stderr + + +def test_new_secret_failure_explains_how_to_fix(tmp_path: Path) -> None: + result = run_check_new( + _scan(("app/config.py", 12, "Secret Keyword", "deadbeef")), + _baseline(), + tmp_path, + ) + + assert result.returncode == 1 + assert "pragma: allowlist secret" in result.stderr + assert "at runtime" in result.stderr + assert "ANYWHERE in the tree" in result.stderr # why baselining is the last resort + assert ".secret-patterns.md" in result.stderr + + +def test_documented_guidance_exists_and_covers_all_three_routes() -> None: + """The error messages point here; the section must actually be present.""" + doc = DOC.read_text(encoding="utf-8") + + assert FIXTURE_SECTION in doc + section = doc.split(FIXTURE_SECTION, 1)[1] + for expected in ("at runtime", "same line", "--exclude-files", "Never re-baseline"): + assert expected.lower() in section.lower(), expected + + +def test_gate_scans_the_last_staged_file(gate_repo: Path) -> None: + """The staged list is NUL-*terminated*, not NUL-separated. + + `read -r -d ''` only emits a field when it sees the delimiter, so joining + paths with NUL instead of terminating each one silently dropped whichever + file sorted last — it was never scanned at all. + """ + # 'zzz_' sorts after every other file the fixture stages. + (gate_repo / "zzz_last.py").write_text( + 'password = "Tr0ub4dor&3-notabaseline"\n', # pragma: allowlist secret + encoding="utf-8", + ) + subprocess.run(["git", "add", "-A"], cwd=gate_repo, check=True, capture_output=True) + + assert run_gate(gate_repo, "--check-only").returncode == 1 + + +def test_gate_passes_on_clean_tree(gate_repo: Path) -> None: + (gate_repo / "ok.py").write_text('password = ""\n', encoding="utf-8") + subprocess.run(["git", "add", "-A"], cwd=gate_repo, check=True, capture_output=True) + + assert run_gate(gate_repo).returncode == 0 + assert run_gate(gate_repo, "--check-only").returncode == 0 + + +def test_verbose_test_ids_do_not_print_credential_shapes(tmp_path: Path) -> None: + """`pytest -v` must not echo fixture values into public CI logs. + + Parametrize ids default to the parameter *values*, so the security suite's + own fake credentials were being printed into the CI log of every run. The + fakes are harmless; the habit is not — the day someone parametrizes with a + real value it lands in a public log. + """ + log = tmp_path / "verbose.log" + result = subprocess.run( + [ + sys.executable, + "-m", + "pytest", + str(REPO_ROOT / "tests/unit/test_credential_literals.py"), + "-v", + "--no-header", + "-p", + "no:cacheprovider", + ], + capture_output=True, + text=True, + check=False, + cwd=REPO_ROOT, + ) + log.write_text(result.stdout + result.stderr, encoding="utf-8") + + scan = subprocess.run( + [ + sys.executable, + str(REPO_ROOT / "scripts/ci/check_credential_literals.py"), + str(log), + ], + capture_output=True, + text=True, + check=False, + ) + + assert scan.returncode == 0, scan.stderr + + +def test_uv_fallback_pins_the_project_root() -> None: + """The uv fallback must pass --project. + + The pre-commit path scans a materialised copy of the index from a temp + directory (`cd "$STAGE_DIR"`). A bare `uv run` there cannot locate the + project and dies with "Failed to spawn: detect-secrets", so the gate fails + for a reason unrelated to secrets. + + CI never sees it: it activates the venv, which puts detect-secrets on PATH + and skips the fallback entirely. Only a developer's hook hits it, which is + why this is asserted on the script text rather than exercised end-to-end — + the scratch repo the other tests use has no uv project for the fallback to + resolve against. + """ + script = GATE.read_text(encoding="utf-8") + fallback = [ + line + for line in script.splitlines() + if "DETECT_SECRETS=" in line and "uv run" in line + ] + + assert fallback, "expected a `uv run` fallback assignment" + for line in fallback: + assert "--project" in line, f"uv fallback must pin --project: {line.strip()}" + assert "--frozen" in line, f"uv fallback must pass --frozen: {line.strip()}" diff --git a/tests/unit/test_cascade_simple.py b/tests/unit/test_cascade_simple.py index 83b0ebf..b740d00 100644 --- a/tests/unit/test_cascade_simple.py +++ b/tests/unit/test_cascade_simple.py @@ -12,7 +12,7 @@ class TestSimpleCascade: def test_user_scenario_fixed(self): """Test the exact user scenario that was broken.""" - target_org = "databricks-pat-botsv3" + target_org = "example-org" # Create a simple mock that doesn't auto-generate attributes class MockGraphistry: @@ -27,7 +27,7 @@ def api_token(self): with patch("graphistry.pygraphistry.GraphistryClient", return_value=mock_g): # User's original code that was broken: lui = louieai.louie( - mock_g, server_url="https://louie-dev.grph.xyz", share_mode="Private" + mock_g, server_url="https://louie.example.com", share_mode="Private" ) # Check if org was extracted correctly @@ -65,7 +65,7 @@ def api_token(self): lui = louieai.louie( mock_g, org_name=explicit_org, # This should win - server_url="https://louie-dev.grph.xyz", + server_url="https://louie.example.com", ) stored_org = lui._client._auth_manager._credentials.get("org_name") @@ -91,7 +91,7 @@ def api_token(self): patch("graphistry.pygraphistry.GraphistryClient", return_value=mock_g), patch.dict(os.environ, {"GRAPHISTRY_ORG_NAME": env_org}), ): - lui = louieai.louie(mock_g, server_url="https://louie-dev.grph.xyz") + lui = louieai.louie(mock_g, server_url="https://louie.example.com") stored_org = lui._client._auth_manager._credentials.get("org_name") assert stored_org == env_org, ( diff --git a/tests/unit/test_org_auth_flow.py b/tests/unit/test_org_auth_flow.py index 20377c3..484160f 100644 --- a/tests/unit/test_org_auth_flow.py +++ b/tests/unit/test_org_auth_flow.py @@ -15,7 +15,7 @@ class TestOrgAuthFlow: def test_direct_client_org_name_flow(self): """Test that LouieClient correctly handles org_name parameter.""" - target_org = "databricks-pat-botsv3" + target_org = "example-org" # Mock PyGraphistry client mock_graphistry = MagicMock() @@ -23,11 +23,11 @@ def test_direct_client_org_name_flow(self): with patch("louieai.auth.GraphistryClient", return_value=mock_graphistry): client = LouieClient( - personal_key_id="CU5V6VZJB7", - personal_key_secret="32RBP6PUCSUVAIYJ", + personal_key_id="test-personal-key-id", + personal_key_secret="test-personal-key-secret", org_name=target_org, - graphistry_server="graphistry-dev.grph.xyz", - server_url="https://louie-dev.grph.xyz", + graphistry_server="graphistry.example.com", + server_url="https://louie.example.com", ) # Verify org_name is stored in credentials @@ -41,7 +41,7 @@ def test_direct_client_org_name_flow(self): def test_louie_factory_with_graphistry_client_confused_deputy(self): """Test the confused deputy problem: louie() factory loses org_name from pre-registered graphistry.""" - target_org = "databricks-pat-botsv3" + target_org = "example-org" # Mock a pre-registered PyGraphistry client (simulating user's scenario) mock_graphistry = MagicMock() @@ -58,7 +58,7 @@ def test_louie_factory_with_graphistry_client_confused_deputy(self): # Test the louie() factory function with pre-registered graphistry lui = louieai.louie( graphistry_client=mock_graphistry, - server_url="https://louie-dev.grph.xyz", + server_url="https://louie.example.com", share_mode="Private", ) @@ -96,7 +96,7 @@ def test_org_slug_conversion(self): """Test that org names are properly converted to slug format.""" test_cases = [ ("My Organization", "my-organization"), - ("databricks-pat-botsv3", "databricks-pat-botsv3"), + ("example-org", "example-org"), ("Test_Org-123", "test-org-123"), ("UPPERCASE ORG", "uppercase-org"), ("Org with Special@#$%", "org-with-special"), @@ -173,12 +173,12 @@ def test_reproduction_of_user_scenario(self): mock_registered_client.api_token.return_value = "user-jwt-token" mock_g_register.return_value = mock_registered_client - # Simulate: g = graphistry.register(org_name='databricks-pat-botsv3', ...) + # Simulate: g = graphistry.register(org_name='example-org', ...) g = mock_registered_client # Step 2: User creates LouieAI interface with the registered client lui = louieai.louie( - g, server_url="https://louie-dev.grph.xyz", share_mode="Private" + g, server_url="https://louie.example.com", share_mode="Private" ) # Step 3: User makes a query - this should use the correct org @@ -188,9 +188,9 @@ def test_reproduction_of_user_scenario(self): auth_manager = lui._client._auth_manager stored_org = auth_manager._credentials.get("org_name") - # This should be 'databricks-pat-botsv3' but is likely None - assert stored_org == "databricks-pat-botsv3", ( - f"Confused deputy: Expected 'databricks-pat-botsv3', got '{stored_org}'" + # This should be 'example-org' but is likely None + assert stored_org == "example-org", ( + f"Confused deputy: Expected 'example-org', got '{stored_org}'" ) # Check headers that would be sent to LouieAI API @@ -199,6 +199,6 @@ def test_reproduction_of_user_scenario(self): "Missing org header - API calls will be made as 'personal'" ) - assert headers["X-Graphistry-Org"] == "databricks-pat-botsv3", ( + assert headers["X-Graphistry-Org"] == "example-org", ( f"Wrong org in API headers: {headers.get('X-Graphistry-Org')}" ) diff --git a/tests/utils.py b/tests/utils.py index 78d19e5..1b79790 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,19 +1,66 @@ """Test utilities for Louie.ai client.""" import os +from pathlib import Path from dotenv import load_dotenv +def repo_root() -> Path: + """Repository root, resolved by walking up to the pyproject.toml. + + Deliberately not `Path(__file__).parents[N]`: a hardcoded depth breaks + silently when a test file moves between directories, and several security + tests copy scripts out of the root — a wrong root there yields a fixture + that exercises nothing while still passing. + """ + for candidate in [Path(__file__).resolve(), *Path(__file__).resolve().parents]: + if (candidate / "pyproject.toml").is_file(): + return candidate + raise RuntimeError("could not locate repository root (no pyproject.toml found)") + + +# Modes that are allowed to reach a real server, and therefore to read `.env`. +INTEGRATION_MODES = frozenset({"integration", "all"}) + + +def get_test_mode() -> str: + """Current test mode. Single source of truth for `LOUIE_TEST_MODE`. + + `tests/conftest.py` imports this rather than re-reading the variable, so the + two cannot drift (they previously disagreed about the `all` mode). + """ + return os.environ.get("LOUIE_TEST_MODE", "unit").lower() + + +def dotenv_enabled() -> bool: + """True when `.env` may supply integration credentials. + + Loading `.env` unconditionally made it impossible to run the suite without + credentials: clearing `GRAPHISTRY_*` / `LOUIE_SERVER` in the environment had + no effect, because `.env` silently put them back and the credentialed + fixtures then dialled the real server (observed: a ~4 minute hang on a run + that was explicitly launched with every credential variable unset). + + `.env` is therefore opt-in. Set `LOUIE_TEST_MODE=integration` (or `all`). + CI is unaffected: it injects real environment variables from secrets and has + no `.env` file. + """ + return get_test_mode() in INTEGRATION_MODES + + def load_test_credentials() -> dict[str, str] | None: """Load test credentials from environment variables. + Reads the process environment. `.env` is consulted only in an integration + mode — see :func:`dotenv_enabled`. + Returns: Dictionary with server, username, and password if all are set. None if any required credential is missing. """ - # Load from .env file if it exists - load_dotenv() + if dotenv_enabled(): + load_dotenv() # Get credentials from environment server = os.getenv("GRAPHISTRY_SERVER")