Skip to content

feat: warn on hardcoded sync secrets#503

Merged
yodakanohoshi merged 2 commits into
drt-hub:mainfrom
Photon101:feat/validate-secret-warnings
May 13, 2026
Merged

feat: warn on hardcoded sync secrets#503
yodakanohoshi merged 2 commits into
drt-hub:mainfrom
Photon101:feat/validate-secret-warnings

Conversation

@Photon101

Copy link
Copy Markdown
Collaborator

Summary

  • Add a raw-YAML hardcoded secret scanner for syncs/*.yml with known token patterns and a Shannon entropy fallback.
  • Wire findings into drt validate as warnings while keeping the default exit code at 0.
  • Add drt validate --strict to promote warnings to validation errors, and include warning details in JSON output.

Closes #424

Validation

  • uv run pytest tests/unit/test_cli_validate.py -v
  • uv run pytest tests/unit/test_deprecations.py tests/unit/test_cli_validate.py -v
  • uv run ruff check drt/config/secrets.py drt/cli/main.py tests/unit/test_cli_validate.py
  • uv run mypy drt/config/secrets.py drt/cli/main.py
  • git diff --check
  • uv run drt validate --help
  • Manual sample: hardcoded destination.auth.token warns with exit 0, and --strict reports the same finding as an error with exit 1.

@codecov

codecov Bot commented May 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.16514% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
drt/cli/main.py 90.90% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@Photon101

Copy link
Copy Markdown
Collaborator Author

Pushed 7651384 to address the Codecov patch warning on the secret scanner.

Added focused tests/unit/test_config_secrets.py coverage for malformed YAML, non-mapping YAML, list recursion/path reporting, _env/_path suffix exclusions, blank/short/whitespace values, high-entropy detection, and entropy helper boundaries.

Local validation:

  • pytest tests/unit/test_config_secrets.py tests/unit/test_cli_validate.py -v -> 23 passed
  • pytest --cov=drt.config.secrets --cov-report=term-missing tests/unit/test_config_secrets.py tests/unit/test_cli_validate.py -q -> drt/config/secrets.py 100% covered
  • ruff check drt/config/secrets.py tests/unit/test_config_secrets.py tests/unit/test_cli_validate.py
  • git diff --check

@yodakanohoshi

Copy link
Copy Markdown
Contributor

🎉 Thanks for the clean implementation of #424!

A few things I particularly liked:

  • Raw-YAML scan before env expansion — Reading the raw YAML rather than the post-Pydantic-expansion view is exactly the right call. ${ENV_VAR} references stay correctly treated as safe, even when the actual env var holds a real secret in the user's environment. No false positives there.
  • Known patterns + entropy fallback, two-tier design — High-confidence patterns like sk- / xoxb- / AKIA give strong signal, and the Shannon entropy ≥ 3.5 fallback catches the long tail. The balance between false positives and missed detections is well thought out.
  • _NON_SECRET_SUFFIXES (_env, _path) — Excluding fields like token_env or password_path (which are secret references, not secrets themselves) is exactly the kind of practical consideration that matters in real-world usage.
  • --strict behavior — Keeping the default exit code at 0 with warnings only, while letting users opt into strict mode for CI, matches the intent of feat: detect hardcoded secrets in sync YAML during drt validate #424 exactly.
  • 8 tests — Edge cases are carefully covered (malformed YAML, nested lists, filename fallback, entropy boundary conditions).

I'll add the [Unreleased] CHANGELOG entry after merge — no action needed on your side.

Also, looking across #496#504: polish work (#496), a Protocol extension (#497), a Postgres bug fix (#498), docs visualization (#502), this PR's security feature, and a new destination (#504) — that's a remarkable breadth of contributions across different layers of the codebase in a short span. The fact that each PR is scoped at an independently reviewable granularity is genuinely helpful on the maintainer side too.

Thanks again, and looking forward to more 🙏

@yodakanohoshi yodakanohoshi merged commit ea0c5a0 into drt-hub:main May 13, 2026
6 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators May 13, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: detect hardcoded secrets in sync YAML during drt validate

2 participants