feat: add GCP Artifact Registry publishing workflow#1
Conversation
|
Warning Rate limit exceeded@williaby has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 56 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (101)
WalkthroughThis PR transitions from PyPI to Google Cloud Artifact Registry publishing, expands Cloudflare Access authentication with configuration management and enhanced middleware, updates Python type annotations to modern union syntax ( Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant GH as GitHub<br/>(version tag push)
participant GHA as GitHub Actions<br/>(Workflow)
participant Inf as Infisical<br/>(Secret Storage)
participant GCP as Google Cloud<br/>(Authentication)
participant AR as Artifact Registry<br/>(Publishing)
Dev->>GH: Push version tag<br/>(e.g., cloudflare-auth-v0.1.0)
GH->>GHA: Trigger publish-artifact-registry
activate GHA
GHA->>GHA: Parse tag for package & version
GHA->>GHA: Validate version matches pyproject.toml
GHA->>Inf: Retrieve GCP Service Account key
note over Inf: (Currently commented/placeholder)
GHA->>GCP: Authenticate with Service Account
GHA->>GCP: Configure Artifact Registry credentials
GHA->>GHA: Build package with uv
alt Dry-run mode
GHA->>GHA: Summarize artifacts
GHA->>Dev: Output summary (no publish)
else Normal mode
GHA->>AR: Publish package via uv publish
AR-->>GHA: Success confirmation
GHA->>Dev: Output job summary<br/>(version, registry, status)
end
GHA->>GHA: Clean up credentials
deactivate GHA
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50–80 minutes Specific areas requiring attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Mutation Testing Results
What is Mutation Testing?Mutation testing introduces small changes (mutations) to your code and checks if your tests detect them. A high mutation score indicates your tests are effective at catching bugs.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
coderabbitai
Bot
added
documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 30
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (26)
docs/development/architecture.md (1)
16-30: Fix the closing code fence (breaks Markdown rendering).
The block opens with```textbut should close with```, not```text.-```text +```text python_libs/ ├── src/ │ └── python_libs/ │ ├── __init__.py # Package initialization │ │ ├── core/ # Core functionality │ │ ├── config.py # Configuration with Pydantic │ │ └── ... │ ├── utils/ # Utility modules │ │ ├── logging.py # Structured logging │ │ └── ... ├── tests/ # Test suite ├── docs/ # Documentation └── pyproject.toml # Project configuration -```text +```.standards/README.md (2)
27-37: Remove the stray ```text fence (renders incorrectly).
Line 37 appears to start atextfence without a corresponding intended block./merge-standards -```text
41-53: Fix the closing code fence for the diagram.
Open with```text, close with```.```text ┌─────────────────┐ cruft update ┌──────────────────┐ │ Template │ ──────────────────► │ .standards/ │ │ Repository │ │ (baselines) │ └─────────────────┘ └────────┬─────────┘ │ │ merge (manual/agent) ▼ ┌──────────────────┐ │ Root files │ │ (customized) │ └──────────────────┘ -```text +```.claude/skills/project-planning/templates/roadmap-template.md (1)
163-168: Remove/replace the stray ```text at the end of the template.
Line 167 starts a fence but doesn’t appear to belong (and would swallow the rest of the doc).- [Architecture Decisions](./adr/) -```text +.github/workflows/ci.yml (1)
25-29: Consider tightening workflow permissions (job-level) if feasible.
Not a regression from this change, butpull-requests: write/checks: writeare often avoidable for test jobs..claude/skills/pr-prepare/SKILL.md (1)
42-50: Fix the step numbering (“### 6. Output” skips 5).
Keeps the workflow doc polished/consistent.Also applies to: 95-103
CONTRIBUTING.md (1)
31-48: Markdown code fences are broken (opening/closing fences don’t match).
You’re closing many blocks withtext instead of a plain; that will break rendering for everything after the first occurrence.@@ ```bash @@ -```text +``` @@ -```text +``` @@ ```bash @@ -```text +``` @@ ```bash @@ -```text +``` @@ ```python @@ -```text +``` @@ ```bash @@ -```text +``` @@ ```python @@ -```text +``` @@ ```text @@ -```text +``` @@ ```bash @@ -```text +```Also applies to: 51-66, 71-81, 103-118, 158-181, 216-233, 236-257, 264-271, 286-309
README.md (2)
12-21: Remove stale PyPI publish badge (workflow deleted) and add Artifact Registry publish badge.
Line 20 still referencespublish-pypi.yml, which this PR deletes.[](https://github.com/ByronWilliamsCPA/python_libs/actions/workflows/sbom.yml?query=branch%3Amain) [](https://github.com/ByronWilliamsCPA/python_libs/actions/workflows/pr-validation.yml) [](https://github.com/ByronWilliamsCPA/python_libs/actions/workflows/release.yml) -[](https://github.com/ByronWilliamsCPA/python_libs/actions/workflows/publish-pypi.yml) +[](https://github.com/ByronWilliamsCPA/python_libs/actions/workflows/publish-artifact-registry.yml)
641-659: Remove/adjust “Publishes to PyPI” from semantic-release description.
Line 658 still says “Publishes to PyPI (if configured)”, but this PR explicitly removes PyPI publishing.- - Publishes to PyPI (if configured) + - (Publishing handled separately via tag-triggered Artifact Registry workflow)packages/gcs-utilities/src/gcs_utilities/client.py (4)
119-128: Env var mismatch vs required secret (GCP_SA_KEY_BASE64): support both names (or align everywhere).- b64_key = service_account_key_b64 or os.getenv("GCP_SA_KEY") + b64_key = ( + service_account_key_b64 + or os.getenv("GCP_SA_KEY_BASE64") + or os.getenv("GCP_SA_KEY") + )
341-366: Fixupload_directorypath join: empty prefix currently creates leading “/” blob names.
Whengcs_prefix == "", Line 364 produces"/{rel_path}", bypassing the no-leading-slash convention used elsewhere.- gcs_path = f"{gcs_prefix.rstrip('/')}/{rel_path}" + if gcs_prefix: + gcs_path = f"{gcs_prefix.rstrip('/')}/{rel_path}" + else: + gcs_path = str(rel_path)
165-196: Avoid catchingExceptionin_get_or_create_bucket(can hide programmer errors).
Prefer catching expected GCS/IO errors and let unexpected exceptions surface.- except Exception as e: + except (GoogleCloudError, OSError, ValueError) as e: if isinstance(e, GCSNotFoundError): raise msg = f"Failed to access bucket '{self.bucket_name}': {e}" raise GCSAuthError(msg) from e
129-164: Use in-memory credentials instead of writing service-account JSON to disk.
Callservice_account.Credentials.from_service_account_info(sa_data)on the decoded JSON dict and pass the credentials object tostorage.Client(credentials=...). This avoids the temporary file, eliminates theGOOGLE_APPLICATION_CREDENTIALSenvironment variable, and removes cleanup logic while maintaining the same functionality.from google.oauth2 import service_account # After loading sa_data from base64 JSON: creds = service_account.Credentials.from_service_account_info( sa_data, scopes=["https://www.googleapis.com/auth/cloud-platform"] ) self.project_id = sa_data.get("project_id") or os.getenv("GCP_PROJECT") self.client = storage.Client(project=self.project_id, credentials=creds)packages/cloudflare-auth/src/cloudflare_auth/csrf.py (1)
72-94: Remove unusedsecrets.token_bytes(32)call (dead code).- # Generate random token - secrets.token_bytes(32) - # Optionally bind to session ID if session_id:packages/cloudflare-auth/src/cloudflare_auth/security_helpers.py (2)
125-169: Background cleanup task can die permanently on first error; handle exceptions per-iteration and tighten task typing.Right now, any unexpected exception during
cleanup_expired_sessions()(or evenasyncio.sleep) will hit the outerexcept Exceptionand end the task silently after logging once.def create_session_cleanup_task( session_manager: SimpleSessionManager, cleanup_interval: int = 300, -) -> asyncio.Task: +) -> asyncio.Task[None]: @@ async def cleanup_loop() -> None: @@ - try: - while True: - await asyncio.sleep(cleanup_interval) - count = session_manager.cleanup_expired_sessions() - if count > 0: - logger.info("Cleaned up %d expired sessions", count) - except asyncio.CancelledError: - logger.info("Session cleanup task cancelled") - raise - except Exception as e: - logger.error("Session cleanup error: %s", e, exc_info=True) + while True: + try: + await asyncio.sleep(cleanup_interval) + count = session_manager.cleanup_expired_sessions() + if count > 0: + logger.info("Cleaned up %d expired sessions", count) + except asyncio.CancelledError: + logger.info("Session cleanup task cancelled") + raise + except Exception: + logger.exception("Session cleanup error") return asyncio.create_task(cleanup_loop())
219-332: AuditLogger: avoid logging raw emails/IPs; sanitize or constrain to trusted sources.If
admin_email,user_email,ip_address, etc. can be user-controlled (directly or indirectly), logging them raw risks log-forging (newlines/control chars) and leaking PII. Prefer sanitizing (or storing only hashed/partial forms) before interpolating into log messages andextra.packages/cloudflare-auth/src/cloudflare_auth/whitelist.py (3)
25-38: Email-validator optional import is fine; avoidEmailNotValidError = Noneif you can keep types sound.If you want to keep
EmailNotValidErrortyped, consider importing it only for typing or guarding exception handling without reassigning the symbol (reassignment forces moretype: ignoredownstream).
426-482: SonarCloud failure:validate_whitelist_configmust be refactored (cognitive complexity).The PR currently fails Sonar at Line 426. The
# noqawon’t fix the Sonar quality gate—this needs decomposition into smaller helpers (each check returnslist[str]or appends towarnings).Example direction (sketch):
_warn_if_empty_whitelist()_warn_if_tier_email_not_in_whitelist()_warn_if_tier_conflicts()_warn_if_public_domains_allowed()This should bring complexity under the threshold and remove the Sonar blocker.
505-587: SonarCloud failure + maintainability:WhitelistManager.add_emailneeds decomposition; also guard against duplicates.Sonar fails at Line 505, and the method currently does: input validation → normalization → email/domain validation → collection updates → admin mutation → logging. Split into helpers (e.g.,
_validate_entry,_normalize_validated_email,_apply_entry) and add duplicate checks (e.g., avoid appending the same admin email repeatedly).This should both (1) unblock Sonar and (2) reduce subtle drift between
admin_emails(list) and thesetcollections.packages/cloudflare-auth/src/cloudflare_auth/middleware_enhanced.py (3)
290-303: CRITICAL:self._get_client_ip(request)is undefined (runtime error) and logging misses sanitization.This will crash on oversized JWTs.
if len(jwt_token) > 8192: # 8KB limit + client_ip = get_client_ip(request) logger.warning( "JWT token too large: %d bytes (path: %s, ip: %s)", len(jwt_token), - request.url.path, - self._get_client_ip(request), + sanitize_path(request.url.path), + sanitize_ip(client_ip), )
384-417: Session-based claims construction can fail ifself.settings.issuerisNone; make it a safe string.
CloudflareJWTClaims.issis a requiredstr. If config is partially set (orcloudflare_team_domainis empty),self.settings.issuercan beNone, which can trigger a pydantic validation error when authenticating from session.claims = CloudflareJWTClaims( email=session["email"], - iss=self.settings.issuer, + iss=self.settings.issuer or "session", aud=[self.settings.cloudflare_audience_tag], sub=session.get("email", ""), iat=int(session["created_at"].timestamp()), exp=int(session["last_accessed"].timestamp()) + self.session_manager.session_timeout, )
458-573: CRITICAL:setup_cloudflare_auth_enhancedreturn type is wrong (returnsNonebut annotated otherwise).Either return the middleware instance (not really possible with
add_middleware) or change the annotation/doc to-> None.-def setup_cloudflare_auth_enhanced( +def setup_cloudflare_auth_enhanced( app: Any, @@ -) -> CloudflareAuthMiddlewareEnhanced: +) -> None: @@ - return None # Middleware is added directly to app + return None # Middleware is added directly to appAlso adjust the docstring “Returns:” section to match.
packages/cloudflare-auth/src/cloudflare_auth/middleware.py (3)
26-28: Stale import paths in docstring.The Dependencies section still references
src.cloudflare_auth.*andsrc.config.settings, but the actual imports and examples now usecloudflare_auth.*. Update for consistency.Dependencies: - fastapi: For Request/Response handling - starlette: For BaseHTTPMiddleware - - src.cloudflare_auth.validators: For JWT validation - - src.cloudflare_auth.models: For user models - - src.config.settings: For configuration + - cloudflare_auth.validators: For JWT validation + - cloudflare_auth.models: For user models + - cloudflare_auth.config: For configuration
250-256: Missingrequest.state.userwhen Cloudflare disabled withrequire_auth=True.When
cloudflare_enabled=Falseandrequire_auth=True, this code path returns early without settingrequest.state.user, potentially causingAttributeErrorin downstream handlers expecting the user attribute.# Skip authentication if disabled (development mode) if not self.settings.cloudflare_enabled: logger.debug("Cloudflare authentication disabled") - # Set a mock user for development - if not self.require_auth: - request.state.user = None + # Set user to None for development mode + request.state.user = None return await call_next(request)
361-391: Rate limiting not applied to all authentication failure paths.Failed attempts are only recorded for
ValueErrorfrom JWT validation (line 405), but authentication failures from missing email header (line 373) or email mismatch (line 388) don't triggerrecord_attempt(). An attacker could probe these failure paths without rate limiting.# CRITICAL SECURITY: Email header must be present when behind Cloudflare if not email_header: logger.error( "SECURITY: Missing required Cloudflare email header: %s (path: %s, ip: %s)", self.settings.email_header_name, sanitize_path(request.url.path), sanitize_ip(get_client_ip(request)), ) + if self.enable_rate_limiting and self.rate_limiter: + self.rate_limiter.record_attempt(get_client_ip(request)) raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, detail="Authentication verification failed", ) # Validate email header matches JWT email if email_header != user.email: logger.error( "SECURITY: Email mismatch detected - potential token manipulation: " "JWT=%s, Header=%s, IP=%s", sanitize_email(user.email), sanitize_email(email_header), sanitize_ip(get_client_ip(request)), ) + if self.enable_rate_limiting and self.rate_limiter: + self.rate_limiter.record_attempt(get_client_ip(request)) # Always fail on mismatch - this is a security issue raise HTTPException(.claude/commands/pr.md (1)
45-53: Fix step numbering inconsistency.Steps are renumbered from 4-5 to 1-2, creating a logical inconsistency. The instructions read: steps 1-3, then restart at steps 1-2. This should be renumbered as steps 4-5 to maintain sequential flow.
Apply this diff to fix the numbering:
-1. **Output the PR description** ready to copy-paste into GitHub. +4. **Output the PR description** ready to copy-paste into GitHub. -2. **Suggest a PR title** following conventional commits: +5. **Suggest a PR title** following conventional commits:
♻️ Duplicate comments (2)
.qlty/out (1)
1-1: Remove accidentally committed cache artifact.This is another cache artifact file that should not be in version control for the same reasons as
.qlty/logs..qlty/results (1)
1-1: Remove accidentally committed cache artifact.This is another cache artifact file that should not be in version control for the same reasons as
.qlty/logs.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (100)
.claude/README.md(2 hunks).claude/agents/code-reviewer.md(2 hunks).claude/agents/merge-standards.md(2 hunks).claude/agents/security-auditor.md(4 hunks).claude/agents/test-engineer.md(4 hunks).claude/commands/merge-standards.md(1 hunks).claude/commands/plan.md(3 hunks).claude/commands/pr.md(2 hunks).claude/commands/quality.md(3 hunks).claude/commands/security.md(6 hunks).claude/commands/testing.md(2 hunks).claude/context/python-standards.md(7 hunks).claude/context/testing-patterns.md(8 hunks).claude/skills/commit-prepare/SKILL.md(9 hunks).claude/skills/git/SKILL.md(1 hunks).claude/skills/pr-prepare/SKILL.md(5 hunks).claude/skills/project-planning/SKILL.md(11 hunks).claude/skills/project-planning/reference/document-guide.md(6 hunks).claude/skills/project-planning/reference/prompting-patterns.md(11 hunks).claude/skills/project-planning/templates/adr-template.md(1 hunks).claude/skills/project-planning/templates/roadmap-template.md(2 hunks).claude/skills/project-planning/templates/tech-spec-template.md(4 hunks).claude/skills/project-planning/workflows/synthesize.md(13 hunks).claude/skills/quality/SKILL.md(3 hunks).claude/skills/security/SKILL.md(2 hunks).claude/skills/testing/SKILL.md(4 hunks).cruft.json(2 hunks).github/workflows/README.md(9 hunks).github/workflows/ci.yml(3 hunks).github/workflows/docs.yml(1 hunks).github/workflows/fips-compatibility.yml(3 hunks).github/workflows/pr-validation.yml(1 hunks).github/workflows/publish-artifact-registry.yml(1 hunks).github/workflows/publish-pypi.yml(0 hunks).github/workflows/python-compatibility.yml(1 hunks).github/workflows/release.yml(4 hunks).github/workflows/scorecard.yml(0 hunks).github/workflows/security-analysis.yml(1 hunks).github/workflows/slsa-provenance.yml(1 hunks).markdownlint.json(1 hunks).pre-commit-config.yaml(1 hunks).qlty/logs(1 hunks).qlty/out(1 hunks).qlty/plugin_cachedir(1 hunks).qlty/qlty.toml(3 hunks).qlty/results(1 hunks).standards/CLAUDE.baseline.md(2 hunks).standards/README.baseline.md(1 hunks).standards/README.md(2 hunks).standards/template_feedback.baseline.md(1 hunks).yamllint(1 hunks)CHANGELOG.md(3 hunks)CLAUDE.md(24 hunks)CONFIG_TEMPLATES_SUMMARY.md(10 hunks)CONTRIBUTING.md(11 hunks)LICENSES/Apache-2.0.txt(0 hunks)LICENSES/BSD-3-Clause.txt(0 hunks)LICENSES/GPL-3.0.txt(0 hunks)README.md(20 hunks)REUSE.toml(5 hunks)docs/ADRs/adr-template.md(3 hunks)docs/OPENSSF_COMPLIANCE.md(1 hunks)docs/PROJECT_SETUP.md(23 hunks)docs/PYTHON_COMPATIBILITY.md(3 hunks)docs/api-reference.md(0 hunks)docs/development/architecture.md(2 hunks)docs/development/code-quality.md(1 hunks)docs/development/contributing.md(1 hunks)docs/development/testing.md(2 hunks)docs/diagrams/publish-workflow.puml(1 hunks)docs/index.md(1 hunks)docs/planning/README.md(3 hunks)docs/planning/project-plan-template.md(15 hunks)docs/planning/project-vision.md(1 hunks)docs/planning/roadmap.md(1 hunks)docs/planning/tech-spec.md(1 hunks)docs/project/license.md(2 hunks)docs/template_feedback.md(1 hunks)mkdocs.yml(0 hunks)packages/cloudflare-auth/src/cloudflare_auth/__init__.py(3 hunks)packages/cloudflare-auth/src/cloudflare_auth/config.py(1 hunks)packages/cloudflare-auth/src/cloudflare_auth/csrf.py(8 hunks)packages/cloudflare-auth/src/cloudflare_auth/middleware.py(10 hunks)packages/cloudflare-auth/src/cloudflare_auth/middleware_enhanced.py(8 hunks)packages/cloudflare-auth/src/cloudflare_auth/models.py(6 hunks)packages/cloudflare-auth/src/cloudflare_auth/rate_limiter.py(11 hunks)packages/cloudflare-auth/src/cloudflare_auth/redis_sessions.py(6 hunks)packages/cloudflare-auth/src/cloudflare_auth/security_helpers.py(6 hunks)packages/cloudflare-auth/src/cloudflare_auth/sessions.py(8 hunks)packages/cloudflare-auth/src/cloudflare_auth/utils.py(10 hunks)packages/cloudflare-auth/src/cloudflare_auth/validators.py(7 hunks)packages/cloudflare-auth/src/cloudflare_auth/whitelist.py(14 hunks)packages/cloudflare-auth/tests/test_models.py(1 hunks)packages/gcs-utilities/src/gcs_utilities/__init__.py(1 hunks)packages/gcs-utilities/src/gcs_utilities/client.py(19 hunks)packages/gcs-utilities/src/gcs_utilities/exceptions.py(0 hunks)packages/gcs-utilities/tests/conftest.py(1 hunks)packages/gcs-utilities/tests/test_exceptions.py(1 hunks)pyproject.toml(2 hunks)scripts/README.md(1 hunks)
💤 Files with no reviewable changes (8)
- LICENSES/Apache-2.0.txt
- LICENSES/GPL-3.0.txt
- .github/workflows/scorecard.yml
- LICENSES/BSD-3-Clause.txt
- .github/workflows/publish-pypi.yml
- docs/api-reference.md
- mkdocs.yml
- packages/gcs-utilities/src/gcs_utilities/exceptions.py
🧰 Additional context used
📓 Path-based instructions (2)
.github/workflows/**
⚙️ CodeRabbit configuration file
.github/workflows/**: Review GitHub Actions workflows for:
- Security best practices (minimal permissions, pinned actions)
- Proper secret handling
- Efficient caching strategies
- Clear job dependencies
Files:
.github/workflows/python-compatibility.yml.github/workflows/README.md.github/workflows/pr-validation.yml.github/workflows/slsa-provenance.yml.github/workflows/release.yml.github/workflows/ci.yml.github/workflows/docs.yml.github/workflows/fips-compatibility.yml.github/workflows/security-analysis.yml.github/workflows/publish-artifact-registry.yml
pyproject.toml
⚙️ CodeRabbit configuration file
pyproject.toml: Review dependency changes for:
- Version constraint appropriateness
- Security implications of new dependencies
- License compatibility
Files:
pyproject.toml
🧬 Code graph analysis (8)
packages/cloudflare-auth/src/cloudflare_auth/middleware_enhanced.py (8)
packages/cloudflare-auth/src/cloudflare_auth/models.py (2)
CloudflareUser(108-265)CloudflareJWTClaims(27-105)packages/cloudflare-auth/src/cloudflare_auth/rate_limiter.py (1)
InMemoryRateLimiter(32-248)packages/cloudflare-auth/src/cloudflare_auth/sessions.py (1)
SimpleSessionManager(28-319)packages/cloudflare-auth/src/cloudflare_auth/utils.py (4)
get_client_ip(235-265)sanitize_email(83-107)sanitize_ip(129-157)sanitize_path(110-126)packages/cloudflare-auth/src/cloudflare_auth/validators.py (1)
CloudflareJWTValidator(43-309)packages/cloudflare-auth/src/cloudflare_auth/whitelist.py (2)
EmailWhitelistValidator(150-482)UserTier(43-92)packages/cloudflare-auth/src/cloudflare_auth/config.py (2)
CloudflareSettings(28-183)get_cloudflare_settings(187-202)packages/cloudflare-auth/src/cloudflare_auth/middleware.py (1)
_authenticate_request(285-422)
packages/cloudflare-auth/src/cloudflare_auth/validators.py (2)
packages/cloudflare-auth/src/cloudflare_auth/models.py (1)
CloudflareJWTClaims(27-105)packages/cloudflare-auth/src/cloudflare_auth/config.py (3)
CloudflareSettings(28-183)get_cloudflare_settings(187-202)is_email_allowed(167-183)
packages/cloudflare-auth/src/cloudflare_auth/security_helpers.py (1)
packages/cloudflare-auth/src/cloudflare_auth/sessions.py (1)
SimpleSessionManager(28-319)
packages/cloudflare-auth/src/cloudflare_auth/config.py (1)
packages/cloudflare-auth/src/cloudflare_auth/models.py (1)
email_domain(199-205)
packages/cloudflare-auth/src/cloudflare_auth/models.py (1)
packages/cloudflare-auth/src/cloudflare_auth/whitelist.py (2)
UserTier(43-92)is_admin(282-304)
packages/gcs-utilities/tests/test_exceptions.py (1)
packages/cloudflare-auth/tests/test_models.py (1)
test_placeholder(7-10)
packages/cloudflare-auth/src/cloudflare_auth/middleware.py (6)
packages/cloudflare-auth/src/cloudflare_auth/models.py (1)
CloudflareUser(108-265)packages/cloudflare-auth/src/cloudflare_auth/rate_limiter.py (1)
InMemoryRateLimiter(32-248)packages/cloudflare-auth/src/cloudflare_auth/utils.py (4)
get_client_ip(235-265)sanitize_email(83-107)sanitize_ip(129-157)sanitize_path(110-126)packages/cloudflare-auth/src/cloudflare_auth/validators.py (1)
CloudflareJWTValidator(43-309)packages/cloudflare-auth/src/cloudflare_auth/config.py (2)
CloudflareSettings(28-183)get_cloudflare_settings(187-202)packages/cloudflare-auth/src/cloudflare_auth/middleware_enhanced.py (1)
_authenticate_request(230-382)
packages/cloudflare-auth/src/cloudflare_auth/__init__.py (7)
packages/cloudflare-auth/src/cloudflare_auth/middleware.py (3)
CloudflareAuthMiddleware(69-422)get_current_user(493-529)get_current_user_optional(532-554)packages/cloudflare-auth/src/cloudflare_auth/middleware_enhanced.py (6)
get_current_user(576-602)get_current_user_optional(605-621)CloudflareAuthMiddlewareEnhanced(62-455)require_admin(624-649)require_tier(652-686)setup_cloudflare_auth_enhanced(458-572)packages/cloudflare-auth/src/cloudflare_auth/models.py (2)
CloudflareJWTClaims(27-105)CloudflareUser(108-265)packages/cloudflare-auth/src/cloudflare_auth/security_helpers.py (4)
AuditLogger(171-332)SecurityHeadersMiddleware(30-122)create_session_cleanup_task(125-168)get_audit_logger(335-343)packages/cloudflare-auth/src/cloudflare_auth/sessions.py (1)
SimpleSessionManager(28-319)packages/cloudflare-auth/src/cloudflare_auth/validators.py (1)
CloudflareJWTValidator(43-309)packages/cloudflare-auth/src/cloudflare_auth/redis_sessions.py (1)
RedisSessionManager(54-360)
🪛 GitHub Check: CodeQL
.github/workflows/publish-artifact-registry.yml
[warning] 152-152: Unpinned tag for a non-immutable Action in workflow
Unpinned 3rd party Action 'Publish to Artifact Registry' step Uses Step uses 'google-github-actions/auth' with ref 'v2', not a pinned commit hash
[warning] 157-157: Unpinned tag for a non-immutable Action in workflow
Unpinned 3rd party Action 'Publish to Artifact Registry' step Uses Step uses 'google-github-actions/setup-gcloud' with ref 'v2', not a pinned commit hash
[warning] 173-173: Unpinned tag for a non-immutable Action in workflow
Unpinned 3rd party Action 'Publish to Artifact Registry' step Uses Step uses 'astral-sh/setup-uv' with ref 'v4', not a pinned commit hash
🪛 GitHub Check: SonarCloud Code Analysis
packages/cloudflare-auth/src/cloudflare_auth/middleware_enhanced.py
[warning] 230-230: Use asynchronous features in this function or remove the async keyword.
[failure] 230-230: Refactor this function to reduce its Cognitive Complexity from 37 to the 15 allowed.
.github/workflows/publish-artifact-registry.yml
[failure] 89-89: Change this workflow to not use user-controlled data directly in a run block.
[warning] 140-140: Complete the task associated to this "TODO" comment.
packages/cloudflare-auth/src/cloudflare_auth/whitelist.py
[failure] 426-426: Refactor this function to reduce its Cognitive Complexity from 22 to the 15 allowed.
[failure] 505-505: Refactor this function to reduce its Cognitive Complexity from 32 to the 15 allowed.
packages/cloudflare-auth/src/cloudflare_auth/middleware.py
[warning] 285-285: Use asynchronous features in this function or remove the async keyword.
[failure] 285-285: Refactor this function to reduce its Cognitive Complexity from 23 to the 15 allowed.
🪛 LanguageTool
.claude/agents/security-auditor.md
[style] ~62-~62: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ... TLS/SSL properly configured - [ ] CORS properly restricted ### Authentication & Author...
(ADVERB_REPETITION_PREMIUM)
|
|
||
| - **branch.md**: Branch creation, validation, and naming conventions | ||
| - **status.md**: Repository status and health checks | ||
|
|
||
| ### Commit Management | ||
|
|
||
| - **commit.md**: Conventional commit message preparation | ||
|
|
||
| ### PR Workflow | ||
|
|
||
| - **pr-prepare.md**: Pull request description generation | ||
| - **pr-check.md**: PR validation and checklist |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all referenced git workflow files exist
WORKFLOW_DIR=".claude/skills/git"
WORKFLOWS=("branch.md" "status.md" "commit.md" "pr-prepare.md" "pr-check.md")
echo "Checking workflow files in $WORKFLOW_DIR..."
for workflow in "${WORKFLOWS[@]}"; do
if [ -f "$WORKFLOW_DIR/$workflow" ]; then
echo "✓ $workflow exists"
else
echo "✗ $workflow NOT FOUND"
fi
done
echo ""
echo "All files in $WORKFLOW_DIR:"
ls -la "$WORKFLOW_DIR/"Repository: ByronWilliamsCPA/python-libs
Length of output: 439
All referenced workflow files are missing from the repository.
The added documentation references point to files that do not exist in .claude/skills/git/: branch.md, status.md, commit.md, pr-prepare.md, and pr-check.md. These workflow documentation files must be created before merging this change, or the references should be removed if these workflows are not yet implemented.
🤖 Prompt for AI Agents
In .claude/skills/git/SKILL.md around lines 12 to 23, the document references
workflow files (branch.md, status.md, commit.md, pr-prepare.md, pr-check.md)
that do not exist; either create these missing files under .claude/skills/git/
with the expected content (branching rules, status checks, commit conventions,
PR preparation, and PR checklist) or remove/update the references in SKILL.md to
only list files that actually exist; ensure new files follow repository naming
and include at least a short description and link back to SKILL.md so the
references are valid.
| ```text | ||
|
|
||
| Phase 0: Foundation ████████░░░░░░░░ (1 week) - Setup | ||
| Phase 1: MVP Core ░░░░░░░░████████ (3 weeks) - Core features | ||
| Phase 2: Enhancement ░░░░░░░░░░░░░░░░ (2 weeks) - Additional features | ||
| Phase 3: Polish ░░░░░░░░░░░░░░░░ (1 week) - Testing & docs | ||
| ``` | ||
|
|
||
| ```text | ||
|
|
There was a problem hiding this comment.
Fix the Timeline Overview fence closing (breaks Markdown).
```text
-
Phase 0: Foundation ████████░░░░░░░░ (1 week) - Setup
Phase 1: MVP Core ░░░░░░░░████████ (3 weeks) - Core features
Phase 2: Enhancement ░░░░░░░░░░░░░░░░ (2 weeks) - Additional features
Phase 3: Polish ░░░░░░░░░░░░░░░░ (1 week) - Testing & docs
-
-```text
+```🤖 Prompt for AI Agents
.claude/skills/project-planning/templates/roadmap-template.md around lines 15 to
23: the Markdown code block fence is closed incorrectly with "```text" instead
of a plain closing "```", which breaks rendering; update the closing fence to
exactly three backticks (```) so the block starts with "```text" and ends with
"```", preserving the existing timeline lines unchanged.
|
|
||
| - **validate-env.md**: GPG/SSH key validation | ||
|
|
||
| ### Scanning | ||
|
|
||
| - **scan.md**: Security vulnerability scanning | ||
|
|
||
| ### Encryption | ||
|
|
||
| - **encrypt.md**: Secret encryption and management | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify security workflow files referenced in SKILL.md
SECURITY_DIR=".claude/skills/security"
WORKFLOWS=("validate-env.md" "scan.md" "encrypt.md")
echo "Verifying security workflow files..."
for workflow in "${WORKFLOWS[@]}"; do
if [ -f "$SECURITY_DIR/$workflow" ]; then
echo "✓ $workflow exists"
else
echo "✗ $workflow NOT FOUND"
fi
done
# Also check what files actually exist in the directory
echo ""
echo "Files in .claude/skills/security/:"
ls -la "$SECURITY_DIR" 2>/dev/null || echo "Directory not found"Repository: ByronWilliamsCPA/python-libs
Length of output: 384
Remove references to non-existent workflow files.
The added workflow references (validate-env.md, scan.md, encrypt.md) do not exist in .claude/skills/security/. The directory contains only SKILL.md. Either create these referenced files or remove the references from the documentation to avoid broken links.
🤖 Prompt for AI Agents
In .claude/skills/security/SKILL.md around lines 12 to 22, the document
references non-existent files validate-env.md, scan.md, and encrypt.md; remove
those broken links or add the missing files. Fix by either deleting the three
bullet lines (or replacing them with correct existing filenames) so SKILL.md
only links to real docs, or create the referenced markdown files under
.claude/skills/security/ with the described content (GPG/SSH validation,
vulnerability scanning, and secret encryption) and commit them so the links
resolve.
| - name: Install uv | ||
| uses: astral-sh/setup-uv@582b2d78a0f5913301dcc87c4e93301fdd2b6711 # v4.1.1 | ||
| uses: astral-sh/setup-uv@e4db8464a088ece1b920f60402e813ea4de65b8f # v4 | ||
| with: | ||
| enable-cache: true | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
astral-sh/setup-uv v4 release commit SHA e4db8464a088ece1b920f60402e813ea4de65b8f
💡 Result:
Do you want me to (pick one)
- verify that commit SHA e4db8464a088ece1b920f60402e813ea4de65b8f exists in the astral-sh/setup-uv repo and show its details, or
- confirm which release/tag (v4) that SHA corresponds to?
🌐 Web query:
site:github.com/astral-sh/setup-uv commit e4db8464a088ece1b920f60402e813ea4de65b8f
💡 Result:
I couldn't find that commit on the repository — accessing the commit URL returns 404 and the repository's commit history/search shows no match for e4db8464a088ece1b920f60402e813ea4de65b8f. See the repo and the failed commit lookup. [1][2]
Sources:
[1] astral-sh/setup-uv repository (README).
[2] 404 response for the commit URL (commit not found).
🌐 Web query:
site:github.com/astral-sh/setup-uv releases v4 tag
💡 Result:
I couldn't find a v4 release/tag for astral-sh/setup-uv. The repository's Releases page shows v6 and v7 series (latest v7.1.5). [1][2]
Sources:
- Releases: astral-sh/setup-uv (GitHub). [1]
- Repository overview / tags: astral-sh/setup-uv (GitHub). [2]
Critical: The pinned SHA e4db8464a088ece1b920f60402e813ea4de65b8f does not exist in the astral-sh/setup-uv repository (404), and the v4 release does not exist. The latest releases are v6 and v7 series (v7.1.5 is current). Update the action reference to a valid, existing commit SHA from an available release version.
Also applies to: 119-123, 161-165
🤖 Prompt for AI Agents
In .github/workflows/ci.yml around lines 73-77 (and likewise for lines 119-123
and 161-165), the workflow pins astral-sh/setup-uv to a non-existent commit SHA
and an invalid v4 release; replace the invalid reference with a valid action
release/commit from the repository (for example use the existing v7 series like
v7.1.5 or a concrete commit SHA from that release), updating all three
occurrences so each uses the same valid tag or SHA and enable-cache remains set
as before.
| def test_placeholder(self) -> None: | ||
| """Placeholder test - replace with actual tests.""" | ||
| # TODO: Add actual exception tests once dependencies are resolved | ||
| assert True |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
-> None test annotation is fine.
Consider tracking the TODO to replace the placeholder with real exception tests.
🤖 Prompt for AI Agents
In packages/gcs-utilities/tests/test_exceptions.py around lines 7 to 10, the
test is a placeholder with a TODO comment; replace it with real exception tests
and ensure the TODO is tracked: either implement concrete pytest tests for the
expected exception behavior now, or create a tracking issue/PR reference and
update the TODO to include that issue number (or mark the test with
pytest.skip/xfail and a linked issue number) so the missing tests are visible
and traceable.
| # Cloudflare auth middleware - Complex authentication logic | ||
| "packages/cloudflare-auth/src/cloudflare_auth/middleware*.py" = [ | ||
| "TRY300", # Return in try block is clearer for auth flow | ||
| "TRY301", # Raising in try block is standard for auth middleware | ||
| "PLC0415", # Local imports to avoid circular dependencies | ||
| ] | ||
|
|
||
| # Cloudflare auth whitelist - Validation logic | ||
| "packages/cloudflare-auth/src/cloudflare_auth/whitelist.py" = [ | ||
| "TRY300", # Return in try block is clearer | ||
| "TRY301", # Raising in try block is standard | ||
| "TRY401", # Exception in log is intentional for audit | ||
| "SLF001", # Internal access to validator internals is intentional | ||
| "PGH003", # Generic type ignore is acceptable for optional import | ||
| "PERF401", # List comprehension vs append is clearer here | ||
| ] | ||
|
|
||
| # Cloudflare auth redis sessions - Session management logic | ||
| "packages/cloudflare-auth/src/cloudflare_auth/redis_sessions.py" = [ | ||
| "TRY300", # Return in try block is clearer for session flow | ||
| "TRY401", # Exception in log message is intentional | ||
| ] | ||
|
|
||
| # Cloudflare auth security helpers - Security patterns | ||
| "packages/cloudflare-auth/src/cloudflare_auth/security_helpers.py" = [ | ||
| "G201", # Using .error with exc_info is intentional for security logging | ||
| "TRY300", # Return patterns in try blocks | ||
| "SLF001", # Function attribute pattern for singleton | ||
| ] | ||
|
|
||
| # Cloudflare auth validators - JWT validation logic | ||
| "packages/cloudflare-auth/src/cloudflare_auth/validators.py" = [ | ||
| "TRY300", # Return in try block is clearer for validation | ||
| "TRY301", # Raising in try block is standard for validation | ||
| "TRY401", # Exception in log is intentional | ||
| ] | ||
|
|
||
| # Cloudflare auth utils - Utility functions | ||
| "packages/cloudflare-auth/src/cloudflare_auth/utils.py" = [ | ||
| "BLE001", # Blind except is intentional for resilient parsing | ||
| "ANN001", # Regex match type annotation not needed | ||
| "ANN202", # Private function return type annotation not needed | ||
| ] | ||
|
|
||
| # Cloudflare auth tests - Test patterns | ||
| "packages/cloudflare-auth/tests/*.py" = [ | ||
| "ANN201", # Return type annotations not needed for fixtures | ||
| "PERF401", # List comprehension vs extend is fine in tests | ||
| ] | ||
|
|
||
| # GCS utilities client - Storage operations | ||
| "packages/gcs-utilities/src/gcs_utilities/client.py" = [ | ||
| "G004", # F-string logging is cleaner for GCS operations | ||
| "TRY300", # Return in try block is clearer for storage ops | ||
| "TRY301", # Raising in try block is standard for storage errors | ||
| "TRY401", # Exception in log is intentional | ||
| "PTH101", # os.chmod preferred for credential file permissions | ||
| "PTH108", # os.unlink preferred for credential file cleanup | ||
| "PTH110", # os.path.exists preferred for credential file check | ||
| "PERF401", # List comprehension vs append is clearer here | ||
| ] |
There was a problem hiding this comment.
Per-file ignores look appropriately scoped, but a couple ignore rationales are confusing/possibly inverted.
- The entries are nicely targeted to specific modules (good).
- However, the comments for
PTH101/PTH108/PTH110underpackages/gcs-utilities/src/gcs_utilities/client.pyread as “os.* preferred”, whilePTH*rules are about preferring pathlib. If the intent is “we intentionally useos.*here”, consider rewording to avoid future confusion.
Suggested comment-only tweak:
"packages/gcs-utilities/src/gcs_utilities/client.py" = [
"G004",
"TRY300",
"TRY301",
"TRY401",
- "PTH101", # os.chmod preferred for credential file permissions
- "PTH108", # os.unlink preferred for credential file cleanup
- "PTH110", # os.path.exists preferred for credential file check
+ "PTH101", # Intentional: use os.* APIs here (pathlib not used for these operations)
+ "PTH108", # Intentional: use os.* APIs here (pathlib not used for these operations)
+ "PTH110", # Intentional: use os.* APIs here (pathlib not used for these operations)
"PERF401",
]🤖 Prompt for AI Agents
In pyproject.toml around lines 389 to 449, the comments explaining ignores for
PTH101/PTH108/PTH110 under packages/gcs-utilities/src/gcs_utilities/client.py
are misleading because the PTH* rules prefer pathlib while the current comments
read as “os.* preferred”; update those three comment strings to clearly state
that the code intentionally uses os.* functions instead of pathlib (e.g.,
“Intentional use of os.* instead of pathlib for credential file operations”) so
future readers understand the rationale without implying the lint rule is being
followed.
| ## Publishing | ||
|
|
||
| Packages are published to **GCP Artifact Registry** (not PyPI) for enhanced supply chain security. This integrates with Google Assured OSS for verified dependencies. | ||
|
|
||
| ### Publishing Workflow | ||
|
|
||
| The publishing process uses GitHub Actions triggered by version tags, with secrets managed securely via Infisical: | ||
|
|
||
| ```text | ||
| Developer → Push Tag → GitHub Actions → Infisical → GCP Auth → Artifact Registry | ||
| ```text | ||
|
|
||
| <details> | ||
| <summary><b>View PlantUML Sequence Diagram</b></summary> | ||
|
|
||
| ```plantuml | ||
| @startuml publish-workflow | ||
| !theme plain | ||
| skinparam backgroundColor #FEFEFE | ||
| skinparam sequenceMessageAlign center | ||
|
|
||
| title Package Publishing Workflow\nGCP Artifact Registry with Infisical Secrets | ||
|
|
||
| actor Developer | ||
| participant "GitHub\nRepository" as GitHub | ||
| participant "GitHub\nActions" as GHA | ||
| participant "Infisical\nSecrets" as Infisical | ||
| participant "Google Cloud\nAuth" as GCP | ||
| participant "Artifact\nRegistry" as AR | ||
|
|
||
| == Tag Creation == | ||
| Developer -> GitHub: Push version tag\n(e.g., cloudflare-auth-v1.0.0) | ||
| activate GitHub | ||
|
|
||
| GitHub -> GHA: Trigger publish workflow | ||
| activate GHA | ||
|
|
||
| == Secret Retrieval == | ||
| GHA -> Infisical: Authenticate with\nClient ID/Secret | ||
| activate Infisical | ||
| Infisical --> GHA: Return GCP_SA_KEY_BASE64 | ||
| deactivate Infisical | ||
|
|
||
| == GCP Authentication == | ||
| GHA -> GCP: Authenticate with\nService Account Key | ||
| activate GCP | ||
| GCP --> GHA: Authentication token | ||
| deactivate GCP | ||
|
|
||
| == Build & Publish == | ||
| GHA -> GHA: Parse tag to determine\npackage directory | ||
| GHA -> GHA: Verify version in\npyproject.toml matches tag | ||
| GHA -> GHA: Build package with UV\n(uv build) | ||
|
|
||
| GHA -> AR: Publish package\n(uv publish) | ||
| activate AR | ||
| AR --> GHA: Publish success | ||
| deactivate AR | ||
|
|
||
| == Summary == | ||
| GHA -> GitHub: Update job summary\nwith publish details | ||
| deactivate GHA | ||
| deactivate GitHub | ||
|
|
||
| note right of AR | ||
| **Registry URL:** | ||
| us-central1-python.pkg.dev/ | ||
| assured-oss-457903/python-libs | ||
|
|
||
| **Supported Tags:** | ||
| - cloudflare-auth-v* | ||
| - cloudflare-api-v* | ||
| - gcs-utilities-v* | ||
| - gemini-image-v* | ||
| end note | ||
|
|
||
| note left of Infisical | ||
| **Secrets Stored:** | ||
| - GCP_SA_KEY_BASE64 | ||
| (Service account JSON, base64) | ||
|
|
||
| **Domain:** | ||
| secrets.byronwilliamscpa.com | ||
| end note | ||
|
|
||
| @enduml | ||
| ```text | ||
|
|
||
| </details> | ||
|
|
||
| See also: [docs/diagrams/publish-workflow.puml](docs/diagrams/publish-workflow.puml) | ||
|
|
||
| ### How to Publish a Package | ||
|
|
||
| 1. **Update version** in the package's `pyproject.toml` | ||
| 2. **Commit and push** the version change | ||
| 3. **Create and push a tag** matching the pattern: | ||
|
|
||
| ```bash | ||
| # Format: {package-name}-v{version} | ||
| git tag cloudflare-auth-v1.0.0 | ||
| git tag cloudflare-api-v1.0.0 | ||
| git tag gcs-utilities-v1.0.0 | ||
| git tag gemini-image-v1.0.0 | ||
|
|
||
| git push origin --tags | ||
| ```text | ||
|
|
||
| 4. **GitHub Actions** automatically: | ||
| - Fetches GCP credentials from Infisical | ||
| - Verifies version matches tag | ||
| - Builds and publishes to Artifact Registry | ||
|
|
||
| ### Registry Details | ||
|
|
||
| | Setting | Value | | ||
| |---------|-------| | ||
| | Registry URL | `us-central1-python.pkg.dev/assured-oss-457903/python-libs` | | ||
| | Secrets Manager | Infisical (secrets.byronwilliamscpa.com) | | ||
| | Service Account | `assured-oss-accessor@assured-oss-457903.iam.gserviceaccount.com` | | ||
|
|
There was a problem hiding this comment.
Publishing docs contradict implementation (Infisical vs GitHub Secrets) and secret name.
PR objectives say secrets are in GitHub Secrets now (and Infisical is future), but Lines 516-593/628-630 describe an Infisical-based flow.
Concrete adjustments I’d expect:
- Replace “GitHub Actions → Infisical …” with “GitHub Actions → GitHub Secrets …” (or mark Infisical explicitly as “future”).
- Ensure the README uses the same secret name as the workflow (
GCP_SA_KEY_BASE64per PR objectives).
🤖 Prompt for AI Agents
In README.md around lines 510 to 630, the publishing docs and PlantUML describe
an Infisical-based secret flow that contradicts the implementation and PR
objectives which use GitHub Secrets and the secret name GCP_SA_KEY_BASE64;
update the text and diagram to reflect "GitHub Actions → GitHub Secrets
(GCP_SA_KEY_BASE64)" or explicitly mark Infisical as a future option, replace
any Infisical mentions in the sequence diagram and notes with GitHub Secrets
(and ensure the note for stored secrets uses the exact name GCP_SA_KEY_BASE64),
and update the short workflow text and example lines (516–593 and 628–630) so
all references and labels consistently match the actual workflow and secret
name.
There was a problem hiding this comment.
Actionable comments posted: 27
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (25)
docs/PROJECT_SETUP.md (2)
300-335: Verify PyPI token references are removed from security configuration.Lines 322-326 document Codecov token setup, and line 413 lists PYPI_API_TOKEN as required. Given the PR removes PyPI publishing, verify that:
- Any Codecov documentation should reference the new Artifact Registry setup
PYPI_API_TOKENreferences should be removed or replaced withGCP_SA_KEY_BASE64This appears related to the major issue flagged above in the CI/CD section.
388-430: Update CI/CD documentation to reflect Artifact Registry publishing instead of PyPI.The PROJECT_SETUP.md section (lines 388-430) documents PyPI publishing, but the PR replaces this with GCP Artifact Registry publishing. Update:
- Remove
Publish to PyPI | publish-pypi.ymlfrom the workflows table (line 402)- Add
Publish to Artifact Registry | publish-artifact-registry.ymlwith purpose: "Publish packages to GCP Artifact Registry on version tags"- Remove
PYPI_API_TOKENfrom required secrets (line 413)- Replace the "PyPI Trusted Publishing" section (lines 418-429) with GCP Artifact Registry configuration including:
- Service account setup requirements
roles/artifactregistry.readerIAM role requirement- Reference to Infisical for credential management
CHANGELOG.md (1)
49-50: Fix repository URL format in comparison links.Lines 49-50 use underscores (
python_libs) but the correct repository name ispython-libs(hyphenated). Update both links to use hyphens:
[Unreleased]: https://github.com/ByronWilliamsCPA/python-libs/compare/v0.1.0...HEAD[0.1.0]: https://github.com/ByronWilliamsCPA/python-libs/releases/tag/v0.1.0.claude/agents/security-auditor.md (1)
57-69: Minor: Repetition of "properly" in configuration checklist.Lines 61-62 use "properly configured" and "properly restricted." Consider varying language for clarity:
- [ ] TLS/SSL properly configured + [ ] TLS/SSL correctly configured - [ ] CORS properly restricted + [ ] CORS properly restrictedThis is a cosmetic improvement; the checklist remains functional as-is.
packages/cloudflare-auth/src/cloudflare_auth/whitelist.py (4)
263-280: Constant-time comparison effectiveness is limited by early exit on match.While
secrets.compare_digestprevents timing attacks on individual comparisons, the loop still exits early when a match is found, leaking whether the email is in the whitelist and approximately where. For authorization checks (boolean result), this is typically acceptable since the result itself reveals authorization status. However, the docstring claim about "constant-time comparison to prevent timing attacks" is somewhat misleading.Consider clarifying the docstring to note that protection is against per-comparison timing attacks, not aggregate enumeration.
346-372: Tier lookup usesinoperator instead of constant-time comparison.The
get_user_tiermethod usesif normalized_email in tier_list(Line 351) andif domain in tier_list(Line 361), which are not constant-time operations unlikeis_authorizedandis_admin. This inconsistency could leak tier assignment timing information.If consistency with the security model is desired:
# First check exact email matches for tier_list, tier_type in zip(tier_lists, tier_types, strict=False): - if normalized_email in tier_list: + for entry in tier_list: + if secrets.compare_digest(normalized_email, entry): + logger.debug( + "Email %s has %s tier (exact match)", email, tier_type.value + ) + return tier_type
426-482: Refactor to reduce cognitive complexity (SonarCloud: 22 vs 15 allowed).Extract validation checks into smaller helper methods to improve readability and maintainability.
- def validate_whitelist_config(self) -> list[str]: # noqa: C901, PLR0912 + def validate_whitelist_config(self) -> list[str]: """Validate the whitelist configuration and return any warnings.""" warnings = [] + warnings.extend(self._check_empty_whitelist()) + warnings.extend(self._check_users_not_in_whitelist()) + warnings.extend(self._check_tier_conflicts()) + warnings.extend(self._check_public_domains()) + return warnings - # Check for empty whitelist - if not self.individual_emails and not self.domain_patterns: - warnings.append("Whitelist is empty - no users will be authorized") - ... + def _check_empty_whitelist(self) -> list[str]: + if not self.individual_emails and not self.domain_patterns: + return ["Whitelist is empty - no users will be authorized"] + return [] + + def _check_users_not_in_whitelist(self) -> list[str]: + # ... extracted logic for admin/full/limited user checks + + def _check_tier_conflicts(self) -> list[str]: + # ... extracted logic for tier conflict detection + + def _check_public_domains(self) -> list[str]: + # ... extracted logic for public domain warnings
505-586: Refactor to reduce cognitive complexity (SonarCloud: 32 vs 15 allowed).The
add_emailmethod handles too many concerns. Extract email validation and domain pattern validation into separate helper methods.- def add_email(self, email: str, is_admin: bool = False) -> bool: # noqa: C901, PLR0912 + def add_email(self, email: str, is_admin: bool = False) -> bool: """Add email to whitelist (runtime operation).""" try: if not email or not email.strip(): msg = "Email cannot be empty" raise ValueError(msg) normalized_email = self.validator._normalize_email(email) - - # Validate email format (if not a domain pattern) - if not normalized_email.startswith("@"): - if EMAIL_VALIDATOR_AVAILABLE and validate_email is not None: - ... - else: - ... + normalized_email = self._validate_email_format(normalized_email) + self._add_to_collection(normalized_email, is_admin) + logger.info("Added email %s to whitelist (admin: %s)", email, is_admin) + return True except ValueError: raise except Exception as e: ... + + def _validate_email_format(self, email: str) -> str: + """Validate and return normalized email/domain pattern.""" + if email.startswith("@"): + return self._validate_domain_pattern(email) + return self._validate_individual_email(email).claude/skills/project-planning/templates/tech-spec-template.md (1)
42-56: Malformed nested code fences will break markdown rendering.Lines 54-55 open a new
```textblock inside an already open code fence, which will render literally instead of closing/opening properly. The template structure appears to have formatting issues with nested fences.The template should use a consistent approach. If demonstrating code blocks within the template, use different fence lengths (e.g., `````):
-```text +### Component Diagram ┌─────────────────────────────────────────┐ │ [Component] │ -... └─────────────────────────────────────────┘ - -```textpackages/cloudflare-auth/src/cloudflare_auth/utils.py (1)
207-232: Regex pattern for email masking may miss edge cases.The default email regex pattern on Line 208 uses
[A-Z|a-z]which incorrectly includes the literal pipe character|in the character class. Should be[A-Za-z].def mask_sensitive_data( - text: str, pattern: str = r"\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Z|a-z]{2,}\b" + text: str, pattern: str = r"\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}\b" ) -> str:README.md (1)
20-20: Stale badge references removed workflow.The
publish-pypi.ymlworkflow was removed in this PR, but the badge on line 20 still references it. This badge will show as failing or not found.Either remove the badge or update it to reference the new
publish-artifact-registry.ymlworkflow:-[](https://github.com/ByronWilliamsCPA/python_libs/actions/workflows/publish-pypi.yml) +[](https://github.com/ByronWilliamsCPA/python_libs/actions/workflows/publish-artifact-registry.yml)packages/cloudflare-auth/src/cloudflare_auth/middleware_enhanced.py (3)
235-387: High cognitive complexity (37) exceeds threshold (15).The
_authenticate_requestmethod handles multiple concerns: rate limiting, session validation, JWT extraction/validation, whitelist checking, and user creation. This makes the function difficult to test and maintain.Consider extracting distinct responsibilities into helper methods:
async def _authenticate_request(self, request: Request) -> CloudflareUser | None: """Authenticate request using JWT and whitelist.""" self._check_rate_limit(request) # Try session-based auth first if user := self._try_session_auth(request): return user # Fall back to JWT auth jwt_token = self._extract_jwt_token(request) if not jwt_token: return self._handle_missing_token(request) claims = self._validate_jwt(request, jwt_token) if not claims: return None return self._create_authenticated_user(request, claims)This refactor would:
- Reduce cognitive complexity significantly
- Make each authentication step independently testable
- Improve readability and maintainability
296-308: Bug:self._get_client_ipmethod does not exist.Line 301 calls
self._get_client_ip(request)but this method is not defined on the class. The imported utility functionget_client_ip(line 52) should be used instead, consistent with its usage elsewhere in the file (lines 251, 316, 324).# Security: Validate JWT token size (prevent DoS) if len(jwt_token) > 8192: # 8KB limit logger.warning( "JWT token too large: %d bytes (path: %s, ip: %s)", len(jwt_token), - request.url.path, - self._get_client_ip(request), + sanitize_path(request.url.path), + sanitize_ip(get_client_ip(request)), )Note: Also applied
sanitize_pathandsanitize_ipfor consistency with the logging pattern used elsewhere in this method.
463-577: Return type annotation inconsistent with actual return value.The function
setup_cloudflare_auth_enhanceddeclares return typeCloudflareAuthMiddlewareEnhanced(line 474) but returnsNone(line 577). This will fail type checking and misleads callers.def setup_cloudflare_auth_enhanced( app: Any, whitelist: list[str] | None = None, admin_emails: list[str] | None = None, full_users: list[str] | None = None, limited_users: list[str] | None = None, excluded_paths: list[str] | None = None, enable_sessions: bool = True, require_auth: bool = True, session_timeout: int = 3600, settings: CloudflareSettings | None = None, -) -> CloudflareAuthMiddlewareEnhanced: +) -> None:Update the docstring's Returns section accordingly.
packages/cloudflare-auth/src/cloudflare_auth/redis_sessions.py (1)
282-290: Use SCAN instead of KEYS for production workloads to avoid blocking the Redis server.The
KEYScommand is O(N) and blocks Redis during execution on large datasets. Whileget_user_sessionsincludes a warning about this expensive operation,get_session_countuses the same pattern without any caution. Additionally,get_session_countis called byget_stats, propagating the same issue.For production environments with many sessions, use
SCANwith cursor-based iteration to avoid blocking, or maintain a separate session counter key that is incremented/decremented on session create/delete operations.docs/development/architecture.md (1)
16-30: Fix the markdown code fence syntax error.Line 30 should be a closing fence delimiter (
```) to properly close the block opened on line 16, not another opening fence with thetextlanguage identifier.Apply this diff to fix the fence syntax:
```text python_libs/ ├── src/ │ └── python_libs/ │ ├── __init__.py # Package initialization │ ├── core/ # Core functionality │ │ ├── config.py # Configuration with Pydantic │ │ └── ... │ ├── utils/ # Utility modules │ │ ├── logging.py # Structured logging │ │ └── ... ├── tests/ # Test suite ├── docs/ # Documentation └── pyproject.toml # Project configuration -```text +```packages/cloudflare-auth/src/cloudflare_auth/security_helpers.py (2)
153-169: Don’t let the cleanup task die on transient exceptions.
Right now any unexpected exception logs once and exits the task (Line 166-168). For a maintenance loop, it’s usually better to log and continue.async def cleanup_loop() -> None: """Background loop for session cleanup.""" logger.info("Session cleanup task started (interval: %ds)", cleanup_interval) - try: - while True: - await asyncio.sleep(cleanup_interval) - count = session_manager.cleanup_expired_sessions() - if count > 0: - logger.info("Cleaned up %d expired sessions", count) - except asyncio.CancelledError: - logger.info("Session cleanup task cancelled") - raise - except Exception as e: - logger.error("Session cleanup error: %s", e, exc_info=True) + try: + while True: + try: + await asyncio.sleep(cleanup_interval) + count = session_manager.cleanup_expired_sessions() + if count > 0: + logger.info("Cleaned up %d expired sessions", count) + except Exception: + logger.exception("Session cleanup error") + except asyncio.CancelledError: + logger.info("Session cleanup task cancelled") + raise
220-234: Structured audit logging looks consistent; consider a PII stance.
Theextra={...}payloads are coherent. Since this is a library, consider documenting (or adding an option for) whether rawadmin_email/user_email/ipshould be logged vs. hashed/sanitized.Also applies to: 253-267, 284-297, 321-333
.claude/skills/project-planning/reference/document-guide.md (1)
19-120: Doc updates are coherent and the ```text fences improve readability.Minor: consider adding one sentence clarifying whether “PVS” is expected to be a single file name (e.g.,
project-vision.md) vs. a doc type, to reduce ambiguity in cross-references.Also applies to: 232-277
packages/cloudflare-auth/src/cloudflare_auth/__init__.py (1)
38-63: Potential public API break:setup_cloudflare_authlikely no longer exported.
middleware.pystill showsfrom cloudflare_auth import setup_cloudflare_auth, but__init__.pydoesn’t import/export it. If this wasn’t intentional, re-export it (or update all docs/examples and consider a deprecation path).from cloudflare_auth.middleware import ( CloudflareAuthMiddleware, get_current_user, get_current_user_optional, + setup_cloudflare_auth, ) @@ __all__ = [ @@ + "setup_cloudflare_auth", "setup_cloudflare_auth_enhanced", ]Also applies to: 74-105
packages/cloudflare-auth/src/cloudflare_auth/middleware.py (3)
189-210: Fix IP allowlist validation: current prefix logic is insecure/incorrect.
client_ip.startswith(allowed_ip.rstrip("/") + ".")(Line 194-196) can over-allow (and doesn’t correctly support CIDR). Useipaddressand explicitip_networkmembership.+import ipaddress @@ if self.settings.allowed_tunnel_ips: client_ip = get_client_ip(request) - - # Check if IP is in allowlist - ip_allowed = any( - client_ip == allowed_ip - or client_ip.startswith(allowed_ip.rstrip("/") + ".") - for allowed_ip in self.settings.allowed_tunnel_ips - ) + try: + client_addr = ipaddress.ip_address(client_ip) + except ValueError: + client_addr = None + + allowed_networks: list[ipaddress._BaseNetwork] = [] + for entry in self.settings.allowed_tunnel_ips: + try: + allowed_networks.append(ipaddress.ip_network(entry, strict=False)) + except ValueError: + # Treat bare IPs as single-host networks + allowed_networks.append(ipaddress.ip_network(f"{entry}/32", strict=False)) + + ip_allowed = client_addr is not None and any( + client_addr in net for net in allowed_networks + )
364-392: Normalize emails before comparison to avoid false mismatches.
Email local/domain are effectively case-insensitive in most real-world identity systems; comparing raw strings (Line 380-381) can reject valid users if Cloudflare varies casing/whitespace.- if email_header != user.email: + if email_header.strip().casefold() != user.email.strip().casefold():
260-282: Remove the "async withoutawait" claim; focus on refactoring high cognitive complexity.The
_authenticate_requestmethod is properly awaited on line 261. However, the method does have elevated cognitive complexity (noted by the# noqa: C901, PLR0912suppressions). Extract the validation logic—JWT validation, email header checks, and rate limiting—into separate helper methods to reduce complexity and improve maintainability.packages/cloudflare-auth/src/cloudflare_auth/csrf.py (2)
72-94: Fix token generation: remove no-op + use HMAC for session-bound tokens.
secrets.token_bytes(32)is dead code (Line 82), andsha256(secret_key || data)is not a safe MAC construction.import hashlib import logging import secrets +import hmac @@ def generate_token(self, session_id: str | None = None) -> str: @@ - # Generate random token - secrets.token_bytes(32) - # Optionally bind to session ID if session_id: - # Create HMAC-like token bound to session data = f"{session_id}{secrets.token_hex(16)}".encode() - token = hashlib.sha256(self.secret_key.encode() + data).hexdigest() + token = hmac.new(self.secret_key.encode(), data, hashlib.sha256).hexdigest() else: # Simple random token token = secrets.token_urlsafe(32)
161-182: Global instance ignores later (cookie_name, header_name) arguments.
After the first call, passing differentcookie_name/header_namesilently won’t apply. Consider documenting this explicitly or keying the cache by(cookie_name, header_name).
♻️ Duplicate comments (7)
.qlty/results (1)
1-1: Cache file flagged above.Same issue as
.qlty/logs—remove cache artifacts from version control..qlty/out (1)
1-1: Same issue: absolute local path should not be committed.This file has the same problem as
.qlty/plugin_cachedir- it contains an absolute local path that is user-specific and should not be committed..github/workflows/pr-validation.yml (1)
68-68: Same action version update as in security-analysis.yml.This workflow has the same
astral-sh/setup-uvaction version update. Please refer to the verification request in.github/workflows/security-analysis.yml(line 41) to ensure this change is intentional and correct..github/workflows/docs.yml (1)
40-40: Same action version update as in security-analysis.yml.This workflow has the same
astral-sh/setup-uvaction version update. Please refer to the verification request in.github/workflows/security-analysis.yml(line 41) to ensure this change is intentional and correct..github/workflows/slsa-provenance.yml (1)
59-59: Verify the action commit hash corresponds to the official v4 release.The
astral-sh/setup-uvaction has been updated to a new commit hash. This is the same update as in other workflows. Ensure this commit hash is legitimate and corresponds to the official v4 release..github/workflows/ci.yml (1)
74-74: Verify the action commit hash corresponds to the official v4 release.The
astral-sh/setup-uvaction has been updated to a new commit hash across three jobs (test-cloudflare-auth, test-gcs-utilities, and security). This is the same update as in other workflows. Ensure this commit hash is legitimate and corresponds to the official v4 release.Also applies to: 120-120, 162-162
.github/workflows/publish-artifact-registry.yml (1)
152-152: Security: Pin actions to commit hashes.Three actions are using mutable tags instead of pinned commit hashes:
google-github-actions/auth@v2google-github-actions/setup-gcloud@v2astral-sh/setup-uv@v4This creates supply chain security risks. Note that the release.yml workflow already pins setup-uv to commit hash
e4db8464a088ece1b920f60402e813ea4de65b8f.Apply these changes:
- name: Authenticate to Google Cloud - uses: google-github-actions/auth@v2 + uses: google-github-actions/auth@75a8f37e45d8c2d94592c5e1227ed2c472bdbfe3 # v2.1.9 - name: Set up Cloud SDK - uses: google-github-actions/setup-gcloud@v2 + uses: google-github-actions/setup-gcloud@f0990588f1e5b5af6827153b93673613abdc6ec7 # v2.1.2 - name: Install uv - uses: astral-sh/setup-uv@v4 + uses: astral-sh/setup-uv@e4db8464a088ece1b920f60402e813ea4de65b8f # v4Also applies to: 157-157, 173-173
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock,!**/*.lock
📒 Files selected for processing (102)
.claude/README.md(2 hunks).claude/agents/code-reviewer.md(2 hunks).claude/agents/merge-standards.md(2 hunks).claude/agents/security-auditor.md(4 hunks).claude/agents/test-engineer.md(4 hunks).claude/commands/merge-standards.md(1 hunks).claude/commands/plan.md(3 hunks).claude/commands/pr.md(2 hunks).claude/commands/quality.md(3 hunks).claude/commands/security.md(6 hunks).claude/commands/testing.md(2 hunks).claude/context/python-standards.md(7 hunks).claude/context/testing-patterns.md(8 hunks).claude/skills/commit-prepare/SKILL.md(9 hunks).claude/skills/git/SKILL.md(1 hunks).claude/skills/pr-prepare/SKILL.md(5 hunks).claude/skills/project-planning/SKILL.md(11 hunks).claude/skills/project-planning/reference/document-guide.md(6 hunks).claude/skills/project-planning/reference/prompting-patterns.md(11 hunks).claude/skills/project-planning/templates/adr-template.md(1 hunks).claude/skills/project-planning/templates/roadmap-template.md(2 hunks).claude/skills/project-planning/templates/tech-spec-template.md(4 hunks).claude/skills/project-planning/workflows/synthesize.md(13 hunks).claude/skills/quality/SKILL.md(3 hunks).claude/skills/security/SKILL.md(2 hunks).claude/skills/testing/SKILL.md(4 hunks).cruft.json(2 hunks).github/workflows/README.md(9 hunks).github/workflows/ci.yml(3 hunks).github/workflows/docs.yml(1 hunks).github/workflows/fips-compatibility.yml(3 hunks).github/workflows/pr-validation.yml(1 hunks).github/workflows/publish-artifact-registry.yml(1 hunks).github/workflows/publish-pypi.yml(0 hunks).github/workflows/python-compatibility.yml(1 hunks).github/workflows/release.yml(4 hunks).github/workflows/scorecard.yml(0 hunks).github/workflows/security-analysis.yml(1 hunks).github/workflows/slsa-provenance.yml(1 hunks).markdownlint.json(1 hunks).pre-commit-config.yaml(1 hunks).qlty/logs(1 hunks).qlty/out(1 hunks).qlty/plugin_cachedir(1 hunks).qlty/qlty.toml(3 hunks).qlty/results(1 hunks).sonarlint/connectedMode.json(1 hunks).standards/CLAUDE.baseline.md(2 hunks).standards/README.baseline.md(1 hunks).standards/README.md(2 hunks).standards/template_feedback.baseline.md(1 hunks).yamllint(1 hunks)CHANGELOG.md(3 hunks)CLAUDE.md(24 hunks)CONFIG_TEMPLATES_SUMMARY.md(10 hunks)CONTRIBUTING.md(11 hunks)LICENSES/Apache-2.0.txt(0 hunks)LICENSES/BSD-3-Clause.txt(0 hunks)LICENSES/GPL-3.0.txt(0 hunks)README.md(20 hunks)REUSE.toml(5 hunks)docs/ADRs/adr-template.md(3 hunks)docs/OPENSSF_COMPLIANCE.md(1 hunks)docs/PROJECT_SETUP.md(23 hunks)docs/PYTHON_COMPATIBILITY.md(3 hunks)docs/api-reference.md(0 hunks)docs/development/architecture.md(2 hunks)docs/development/code-quality.md(1 hunks)docs/development/contributing.md(1 hunks)docs/development/testing.md(2 hunks)docs/diagrams/publish-workflow.puml(1 hunks)docs/index.md(1 hunks)docs/planning/README.md(3 hunks)docs/planning/project-plan-template.md(15 hunks)docs/planning/project-vision.md(1 hunks)docs/planning/roadmap.md(1 hunks)docs/planning/tech-spec.md(1 hunks)docs/project/license.md(2 hunks)docs/template_feedback.md(1 hunks)mkdocs.yml(0 hunks)packages/cloudflare-auth/pyproject.toml(1 hunks)packages/cloudflare-auth/src/cloudflare_auth/__init__.py(3 hunks)packages/cloudflare-auth/src/cloudflare_auth/config.py(1 hunks)packages/cloudflare-auth/src/cloudflare_auth/csrf.py(8 hunks)packages/cloudflare-auth/src/cloudflare_auth/middleware.py(10 hunks)packages/cloudflare-auth/src/cloudflare_auth/middleware_enhanced.py(8 hunks)packages/cloudflare-auth/src/cloudflare_auth/models.py(6 hunks)packages/cloudflare-auth/src/cloudflare_auth/rate_limiter.py(11 hunks)packages/cloudflare-auth/src/cloudflare_auth/redis_sessions.py(6 hunks)packages/cloudflare-auth/src/cloudflare_auth/security_helpers.py(6 hunks)packages/cloudflare-auth/src/cloudflare_auth/sessions.py(8 hunks)packages/cloudflare-auth/src/cloudflare_auth/utils.py(10 hunks)packages/cloudflare-auth/src/cloudflare_auth/validators.py(7 hunks)packages/cloudflare-auth/src/cloudflare_auth/whitelist.py(14 hunks)packages/cloudflare-auth/tests/test_models.py(1 hunks)packages/gcs-utilities/src/gcs_utilities/__init__.py(1 hunks)packages/gcs-utilities/src/gcs_utilities/client.py(19 hunks)packages/gcs-utilities/src/gcs_utilities/exceptions.py(0 hunks)packages/gcs-utilities/tests/conftest.py(1 hunks)packages/gcs-utilities/tests/test_exceptions.py(1 hunks)pyproject.toml(2 hunks)scripts/README.md(1 hunks)
💤 Files with no reviewable changes (8)
- LICENSES/BSD-3-Clause.txt
- LICENSES/GPL-3.0.txt
- packages/gcs-utilities/src/gcs_utilities/exceptions.py
- LICENSES/Apache-2.0.txt
- .github/workflows/scorecard.yml
- mkdocs.yml
- docs/api-reference.md
- .github/workflows/publish-pypi.yml
🧰 Additional context used
📓 Path-based instructions (2)
.github/workflows/**
⚙️ CodeRabbit configuration file
.github/workflows/**: Review GitHub Actions workflows for:
- Security best practices (minimal permissions, pinned actions)
- Proper secret handling
- Efficient caching strategies
- Clear job dependencies
Files:
.github/workflows/python-compatibility.yml.github/workflows/pr-validation.yml.github/workflows/security-analysis.yml.github/workflows/release.yml.github/workflows/README.md.github/workflows/fips-compatibility.yml.github/workflows/ci.yml.github/workflows/slsa-provenance.yml.github/workflows/docs.yml.github/workflows/publish-artifact-registry.yml
pyproject.toml
⚙️ CodeRabbit configuration file
pyproject.toml: Review dependency changes for:
- Version constraint appropriateness
- Security implications of new dependencies
- License compatibility
Files:
pyproject.toml
🧬 Code graph analysis (6)
packages/cloudflare-auth/src/cloudflare_auth/models.py (1)
packages/cloudflare-auth/src/cloudflare_auth/whitelist.py (2)
UserTier(43-92)is_admin(282-304)
packages/gcs-utilities/tests/test_exceptions.py (1)
packages/cloudflare-auth/tests/test_models.py (1)
test_placeholder(7-10)
packages/cloudflare-auth/src/cloudflare_auth/config.py (1)
packages/cloudflare-auth/src/cloudflare_auth/models.py (1)
email_domain(200-206)
packages/gcs-utilities/src/gcs_utilities/client.py (1)
packages/gcs-utilities/src/gcs_utilities/exceptions.py (5)
GCSAuthError(8-9)GCSConfigError(24-25)GCSNotFoundError(20-21)GCSUploadError(12-13)GCSDownloadError(16-17)
packages/cloudflare-auth/tests/test_models.py (1)
packages/gcs-utilities/tests/test_exceptions.py (1)
test_placeholder(7-10)
packages/cloudflare-auth/src/cloudflare_auth/security_helpers.py (1)
packages/cloudflare-auth/src/cloudflare_auth/sessions.py (1)
SimpleSessionManager(28-321)
🪛 GitHub Check: CodeQL
.github/workflows/publish-artifact-registry.yml
[warning] 152-152: Unpinned tag for a non-immutable Action in workflow
Unpinned 3rd party Action 'Publish to Artifact Registry' step Uses Step uses 'google-github-actions/auth' with ref 'v2', not a pinned commit hash
[warning] 157-157: Unpinned tag for a non-immutable Action in workflow
Unpinned 3rd party Action 'Publish to Artifact Registry' step Uses Step uses 'google-github-actions/setup-gcloud' with ref 'v2', not a pinned commit hash
[warning] 173-173: Unpinned tag for a non-immutable Action in workflow
Unpinned 3rd party Action 'Publish to Artifact Registry' step Uses Step uses 'astral-sh/setup-uv' with ref 'v4', not a pinned commit hash
🪛 GitHub Check: SonarCloud Code Analysis
.github/workflows/publish-artifact-registry.yml
[failure] 89-89: Change this workflow to not use user-controlled data directly in a run block.
[warning] 140-140: Complete the task associated to this "TODO" comment.
packages/cloudflare-auth/src/cloudflare_auth/whitelist.py
[failure] 426-426: Refactor this function to reduce its Cognitive Complexity from 22 to the 15 allowed.
[failure] 505-505: Refactor this function to reduce its Cognitive Complexity from 32 to the 15 allowed.
packages/cloudflare-auth/src/cloudflare_auth/middleware.py
[failure] 286-286: Refactor this function to reduce its Cognitive Complexity from 23 to the 15 allowed.
[warning] 286-286: Use asynchronous features in this function or remove the async keyword.
packages/cloudflare-auth/src/cloudflare_auth/middleware_enhanced.py
[warning] 235-235: Use asynchronous features in this function or remove the async keyword.
[failure] 235-235: Refactor this function to reduce its Cognitive Complexity from 37 to the 15 allowed.
🪛 LanguageTool
.claude/agents/security-auditor.md
[style] ~62-~62: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ... TLS/SSL properly configured - [ ] CORS properly restricted ### Authentication & Author...
(ADVERB_REPETITION_PREMIUM)
| 1. **Output the PR description** ready to copy-paste into GitHub. | ||
|
|
||
| 5. **Suggest a PR title** following conventional commits: | ||
| 2. **Suggest a PR title** following conventional commits: |
There was a problem hiding this comment.
Fix incorrect step numbering.
The steps are renumbered to 1 and 2, but there are already steps 1-3 defined earlier in the document (lines 7-43). This creates confusion in the workflow sequence.
Apply this diff to restore the correct numbering:
-1. **Output the PR description** ready to copy-paste into GitHub.
+4. **Output the PR description** ready to copy-paste into GitHub.
-2. **Suggest a PR title** following conventional commits:
+5. **Suggest a PR title** following conventional commits:🤖 Prompt for AI Agents
In .claude/commands/pr.md around lines 45 to 47, the two steps were renumbered
as "1" and "2" which conflicts with existing steps 1–3 earlier in the file;
update the numbering to continue the sequence (change those two steps to "4."
and "5.") so the workflow sequence is correct and consistent with the earlier
lines.
| ```text | ||
|
|
||
| Phase 0: Foundation ████████░░░░░░░░ (1 week) - Setup | ||
| Phase 1: MVP Core ░░░░░░░░████████ (3 weeks) - Core features | ||
| Phase 2: Enhancement ░░░░░░░░░░░░░░░░ (2 weeks) - Additional features | ||
| Phase 3: Polish ░░░░░░░░░░░░░░░░ (1 week) - Testing & docs | ||
| ``` | ||
|
|
||
| ```text | ||
|
|
||
| ## Milestones |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .claude/skills/project-planning/templates/roadmap-template.md | sed -n '10,30p'Repository: ByronWilliamsCPA/python-libs
Length of output: 866
Fix malformed code fence in Timeline Overview section.
Line 15 opens a code fence with \``textbut it is never properly closed. Line 22 incorrectly attempts to open another```textblock without closing the first one. Add a closing fence (`` ``) at line 22 before starting the new section.
Phase 3: Polish ░░░░░░░░░░░░░░░░ (1 week) - Testing & docs
Milestones
The first `\`\`\`text` should close at line 22, then optionally a new `\`\`\`text` can begin if needed (though the Milestones section uses a table, not a text block).
<details>
<summary>🤖 Prompt for AI Agents</summary>
.claude/skills/project-planning/templates/roadmap-template.md around lines 15 to
24: the Timeline Overview opens a code fence with text at line 15 but it is never closed and line 22 incorrectly starts another text; close the first
code fence by inserting a closing triple backtick (```) at line 22 before the
Milestones section and remove the stray second opening fence so the timeline
block is properly terminated and the Milestones section is not inside a code
block.
</details>
<!-- fingerprinting:phantom:triton:mongoose -->
<!-- This is an auto-generated comment by CodeRabbit -->
| "[field]": "[type]" | ||
| } | ||
| ``` | ||
| ```text |
There was a problem hiding this comment.
Misplaced ```text fence after JSON block.
Line 96 has ```text appearing after what should be a closing ``` for the JSON block. This breaks the template structure.
🤖 Prompt for AI Agents
In .claude/skills/project-planning/templates/tech-spec-template.md around line
96, there is a misplaced "```text" fence used where the JSON block should have
been closed; remove or replace that stray "```text" with a proper closing "```"
(or delete it if redundant) so the JSON block is terminated correctly and the
rest of the template resumes with the intended fences and content.
| Dictionary with session statistics | ||
| """ | ||
| now = datetime.now() | ||
| datetime.now(tz=UTC) |
There was a problem hiding this comment.
Dead code: unused datetime.now(tz=UTC) call.
This statement has no side effects and its result is discarded. It appears to be leftover from a refactor. Remove it to avoid confusion.
def get_stats(self) -> dict[str, Any]:
"""Get session manager statistics.
Returns:
Dictionary with session statistics
"""
- datetime.now(tz=UTC)
active_sessions = []
expired_sessions = []📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| datetime.now(tz=UTC) | |
| def get_stats(self) -> dict[str, Any]: | |
| """Get session manager statistics. | |
| Returns: | |
| Dictionary with session statistics | |
| """ | |
| active_sessions = [] | |
| expired_sessions = [] |
🤖 Prompt for AI Agents
In packages/cloudflare-auth/src/cloudflare_auth/sessions.py around line 289,
there is a standalone datetime.now(tz=UTC) call whose return value is discarded;
remove this dead code line. Delete the extraneous datetime.now(tz=UTC) statement
so no unused expression remains, and ensure imports are adjusted if UTC or
datetime are no longer used elsewhere (remove unused imports).
| # Cloudflare auth middleware - Complex authentication logic | ||
| "packages/cloudflare-auth/src/cloudflare_auth/middleware*.py" = [ | ||
| "TRY300", # Return in try block is clearer for auth flow | ||
| "TRY301", # Raising in try block is standard for auth middleware | ||
| "PLC0415", # Local imports to avoid circular dependencies | ||
| ] | ||
|
|
||
| # Cloudflare auth whitelist - Validation logic | ||
| "packages/cloudflare-auth/src/cloudflare_auth/whitelist.py" = [ | ||
| "TRY300", # Return in try block is clearer | ||
| "TRY301", # Raising in try block is standard | ||
| "TRY401", # Exception in log is intentional for audit | ||
| "SLF001", # Internal access to validator internals is intentional | ||
| "PGH003", # Generic type ignore is acceptable for optional import | ||
| "PERF401", # List comprehension vs append is clearer here | ||
| ] | ||
|
|
||
| # Cloudflare auth redis sessions - Session management logic | ||
| "packages/cloudflare-auth/src/cloudflare_auth/redis_sessions.py" = [ | ||
| "TRY300", # Return in try block is clearer for session flow | ||
| "TRY401", # Exception in log message is intentional | ||
| ] | ||
|
|
||
| # Cloudflare auth security helpers - Security patterns | ||
| "packages/cloudflare-auth/src/cloudflare_auth/security_helpers.py" = [ | ||
| "G201", # Using .error with exc_info is intentional for security logging | ||
| "TRY300", # Return patterns in try blocks | ||
| "SLF001", # Function attribute pattern for singleton | ||
| ] | ||
|
|
||
| # Cloudflare auth validators - JWT validation logic | ||
| "packages/cloudflare-auth/src/cloudflare_auth/validators.py" = [ | ||
| "TRY300", # Return in try block is clearer for validation | ||
| "TRY301", # Raising in try block is standard for validation | ||
| "TRY401", # Exception in log is intentional | ||
| ] | ||
|
|
||
| # Cloudflare auth utils - Utility functions | ||
| "packages/cloudflare-auth/src/cloudflare_auth/utils.py" = [ | ||
| "BLE001", # Blind except is intentional for resilient parsing | ||
| "ANN001", # Regex match type annotation not needed | ||
| "ANN202", # Private function return type annotation not needed | ||
| ] | ||
|
|
||
| # Cloudflare auth tests - Test patterns | ||
| "packages/cloudflare-auth/tests/*.py" = [ | ||
| "ANN201", # Return type annotations not needed for fixtures | ||
| "PERF401", # List comprehension vs extend is fine in tests | ||
| ] | ||
|
|
||
| # GCS utilities client - Storage operations | ||
| "packages/gcs-utilities/src/gcs_utilities/client.py" = [ | ||
| "G004", # F-string logging is cleaner for GCS operations | ||
| "TRY300", # Return in try block is clearer for storage ops | ||
| "TRY301", # Raising in try block is standard for storage errors | ||
| "TRY401", # Exception in log is intentional | ||
| "PTH101", # os.chmod preferred for credential file permissions | ||
| "PTH108", # os.unlink preferred for credential file cleanup | ||
| "PTH110", # os.path.exists preferred for credential file check | ||
| "PERF401", # List comprehension vs append is clearer here | ||
| ] |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider consolidating per-file lint exceptions.
While the per-file ignores are contextually appropriate, the extensive list (60+ lines) may indicate that some patterns could be refactored or that certain rules might be too strict for this codebase.
Consider:
- Grouping common patterns (e.g., TRY300 appears in 5+ files)
- Evaluating if some global ignores would be more appropriate
- Documenting why each exception is necessary for future maintainers
🤖 Prompt for AI Agents
In pyproject.toml around lines 389 to 449, the per-file lint ignore lists are
repetitive; consolidate repeated codes (e.g., TRY300, TRY301, TRY401, PERF401,
SLF001) into a shared section or a named variable and reference it from each
file entry, evaluate which rules are genuinely safe to promote to a global
ignore versus per-file (adjust the pyproject top-level [tool.<linter>.ignore] or
equivalent), add brief inline comments documenting the rationale for each
grouped ignore, and run the linter to verify no unintended rules are suppressed.
- Fix boolean type in fips-compatibility.yml input default - Remove trailing newlines from scorecard.yml and api-reference.md - Improve TruffleHog pre-commit entry formatting 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add execute permissions to utility scripts for direct invocation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update README.md: - Add all four packages (cloudflare-auth, cloudflare-api, gcs-utilities, gemini-image) - Document GCP Artifact Registry as publish target (not PyPI) - Add Publishing section with workflow and tag format - Update project structure to reflect UV workspace layout - Add installation instructions for private registry Add PlantUML sequence diagram for publish workflow showing: - Tag-triggered GitHub Actions - Infisical secrets retrieval - GCP authentication flow - Artifact Registry publishing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Enable qlty plugins for comprehensive code analysis: - ruff: Python linting (blocking) - bandit: Python security (comment mode) - shellcheck: Shell script linting (comment mode) - yamllint: YAML linting (comment mode) - markdownlint: Markdown linting (comment mode) - actionlint: GitHub Actions linting (comment mode) - osv-scanner: Vulnerability scanning (comment mode) Fix code quality issues: - Add timezone to all datetime.now() calls (DTZ005) - Add specific pyright ignore codes (PGH003) - Configure per-file ignores for complex middleware files - Fix logging patterns (G201, TRY401) - Add noqa comments for intentional complexity - Format markdown files with markdownlint Update pyproject.toml with per-file-ignores for: - cloudflare-auth middleware (TRY300, TRY301, PLC0415) - cloudflare-auth whitelist (TRY300, TRY301, SLF001) - cloudflare-auth validators (TRY300, TRY301, TRY401) - gcs-utilities client (G004, TRY300, PTH rules) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add language specifiers to fenced code blocks (MD040) - Disable stylistic markdownlint rules (MD024, MD029, MD051, MD055, MD060) - Configure yamllint to allow truthy values (on/off/yes/no) - Add exclude_patterns and test_patterns to qlty.toml - Fix mkdocs.yml leading blank line - Run qlty fmt on unformatted markdown files 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add publish-artifact-registry.yml for private package publishing - Remove publish-pypi.yml (switching from public PyPI to private AR) - Update release.yml to remove PyPI publish step - Update workflow README with new publishing documentation - Add AR publishing configuration comments to pyproject.toml Packages are now published to: https://us-central1-python.pkg.dev/assured-oss-457903/python-libs/ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Infisical is behind Cloudflare Access tunnel which requires additional authentication. Temporarily using GitHub Secrets until Cloudflare Access service token authentication is configured for the Infisical action.
Adds a validation step that checks the tagged commit exists on the main branch. This prevents accidental publishing from feature branches.
- Fix invalid astral-sh/setup-uv SHA (use correct v4 SHA) - Fix cruft.json to use GitHub URL instead of local path - Fix cloudflare-auth package imports (remove src. prefix) - Add cloudflare_auth.config module with CloudflareSettings The setup-uv action SHA 582b2d78... did not exist, replaced with the correct SHA e4db8464... for the v4 floating tag. Cloudflare-auth imports were incorrectly using src.cloudflare_auth which doesn't work when the package is installed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add packages/**/src/**/*.py to MIT source code annotation - Add packages/**/*.md to CC-BY-4.0 documentation annotation - Add packages/**/pyproject.toml and py.typed to CC0-1.0 config annotation - Add common config files (.cruft.json, .env.example, linter configs) - Add uv.lock and other lockfiles to CC0-1.0 annotation - Remove unused license files (Apache-2.0, BSD-3-Clause, GPL-3.0) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ormatting - Add pydantic-settings>=2.0.0 as required dependency for CloudflareSettings - Fix ruff linting issues (unused imports, import sorting) - Format code with ruff - Add SonarLint connected mode configuration 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Use environment variables instead of direct ${{ inputs.* }} in shell
to prevent workflow injection vulnerabilities
- Add strict input validation for package name and version format
- Pin all GitHub Actions to commit SHAs:
- actions/checkout@34e1148 (v4)
- google-github-actions/auth@c200f36 (v2)
- google-github-actions/setup-gcloud@e427ad8 (v2)
- astral-sh/setup-uv@e4db846 (v4)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…functions
Refactor complex functions to reduce cognitive complexity below SonarCloud's
limit of 15:
- middleware.py _authenticate_request: 23 → ~8
- Extracted: _check_rate_limit, _handle_missing_token, _validate_token_size,
_validate_email_header, _record_failed_attempt, _handle_validation_error
- middleware_enhanced.py _authenticate_request: 37 → ~12
- Extracted: _check_rate_limit, _authenticate_from_session,
_handle_missing_token, _validate_token_size, _record_failed_attempt,
_handle_jwt_validation_error, _check_whitelist, _create_session_if_enabled
- Fixed undefined self._get_client_ip() → get_client_ip()
- whitelist.py validate_whitelist_config: 22 → ~6
- Extracted: _check_empty_whitelist, _check_tier_authorization,
_check_tier_conflicts, _check_public_domains
- whitelist.py add_email: 32 → ~8
- Extracted: _validate_empty_input, _validate_email_with_library,
_validate_email_basic, _validate_email_format, _validate_domain_pattern,
_add_to_collections
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add .qlty cache symlinks (logs, out, plugin_cachedir, results) to .gitignore to prevent tracking ephemeral cache artifacts. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fix markdown code fence closing tags (```text → ```) - Update PyPI Publish badge to Artifact Registry Publish - All code blocks now properly closed with bare ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add proper docstrings with Args/Returns sections to whitelist.py helper methods to fix darglint validation errors - Fix end-of-file markers in .env.example and .sonarlint/connectedMode.json (from pre-commit hooks) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
williaby
force-pushed
the
chore/config-workflow-updates
branch
from
e52fa5e to
ed4c077
Compare
✅ FIPS Compatibility Check
Status: ✅ PASSED What is FIPS?FIPS 140-2/140-3 is a US government standard for cryptographic modules. Common issues:
|
… paths - Add _record_failed_attempt() call to _handle_missing_token in both middlewares - Add _record_failed_attempt() call to _validate_email_header for missing/mismatched headers - Ensures rate limiting is applied consistently to prevent auth probing attacks Security improvement: Previously, missing JWT tokens and email header mismatches weren't triggering rate limiting, allowing attackers to probe these failure paths without rate limit restrictions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The root python_libs package uses pydantic_settings in its Settings class but was missing it from dependencies, causing CI test failures. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Skip TestCLI tests - CLI module not implemented yet - Add shell: bash to Windows CI to fix multi-line command parsing The CLI tests are placeholders from the template that expect a python_libs.cli module which doesn't exist in this project. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
2 participants
Summary
Changes
New Workflow:
publish-artifact-registry.ymlcloudflare-auth-v1.0.0)https://us-central1-python.pkg.dev/assured-oss-457903/python-libs/Modified:
release.ymlRemoved
publish-pypi.yml- No longer publishing to public PyPIRequired Secrets
GCP_SA_KEY_BASE64- Base64-encoded GCP service account JSON withroles/artifactregistry.writerTest plan
byronwilliamscpa-cloudflare-authv0.1.0 to Artifact Registry🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes & Improvements
Documentation
Security
✏️ Tip: You can customize this high-level summary in your review settings.