Skip to content

🛡️ Sentinel: [CRITICAL] Fix hardcoded staff password#42

Open
LVT-ENG wants to merge 8 commits intomainfrom
jules-8424711469455478886-e92ce9eb
Open

🛡️ Sentinel: [CRITICAL] Fix hardcoded staff password#42
LVT-ENG wants to merge 8 commits intomainfrom
jules-8424711469455478886-e92ce9eb

Conversation

@LVT-ENG
Copy link
Copy Markdown
Member

@LVT-ENG LVT-ENG commented Mar 29, 2026

  • Analyze existing code and test files
  • Fix test_staff.py: replace import main with import backend.main so monkeypatch correctly patches the module the endpoint reads
  • Remove raising=False from monkeypatch.setattr calls
  • Use a generic test password (test_staff_password_123) to avoid leaking format hints about the real password
  • Add STAFF_PASSWORD to .env.example
  • All 3 tests pass

- Removed hardcoded 'SAC_MUSEUM_2026' password from client-side `js/main.js`.
- Implemented secure backend verification endpoint `/api/verify-staff` in `backend/main.py`.
- Added `StaffLogin` model to `backend/models.py`.
- Password comparison now uses `hmac.compare_digest` to prevent timing attacks.
- Staff password is now configurable via the `STAFF_PASSWORD` environment variable.
- Added comprehensive integration tests in `backend/tests/test_staff.py`.
- Verified UI flow with automated Playwright verification.

Co-authored-by: LVT-ENG <214667862+LVT-ENG@users.noreply.github.com>
@google-labs-jules
Copy link
Copy Markdown
Contributor

👋 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
Copy link
Copy Markdown

vercel bot commented Mar 29, 2026

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

Project Deployment Actions Updated (UTC)
_deploy_build Ready Ready Preview, Comment Mar 29, 2026 4:56am
tryonyou-org Ready Ready Preview, Comment Mar 29, 2026 4:56am

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request migrates staff password verification from the frontend to a new backend API endpoint /api/verify-staff. The implementation includes a StaffLogin model, secure password comparison using hmac.compare_digest, and a Vite proxy configuration. Feedback identifies a critical security risk regarding the hardcoded default password in the backend and suggests using monkeypatch in tests to avoid hardcoding credentials.

Copy link
Copy Markdown
Member Author

@LVT-ENG LVT-ENG left a comment

Choose a reason for hiding this comment

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

Yes

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@LVT-ENG
Copy link
Copy Markdown
Member Author

LVT-ENG commented Mar 31, 2026

Yes

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR moves the staff “private pass” verification from a hardcoded frontend string to a backend-validated endpoint, so the password is no longer exposed in client-side JavaScript.

Changes:

  • Added a new FastAPI endpoint (POST /api/verify-staff) to validate staff credentials server-side.
  • Refactored the frontend staff check to call the backend endpoint via fetch.
  • Added a Vite dev proxy for /api to forward requests to the backend during local development, plus new backend tests for the staff verification endpoint.

Reviewed changes

Copilot reviewed 5 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
vite.config.js Adds dev-time /api proxy to the FastAPI server.
js/main.js Replaces hardcoded staff password check with an async call to /api/verify-staff.
backend/main.py Introduces /api/verify-staff and reads STAFF_PASSWORD from env.
backend/models.py Adds StaffLogin request model for the staff verification endpoint.
backend/tests/test_staff.py Adds tests intended to cover staff verification success/failure cases.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Member Author

@LVT-ENG LVT-ENG left a comment

Choose a reason for hiding this comment

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

Uds

@LVT-ENG
Copy link
Copy Markdown
Member Author

LVT-ENG commented Apr 5, 2026

@gemini reviuw @copilot @Julees

LVT-ENG and others added 6 commits April 5, 2026 10:39
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…SWORD to .env.example

Agent-Logs-Url: https://github.com/Tryonme-com/-TRYONYOU2/sessions/e1359dae-0604-4ed4-97ba-aecb7bb775ae

Co-authored-by: LVT-ENG <214667862+LVT-ENG@users.noreply.github.com>
Copy link
Copy Markdown
Member Author

@LVT-ENG LVT-ENG left a comment

Choose a reason for hiding this comment

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

Yes

@LVT-ENG
Copy link
Copy Markdown
Member Author

LVT-ENG commented Apr 5, 2026

@copilot

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants