Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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