Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/security-scan.yml
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@

# Check package.json for issues
- name: Package.json security audit
uses: lirantal/lockfile-lint-action@master
uses: lirantal/lockfile-lint-action@v4.7.1

Check warning

Code scanning / CodeQL

Unpinned tag for a non-immutable Action in workflow Medium

Unpinned 3rd party Action 'Security Scan' step
Uses Step
uses 'lirantal/lockfile-lint-action' with ref 'v4.7.1', not a pinned commit hash
with:
path: package-lock.json
allowed-hosts: npm
Expand Down
21 changes: 0 additions & 21 deletions LICENSE.txt

This file was deleted.

7 changes: 0 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,6 @@ TourGuideAI/
│ ├── pics/ # Images for documentation
│ ├── prototype/ # Prototype data and mockups
│ └── project_lifecycle/ # Project management documentation
├── models/ # AI models and related resources
│ ├── data/ # Training data
│ ├── checkpoints/ # Model checkpoints
│ └── infra/ # Model infrastructure code
├── tourai_platform/ # TourAI platform specific code
│ ├── backend/ # Platform backend
│ └── frontend/ # Platform frontend
└── tools/ # Development and deployment tools
```

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# Migration Plan: Move from Create React App (CRA) to Vite or Next.js

## Rationale

Persistent security vulnerabilities in transitive dependencies (e.g., nth-check via react-scripts) cannot be resolved without breaking changes. CRA is no longer actively maintained for modern security needs. Migrating to Vite or Next.js will:
- Eliminate legacy dependency vulnerabilities
- Improve build speed and developer experience
- Enable more flexible configuration and modern features
- Align with project security and maintainability goals

## High-Level Steps

1. **Audit Current Dependencies and Features**
- List all dependencies, scripts, and custom configurations in the current CRA setup
- Identify any custom Webpack, Babel, or environment settings

2. **Select Target Build System**
- Evaluate Vite and Next.js for project fit (SSR, routing, etc.)
- Decide on Vite (for SPA) or Next.js (for SSR/SSG needs)

3. **Set Up New Project Structure**
- Initialize a new Vite or Next.js project in a separate branch or directory
- Configure TypeScript, ESLint, Prettier, and other tooling as needed

4. **Migrate Source Code**
- Copy src/ and public/ assets to the new project
- Update import paths, environment variables, and entry points as required
- Refactor any CRA-specific code (e.g., service worker, index.js)

5. **Migrate and Update Build/Test Scripts**
- Update package.json scripts for build, start, test, and deploy
- Remove react-scripts and related dependencies
- Ensure patch-package and overrides are no longer needed for nth-check

6. **Test Thoroughly**
- Run all unit, integration, and E2E tests
- Validate all features, routes, and environment configurations
- Fix any issues with static assets, routing, or environment variables

7. **Update Documentation**
- Document new build/test/deploy processes
- Update onboarding and developer guides
- Reference this migration in project.lessons.md and refactors.md

8. **Deploy and Monitor**
- Deploy to staging and production
- Monitor for regressions or new issues

## Risks and Mitigations

- **Dependency mismatches**: Audit and test all dependencies for compatibility
- **Build or runtime errors**: Use incremental migration and thorough testing
- **Team onboarding**: Update documentation and provide migration guides
- **CI/CD pipeline changes**: Update workflows and scripts for new build system

## References
- See Security & Build Lessons (2025-05-18) in project.lessons.md
- See TODO and Milestone entries for migration tracking
- See project.refactors.md for rationale and audit trail

---
*Last updated: 2025-05-18*
Original file line number Diff line number Diff line change
Expand Up @@ -806,4 +806,50 @@ Future refactorings should follow these guidelines, based on our [Code Review Ch
7. **Performance**: Measure and maintain or improve performance characteristics
8. **Code Health**: Every refactoring should improve the overall health of the codebase

Each refactoring record should document impacts across these dimensions to provide a complete picture of the changes made.

## Security and Build Fixes (2025-05-18)
**Type: Security, Build, Code Health**

### Summary
Addressed multiple security and build issues identified by automated scans and manual review. Implemented dependency overrides, improved file system safety, prevented prototype pollution, and ensured CI stability.

### Issues Addressed
- **Dependabot nth-check/react-scripts**: Documented the unfixable vulnerability due to upstream lock. Added monitoring note in package.json. Now using patch-package to track local awareness and maintain a patch for audit purposes.
- **PostCSS Vulnerability**: Forced postcss to ^8.4.31 using npm overrides in package.json.
- **File System Race Condition**: Refactored scripts and vaultService to use atomic file operations and try-catch, avoiding TOCTOU vulnerabilities.
- **User-Controlled Bypass of Security Check**: Audited permission checks to ensure only server-validated user context is used; no direct user input in permission logic.
- **Prototype Pollution**: Added property name validation in tokenProvider.js to prevent remote property injection.
- **lockfile-lint-action Pinning**: Updated GitHub Actions workflow to use a specific version for security scan stability.
- **AnalyticsService.js Build Error**: Reviewed and confirmed no syntax error; code is valid.

### Modified Files
- package.json (overrides, documentation)
- .github/workflows/security-scan.yml (action pinning)
- scripts/utils/test-script-template.js (atomic file ops)
- scripts/generate-keys.js (atomic file ops)
- server/utils/vaultService.js (atomic file ops, doc comment)
- server/utils/tokenProvider.js (prototype pollution prevention)
- src/features/beta-program/services/analytics/AnalyticsService.js (build error review)
- patches/nth-check+2.1.1.patch (local vulnerability monitoring)

### Code Health Impact
- **Positive**: Improved security posture and file operation safety
- **Positive**: Reduced risk of prototype pollution and race conditions
- **Positive**: Ensured CI stability and clear documentation of dependency risks
- **Neutral**: nth-check issue remains due to upstream lock; documented for monitoring

## Review Guidelines for Future Refactorings

Future refactorings should follow these guidelines, based on our [Code Review Checklist](../../code_and_project_structure_refactors/references/code-review-checklist.md):

1. **Design**: Ensure architectural patterns are followed and components are properly decomposed
2. **Functionality**: Maintain or improve existing functionality while making structural changes
3. **Complexity**: Aim to reduce complexity rather than increase it
4. **Tests**: Update tests to reflect changes and ensure continued coverage
5. **Documentation**: Keep documentation in sync with code changes
6. **Security**: Consider security implications, especially for API changes
7. **Performance**: Measure and maintain or improve performance characteristics
8. **Code Health**: Every refactoring should improve the overall health of the codebase

Each refactoring record should document impacts across these dimensions to provide a complete picture of the changes made.
10 changes: 10 additions & 0 deletions docs/project_lifecycle/knowledge/project.lessons.md
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,16 @@
- **LESSON**: Build report generation scripts with extensibility in mind to accommodate new test categories
- **LESSON**: Implement proper error handling in test scripts to prevent misleading results when environment issues occur

## Security & Build Lessons (2025-05-18)
- **MUST-OBEY PRINCIPLE**: When a critical dependency vulnerability cannot be fixed due to upstream lock (e.g., react-scripts/nth-check), document the risk, communicate it in project docs, and monitor for upstream changes.
- **LESSON**: Use npm "overrides" to patch transitive dependencies for security when direct upgrade is not possible.
- **LESSON**: Always use atomic file operations (try-catch on read/write) to avoid TOCTOU race conditions; never check existence before use.
- **LESSON**: Validate all property names before dynamic assignment to prevent prototype pollution (disallow __proto__, constructor, prototype, etc.).
- **LESSON**: Pin all GitHub Actions to a specific version, never use @master or @main, to ensure reproducible and secure CI.
- **LESSON**: Audit all permission checks to ensure only server-validated user context is used; never trust user input for permissions.
- **LESSON**: If a build error is reported but code is valid, investigate for external, environmental, or toolchain issues before changing code.
- **LESSON**: Use patch-package to document and monitor unfixable vulnerabilities in transitive dependencies when upstream fixes are not available.

---

*Last Updated: May 15, 2025*
Loading
Loading