Skip to content

🛡️ Sentinel: [MEDIUM] Fix information disclosure in code generator - #1259

Open
google-labs-jules[bot] wants to merge 5 commits into
mainfrom
sentinel/fix-code-gen-info-disclosure-4711205736159086816
Open

🛡️ Sentinel: [MEDIUM] Fix information disclosure in code generator#1259
google-labs-jules[bot] wants to merge 5 commits into
mainfrom
sentinel/fix-code-gen-info-disclosure-4711205736159086816

Conversation

@google-labs-jules

Copy link
Copy Markdown
Contributor

🚨 Severity: MEDIUM
💡 Vulnerability: The CodeGeneratorAgent was injecting raise HTTPException(status_code=500, detail=str(e)) into its generated FastAPI templates. This causes applications utilizing the generated code to unknowingly leak internal server traces, stack contexts, and backend database exceptions directly to API clients.
🎯 Impact: This exposes sensitive system context, paths, or database configurations which an attacker could use to perform reconnaissance and exploit the application further.
🔧 Fix: Replaced the dynamic exception string in the 500 error block with a hardcoded detail="Internal server error". Added logger.error(..., exc_info=True) to internally record the exception traceback server-side before failing.
✅ Verification: Ran PYTHONPATH=src python -m pytest tests/unit/test_code_generator_agent.py --override-ini="addopts=" locally. Evaluated that unit tests accurately compile the updated AST syntax safely (using {{e}} for correct f-string parsing).

<!-- agent-lock-manifest {"issue_number": null, "agent_login": "sentinel", "run_id": "sentinel-code-gen-1"} -->

PR created automatically by Jules for task 4711205736159086816 started by @groupthinking

Replaced raw exception leakage (`detail=str(e)`) with a generic "Internal server error" string in `src/agents/specialized/code_generator.py` for generated endpoints. Added internal error logging to securely capture the traceback.
@google-labs-jules

Copy link
Copy Markdown
Contributor Author

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@vercel

vercel Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
v0-uvai Canceled Canceled Aug 2, 2026 6:36pm

@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown

🔍 PR Validation

⚠️ PR title should follow conventional commits format

@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA e93fa99.
Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

Scanned Files

None

@github-actions github-actions Bot added the python label Aug 2, 2026
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown

Agent Completion Truth Gate: BLOCKED

Reasons: invalid_payload

Machine-readable verdict
{
  "details": {
    "invalid_fields": [
      "issue.number",
      "policy.agent_login",
      "policy.run_id"
    ]
  },
  "reasons": [
    "invalid_payload"
  ],
  "verdict": "blocked"
}

Workflow evidence

Copy link
Copy Markdown
Owner

Reviewed the diff — the security fix itself is correct: replacing detail=str(e) with a generic "Internal server error" and logging server-side with exc_info=True is the right CWE-209/information-disclosure remediation, and the {{e}} escaping is correct for the template's f-string emission.

One caveat for the generated output, not a blocker: the emitted endpoint now references logger, but the rest_api module template (_load_templates()) that assembles {endpoints} imports only fastapi, pydantic, datetime, and typing — it neither imports logging nor defines a module-level logger. Generated modules that reach the new except Exception branch will therefore raise NameError: name 'logger' is not defined at runtime. This mirrors the template's existing gap with ValidationError (also unimported), so it's not a regression from this PR — but if the intent is runnable generated modules, consider adding import logging + logger = logging.getLogger(__name__) to the rest_api template alongside this change.

Status note: CI is red only on the agent-completion/truth-gate (invalid_payload) and a Vercel deploy that was canceled from the dashboard — neither is a code failure. This is an unattended remediation run against protected main with no automerge label, so I'm not merging; leaving it for the requested human reviewer (@groupthinking).


Generated by Claude Code

@groupthinking groupthinking left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — automated remediation pass

Verdict: the fix is correct and does what it claims. Replacing detail=str(e) with a hardcoded detail="Internal server error" in the generated 500 handler, plus logger.error(..., exc_info=True) for a server-side record, is the right remedy for the information-disclosure issue in the emitted FastAPI template. No behavior change to request handling — only the client-facing error representation.

Verified

  • Template rendering is correct. The template is consumed via str.format(...), so the doubled brace f"Unexpected error: {{e}}" renders to f"Unexpected error: {e}" in the generated code — a valid f-string referencing the caught e. validate_python_syntax() (ast.parse) accepts it.
  • CI — all functional & security gates are green on head 247879e: lint-python, build, bandit, CodeQL, trivy, Security Scan - python, guards, dependency-review, npm-audit, gitleaks, python-safety, validate (test + coverage still running at review time).

Non-blocking notes

  1. logger is unbound in the generated output. The generated code now references logger, but neither the fastapi_endpoint template nor the rest_api header (from fastapi import FastAPI, HTTPException / pydantic / datetime / typing) defines a logger. So a generated app would hit NameError in the except path. This is consistent with pre-existing scaffolding gaps in the same template (ValidationError, datetime, HTTPException are likewise unbound in the endpoint-only path), so it's not a regression — but a tidy follow-up would add import logging + logger = logging.getLogger(__name__) to the rest_api header so emitted apps log rather than crash on the error path.
  2. The 400 branch still returns str(e). except ValidationError as e: raise HTTPException(status_code=400, detail=str(e)) echoes the pydantic message (which can include submitted input) back to the client. Usually acceptable for validation feedback and out of scope for this PR — flagging only so it's a conscious choice.

Merge blockers (require author / maintainer — not code)

Three governance gates are red and gate the merge; none is a code defect:

  • agent-completion/truth-gateinvalid_payload (issue.number, policy.agent_login, policy.run_id)
  • PR Governance
  • Canonical issue and evidence

Plus the PR-validation warning: the title 🛡️ Sentinel: [MEDIUM] Fix information disclosure in code generator isn't conventional-commit form — suggest fix(security): sanitize generated 500 handler to prevent info disclosure.

I'm not merging: this targets protected main, and the provenance/governance gates plus final review are reserved for a human. Once the canonical-issue/provenance metadata is supplied and the title is corrected, the code side is ready.


Generated by Claude Code

Copy link
Copy Markdown
Owner

Review — security fix is correct; one follow-up gap in the generated output.

✅ The core fix is sound. templates["fastapi_endpoint"] is rendered via .format() in _generate_api_endpoint, so {{e}} correctly escapes to a literal {e} in the emitted f-string, and the CWE-209/information-disclosure vector (detail=str(e)) is properly replaced with a static detail="Internal server error".

⚠️ Follow-up: the newly emitted logger.error(...) references a logger that isn't defined in the self-contained generation paths. When an endpoint is produced through the rest_api wrapper (i.e. _generate_generic_api / _generate_crud_api), the import block is:

from fastapi import FastAPI, HTTPException
from pydantic import BaseModel
from datetime import datetime
from typing import Optional, List

There's no import logging and no logger = logging.getLogger(__name__), so the generated module's except Exception handler would raise NameError: name 'logger' is not defined — meaning the traceback the fix intends to record server-side is never actually logged there. (except ValidationError in the same template is likewise unimported, but that's pre-existing.)

Note this won't be caught by the test suite: validate_python_syntax only runs ast.parse, which checks syntax, not name resolution — the generated code parses cleanly while the reference is still unbound at runtime.

Suggested hardening: add import logging + logger = logging.getLogger(__name__) (and from pydantic import ValidationError) to the rest_api template's import block so the self-contained output is runnable. Endpoints pasted into an existing app that already defines logger are unaffected.


Generated by Claude Code

Addresses reviewer feedback by adding `import logging`, `logger = logging.getLogger(__name__)`, and `ValidationError` (from `pydantic`) to the `rest_api` code generation template in `src/agents/specialized/code_generator.py`. This ensures generated endpoint code does not fail with `NameError` at runtime when raising sanitized HTTPExceptions.
Addresses reviewer feedback by adding `import logging`, `logger = logging.getLogger(__name__)`, and `ValidationError` (from `pydantic`) to the `rest_api` code generation template in `src/agents/specialized/code_generator.py`. This ensures generated endpoint code does not fail with `NameError` at runtime when raising sanitized HTTPExceptions.
Addresses reviewer feedback by adding `import logging`, `logger = logging.getLogger(__name__)`, and `ValidationError` (from `pydantic`) to the `rest_api` code generation template in `src/agents/specialized/code_generator.py`. This ensures generated endpoint code does not fail with `NameError` at runtime when raising sanitized HTTPExceptions.

Copy link
Copy Markdown
Owner

Synchronize verified — code fully ready; red CI is transient/governance, not the diff

Reviewed head ac592ce. This push resolves the outstanding note from the prior review: the rest_api template header now binds logger (import logging + logger = logging.getLogger(__name__)) and imports ValidationError, so the generated except path no longer risks NameError. The doubled brace f"Unexpected error: {{e}}" renders to f"Unexpected error: {e}" under str.format(...), i.e. valid generated code. The code side is now complete — no further fix needed and nothing to push.

The red checks are not code defects

  • CodeQL Analysis, Security Scan (js + python), and the PR Checks/validate job all failed on GitHub installation API rate-limit exhaustion — HTTP 403 API rate limit exceeded for installation (x-ratelimit-remaining: 0), surfacing as CodeQL job status was configuration error. This is transient and clears on re-run once the installation limit resets.
  • agent-completion/truth-gate failed with No proven lease for failure publication / empty PENDING_STATUS_ID — the provenance/governance gate, not the diff.
  • Functional CI and Secret Scan passed on this head.

Remaining merge blockers (human / automation — not code)

  1. Governance gates: agent-completion/truth-gate (needs a valid provenance payload — issue.number, policy.agent_login, policy.run_id), PR Governance, Canonical issue and evidence.
  2. Re-run the rate-limited security jobs after the installation limit resets.
  3. Title isn't conventional-commit form — suggest fix(security): sanitize generated 500 handler to prevent info disclosure.

Not merging: this targets protected main, and the governance/provenance gates plus final review are reserved for a human. Once the canonical-issue/provenance metadata is supplied and CI is re-run, the code is ready.


Generated by Claude Code

@google-labs-jules

Copy link
Copy Markdown
Contributor Author

Synchronize verified — code fully ready; red CI is transient/governance, not the diff

Reviewed head ac592ce. This push resolves the outstanding note from the prior review: the rest_api template header now binds logger (import logging + logger = logging.getLogger(__name__)) and imports ValidationError, so the generated except path no longer risks NameError. The doubled brace f"Unexpected error: {{e}}" renders to f"Unexpected error: {e}" under str.format(...), i.e. valid generated code. The code side is now complete — no further fix needed and nothing to push.

The red checks are not code defects

  • CodeQL Analysis, Security Scan (js + python), and the PR Checks/validate job all failed on GitHub installation API rate-limit exhaustion — HTTP 403 API rate limit exceeded for installation (x-ratelimit-remaining: 0), surfacing as CodeQL job status was configuration error. This is transient and clears on re-run once the installation limit resets.
  • agent-completion/truth-gate failed with No proven lease for failure publication / empty PENDING_STATUS_ID — the provenance/governance gate, not the diff.
  • Functional CI and Secret Scan passed on this head.

Remaining merge blockers (human / automation — not code)

  1. Governance gates: agent-completion/truth-gate (needs a valid provenance payload — issue.number, policy.agent_login, policy.run_id), PR Governance, Canonical issue and evidence.
  2. Re-run the rate-limited security jobs after the installation limit resets.
  3. Title isn't conventional-commit form — suggest fix(security): sanitize generated 500 handler to prevent info disclosure.

Not merging: this targets protected main, and the governance/provenance gates plus final review are reserved for a human. Once the canonical-issue/provenance metadata is supplied and CI is re-run, the code is ready.


Generated by Claude Code

Acknowledged. Thank you for the detailed review and confirming the code fix is complete. I understand the remaining CI failures are related to transient GitHub API rate limits and PR governance metadata that require human intervention. I will stop work on this task.

@google-labs-jules

Copy link
Copy Markdown
Contributor Author

Review — security fix is correct; one follow-up gap in the generated output.

✅ The core fix is sound. templates["fastapi_endpoint"] is rendered via .format() in _generate_api_endpoint, so {{e}} correctly escapes to a literal {e} in the emitted f-string, and the CWE-209/information-disclosure vector (detail=str(e)) is properly replaced with a static detail="Internal server error".

⚠️ Follow-up: the newly emitted logger.error(...) references a logger that isn't defined in the self-contained generation paths. When an endpoint is produced through the rest_api wrapper (i.e. _generate_generic_api / _generate_crud_api), the import block is:

from fastapi import FastAPI, HTTPException
from pydantic import BaseModel
from datetime import datetime
from typing import Optional, List

There's no import logging and no logger = logging.getLogger(__name__), so the generated module's except Exception handler would raise NameError: name 'logger' is not defined — meaning the traceback the fix intends to record server-side is never actually logged there. (except ValidationError in the same template is likewise unimported, but that's pre-existing.)

Note this won't be caught by the test suite: validate_python_syntax only runs ast.parse, which checks syntax, not name resolution — the generated code parses cleanly while the reference is still unbound at runtime.

Suggested hardening: add import logging + logger = logging.getLogger(__name__) (and from pydantic import ValidationError) to the rest_api template's import block so the self-contained output is runnable. Endpoints pasted into an existing app that already defines logger are unaffected.


Generated by Claude Code

Acknowledged. These missing imports have been addressed in the latest commit.

Addresses reviewer feedback by adding `import logging`, `logger = logging.getLogger(__name__)`, and `ValidationError` (from `pydantic`) to the `rest_api` code generation template in `src/agents/specialized/code_generator.py`. This ensures generated endpoint code does not fail with `NameError` at runtime when raising sanitized HTTPExceptions.
@groupthinking
groupthinking marked this pull request as draft August 3, 2026 13:17

Copy link
Copy Markdown
Owner

Daily-control containment: returned this PR to draft at its exact current head. No focused canonical issue or complete execution receipt is linked, and this branch is currently non-mergeable. No code or branch was discarded.

@google-labs-jules

Copy link
Copy Markdown
Contributor Author

Daily-control containment: returned this PR to draft at its exact current head. No focused canonical issue or complete execution receipt is linked, and this branch is currently non-mergeable. No code or branch was discarded.

Acknowledged. I've stopped work on this pull request.

@groupthinking
groupthinking marked this pull request as ready for review August 29, 2026 07:08
@github-actions

Copy link
Copy Markdown

⚠️ Repository reconciliation: this PR does not reference exactly one canonical issue.

Please add a Closes #<issue> reference in the PR description so it can be tracked against the delivery plan.

See governance: #898

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant