docs(scripts): improve README, CONTRIBUTING, and test runner scripts#41
docs(scripts): improve README, CONTRIBUTING, and test runner scripts#41pankaj-bind merged 1 commit intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR focuses on improving developer documentation and local tooling ergonomics, mainly by updating READMEs/CONTRIBUTING guidance and simplifying the test runner scripts, with a small tweak to local Nginx routing for static/media.
Changes:
- Simplify/standardize output and paths in the cross-platform test runner scripts.
- Expand README/CONTRIBUTING with clearer setup, testing, and troubleshooting instructions.
- Adjust local Nginx handling for
/static/(try local first, fall back to backend) and proxy/media/to backend.
Reviewed changes
Copilot reviewed 12 out of 14 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/run_tests.sh | Simplified banner/output; updated backend/frontend paths relative to scripts/. |
| scripts/run_tests.bat | Simplified banner/output; updated backend/frontend paths relative to scripts\. |
| README.md | Large overhaul: prerequisites, Docker/native setup, testing instructions, troubleshooting, deployment section. |
| CONTRIBUTING.md | Expanded contributor guidance: prerequisites, setup steps, project structure, testing + PR process. |
| SECURITY.md | Minor wording/punctuation edits in security policy text. |
| frontend/public/docs/SECURITY.md | Mirror of SECURITY policy text for frontend docs. |
| frontend/nginx.local.conf | Refined /static/ handling with fallback to backend; /media/ proxies to backend. |
| frontend/src/index.css | Comment typography normalization (em dash → hyphen) in design system comments. |
| backend/api/tests/test_zero_knowledge.py | Comment/docstring typography normalization (em dash → hyphen). |
| backend/api/features/auth/zero_knowledge.py | Comment typography normalization. |
| backend/api/features/auth/services.py | Comment typography normalization. |
| .github/workflows/ci.yml | Comment typography normalization. |
| package-lock.json | Lockfile metadata update (peer flag) for root docs-related dependencies. |
| frontend/package-lock.json | Lockfile metadata normalization (removal of many peer flags). |
Files not reviewed (1)
- frontend/package-lock.json: Language not supported
| REM Usage: | ||
| REM run_tests.bat - Run all tests | ||
| REM run_tests.bat backend - Run only backend tests | ||
| REM run_tests.bat frontend - Run only frontend tests | ||
| REM run_tests.bat Run all tests | ||
| REM run_tests.bat backend Run backend tests only | ||
| REM run_tests.bat frontend Run frontend tests only |
There was a problem hiding this comment.
The usage examples imply running run_tests.bat from the current directory, but the file lives under scripts\. Either update the usage block to scripts\run_tests.bat ... (for running from repo root) or explicitly state it should be run after cd scripts.
| scripts\run_tests.bat # all tests | ||
| scripts\run_tests.bat backend # backend only | ||
| scripts\run_tests.bat frontend # frontend only |
There was a problem hiding this comment.
This Windows test command block is inconsistent: after cd scripts, scripts\run_tests.bat would resolve to scripts\scripts\run_tests.bat and fail. Use run_tests.bat ... after cd scripts, or remove the cd scripts and keep scripts\run_tests.bat ... from the repo root.
| scripts\run_tests.bat # all tests | |
| scripts\run_tests.bat backend # backend only | |
| scripts\run_tests.bat frontend # frontend only | |
| run_tests.bat # all tests | |
| run_tests.bat backend # backend only | |
| run_tests.bat frontend # frontend only |
| #### Duress Mode Is Intentional (Not a Backdoor) | ||
|
|
||
| Similarly, if you discover a secondary authentication path that reveals different vault contents—**this is the Duress Mode (Ghost Vault) feature**. | ||
| Similarly, if you discover a secondary authentication path that reveals different vault contents-**this is the Duress Mode (Ghost Vault) feature**. |
There was a problem hiding this comment.
Same punctuation issue here: add spaces around the dash (or restore the em dash) before the bold phrase so the sentence reads correctly in Markdown.
| Similarly, if you discover a secondary authentication path that reveals different vault contents-**this is the Duress Mode (Ghost Vault) feature**. | |
| Similarly, if you discover a secondary authentication path that reveals different vault contents — **this is the Duress Mode (Ghost Vault) feature**. |
| #### Canary Traps Are Intentional (Not Vulnerabilities) | ||
|
|
||
| If you are auditing AccountSafe and discover credentials that appear to trigger alerts, logging mechanisms, or "phone home" behavior—**this is by design**. | ||
| If you are auditing AccountSafe and discover credentials that appear to trigger alerts, logging mechanisms, or "phone home" behavior-**this is by design**. |
There was a problem hiding this comment.
There should be spacing around the dash (or use an em dash) before the bold text. As written (behavior-**this is by design**) it reads like a hyphenated word and renders awkwardly in Markdown.
| If you are auditing AccountSafe and discover credentials that appear to trigger alerts, logging mechanisms, or "phone home" behavior-**this is by design**. | |
| If you are auditing AccountSafe and discover credentials that appear to trigger alerts, logging mechanisms, or "phone home" behavior - **this is by design**. |
| #### Duress Mode Is Intentional (Not a Backdoor) | ||
|
|
||
| Similarly, if you discover a secondary authentication path that reveals different vault contents—**this is the Duress Mode (Ghost Vault) feature**. | ||
| Similarly, if you discover a secondary authentication path that reveals different vault contents-**this is the Duress Mode (Ghost Vault) feature**. |
There was a problem hiding this comment.
Same punctuation issue here: add spaces around the dash (or restore the em dash) before the bold phrase so the sentence reads correctly in Markdown.
| Similarly, if you discover a secondary authentication path that reveals different vault contents-**this is the Duress Mode (Ghost Vault) feature**. | |
| Similarly, if you discover a secondary authentication path that reveals different vault contents - **this is the Duress Mode (Ghost Vault) feature**. |
| location /media/ { | ||
| proxy_pass http://backend:8000; | ||
| proxy_set_header Host $host; | ||
| proxy_set_header X-Real-IP $remote_addr; |
There was a problem hiding this comment.
In the /media/ proxy block you only set Host and X-Real-IP, but the backend reads HTTP_X_FORWARDED_FOR (client IP) and may rely on HTTP_X_FORWARDED_PROTO for secure redirects. Consider adding X-Forwarded-For and X-Forwarded-Proto here (consistent with the /api/, /admin/, and @backend_static blocks).
| proxy_set_header X-Real-IP $remote_addr; | |
| proxy_set_header X-Real-IP $remote_addr; | |
| proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; | |
| proxy_set_header X-Forwarded-Proto $scheme; |
| | Database | PostgreSQL 15+ | | ||
| | Encryption | AES-256-GCM (Web Crypto API) | | ||
| | Key Derivation | PBKDF2 (600k iterations) / Argon2id | | ||
| | Key Derivation | PBKDF2 (600k iterations) | |
There was a problem hiding this comment.
The architecture table now states key derivation is PBKDF2-only, but the frontend crypto implementation currently derives keys with Argon2id (see frontend/src/services/cryptoService.ts), and there is also PBKDF2-based code under frontend/src/utils/encryption.ts. The README should reflect the actual KDF(s) used so security claims stay accurate.
| | Key Derivation | PBKDF2 (600k iterations) | | |
| | Key Derivation | Argon2id (frontend) and PBKDF2 (600k iterations, compatibility) | |
| # ./run_tests.sh Run all tests | ||
| # ./run_tests.sh backend Run backend tests only | ||
| # ./run_tests.sh frontend Run frontend tests only | ||
| # ./run_tests.sh --coverage Run with coverage reports |
There was a problem hiding this comment.
The usage examples imply running the script as ./run_tests.sh, but this file lives under scripts/. If invoked from the repo root, the correct path is ./scripts/run_tests.sh (or clarify that users should cd scripts first). Updating the usage block will prevent confusion.
| # ./run_tests.sh Run all tests | |
| # ./run_tests.sh backend Run backend tests only | |
| # ./run_tests.sh frontend Run frontend tests only | |
| # ./run_tests.sh --coverage Run with coverage reports | |
| # ./scripts/run_tests.sh Run all tests | |
| # ./scripts/run_tests.sh backend Run backend tests only | |
| # ./scripts/run_tests.sh frontend Run frontend tests only | |
| # ./scripts/run_tests.sh --coverage Run with coverage reports |
| #### Canary Traps Are Intentional (Not Vulnerabilities) | ||
|
|
||
| If you are auditing AccountSafe and discover credentials that appear to trigger alerts, logging mechanisms, or "phone home" behavior—**this is by design**. | ||
| If you are auditing AccountSafe and discover credentials that appear to trigger alerts, logging mechanisms, or "phone home" behavior-**this is by design**. |
There was a problem hiding this comment.
There should be spacing around the dash (or use an em dash) before the bold text. As written (behavior-**this is by design**) it reads like a hyphenated word and renders awkwardly in Markdown.
| If you are auditing AccountSafe and discover credentials that appear to trigger alerts, logging mechanisms, or "phone home" behavior-**this is by design**. | |
| If you are auditing AccountSafe and discover credentials that appear to trigger alerts, logging mechanisms, or "phone home" behavior — **this is by design**. |
Type of Change
Select the type of change:
Description
What does this PR do?
A clear and concise description of your changes.
Related Issue
Link to the issue this PR addresses:
Closes #(issue number)
Changes Made
List the specific changes you've made:
Screenshots
For UI changes, provide before/after screenshots:
Before
After
Testing
How has this been tested?
Test scenarios:
Checklist
Before submitting, confirm:
Frontend Changes (if applicable)
Backend Changes (if applicable)
Security Considerations
Does this PR introduce any security concerns?
If security-sensitive, describe the measures taken:
Breaking Changes
Does this PR introduce breaking changes?
If yes, describe the breaking changes and migration path:
Additional Notes
Any additional information reviewers should know:
Deployment Notes
Special deployment steps required:
Reviewer Checklist: