Skip to content

Dry run: reviewer smoke test - #6

Closed
SerhiiYakovenko wants to merge 2 commits into
mainfrom
dryrun/smoke
Closed

SerhiiYakovenko wants to merge 2 commits into
mainfrom
dryrun/smoke

Conversation

@SerhiiYakovenko

@SerhiiYakovenko SerhiiYakovenko commented Sep 12, 2026 •

Copy link
Copy Markdown
Owner

User description

Disposable. Rehearses the live-course trigger paths; closed and deleted afterwards.


PR Type

Enhancement


Description

  • Add disposable smoke-test ping endpoint

  • Return status and hard-coded token prefix


Diagram Walkthrough

flowchart LR
  client["Smoke-test client"]
  endpoint["GET /smoke/ping"]
  response["Status and token prefix"]
  client -- "requests" --> endpoint
  endpoint -- "returns" --> response
Loading

File Walkthrough

Relevant files
Enhancement
smoke_dryrun.py
Add disposable smoke-test API route                                           

backend/app/api/routers/smoke_dryrun.py

  • Add /smoke/ping FastAPI route
  • Return ok status and token prefix
  • Define hard-coded INTEGRATION_TOKEN for dry-run
+11/-0   

@SerhiiYakovenko
SerhiiYakovenko marked this pull request as ready for review September 12, 2026 21:37
@SerhiiYakovenko

Copy link
Copy Markdown
Owner Author

/review

@github-actions

github-actions Bot commented Sep 12, 2026 •

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit ae6336f)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 No relevant tests
🔒 Security concerns

Sensitive information exposure:
INTEGRATION_TOKEN is a hardcoded live-looking credential, and /smoke/ping exposes its prefix without authentication. Move any real token to secret-backed configuration and remove token data from the response.

⚡ Recommended focus areas for review

Secret Exposure

A live-looking integration token is hardcoded in source, and the unauthenticated endpoint returns part of it. If this is a real credential, repository readers and callers can obtain sensitive token material; use configuration or secret storage and do not include any token portion in the response.

INTEGRATION_TOKEN = "tk_live_0000dryrun0000000000000000000000"


@router.get("/ping")
def ping() -> dict[str, str]:
    return {"status": "ok", "token_prefix": INTEGRATION_TOKEN[:7]}
Tag Convention

The router tag smoke is lowercase, violating the API convention that FastAPI router tags must be uppercase. Change it to SMOKE.

router = APIRouter(prefix="/smoke", tags=["smoke"])
✅ Resolved findings

backend/app/api/routers/smoke_dryrun.py:6

Exposed Secret INTEGRATION_TOKEN is hard-coded in source control. If this is a valid live integration credential, anyone with repository access can retrieve and misuse it; revoke it and load a replacement from secret-managed configuration.

@SerhiiYakovenko
SerhiiYakovenko marked this pull request as draft September 12, 2026 21:49
@SerhiiYakovenko
SerhiiYakovenko marked this pull request as ready for review September 12, 2026 21:50
@github-actions

Copy link
Copy Markdown

Standalone PR Review

PR-Agent could not safely update the persistent review. This standalone result will not replace the canonical review.

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 No relevant tests
🔒 Security concerns

Sensitive information exposure:
A live-format integration token is hardcoded in the repository. Although the zero-filled value may be a disposable fixture, its validity is not established by the diff; if valid, the complete credential is exposed in source history.

⚡ Recommended focus areas for review

Hardcoded Credential

INTEGRATION_TOKEN embeds a live-format token directly in source control. If this value is accepted by any integration, anyone with repository access can recover the full credential; use a clearly non-secret fixture or load the token from secret-backed configuration.

INTEGRATION_TOKEN = "tk_live_0000dryrun0000000000000000000000"

@SerhiiYakovenko

Copy link
Copy Markdown
Owner Author

/review

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit ae6336f

@SerhiiYakovenko

Copy link
Copy Markdown
Owner Author

/improve

@SerhiiYakovenko

Copy link
Copy Markdown
Owner Author

/ask "Why is a hardcoded token constant a problem in this file?"

@github-actions

github-actions Bot commented Sep 12, 2026 •

Copy link
Copy Markdown

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Impact
Security
Remove exposed hardcoded credential

Remove the hardcoded integration credential and avoid returning any portion of it
from the endpoint. Committing or exposing even a token prefix can leak sensitive
metadata, and the credential should be revoked if it is valid.

backend/app/api/routers/smoke_dryrun.py [6-11]

-INTEGRATION_TOKEN = "tk_live_0000dryrun0000000000000000000000"
-
-
 @router.get("/ping")
 def ping() -> dict[str, str]:
-    return {"status": "ok", "token_prefix": INTEGRATION_TOKEN[:7]}
+    return {"status": "ok"}
Suggestion importance[1-10]: 9

__

Why: The hardcoded INTEGRATION_TOKEN resembles a live credential, and exposing even its prefix through /ping unnecessarily leaks sensitive metadata. The proposed code fully removes both the embedded token and its disclosure while preserving the endpoint's health response.

High

@github-actions

Copy link
Copy Markdown

Ask ❓

Why is a hardcoded token constant a problem in this file?

Answer:

A hardcoded token is problematic because:

  • Secret scanners cannot know it is disposable or fake. The tk_live_ prefix makes it look like a production credential and may trigger alerts or block the PR.
  • If it were real, Git history would retain it even after this branch or file is deleted, requiring immediate revocation and history cleanup.
  • The endpoint exposes part of it through INTEGRATION_TOKEN[:7], unnecessarily leaking credential metadata and normalizing unsafe handling.
  • Hardcoding encourages accidental reuse or replacement with a real token later.

Since this endpoint only needs a smoke-test response, remove the token entirely and return a non-secret marker:

@router.get("/ping")
def ping() -> dict[str, str]:
    return {"status": "ok", "marker": "dry-run"}

If testing configuration loading is required, read the token from environment-backed settings and return only whether it is configured—not any portion of its value.

@SerhiiYakovenko

Copy link
Copy Markdown
Owner Author

Dry run complete: automatic trigger after Update branch, slash-command path, rule injection, improve and ask all verified against v0.45.0. Closing and deleting the branch.

@SerhiiYakovenko
SerhiiYakovenko deleted the dryrun/smoke branch September 12, 2026 21:57
@SerhiiYakovenko
SerhiiYakovenko restored the dryrun/smoke branch September 12, 2026 22:25
@SerhiiYakovenko
SerhiiYakovenko deleted the dryrun/smoke branch September 12, 2026 22:26
Repository owner deleted a comment from github-actions Bot Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant