diff --git a/.github/agents/developer.agent.md b/.github/agents/developer.agent.md index 2028f79..d936129 100644 --- a/.github/agents/developer.agent.md +++ b/.github/agents/developer.agent.md @@ -16,7 +16,8 @@ Perform software development tasks by determining and applying appropriate DEMA 2. **Read relevant standards** from `.github/standards/` as defined in AGENTS.md based on work performed 3. **Apply loaded standards** throughout development process 4. **Execute work** following standards requirements and quality checks -5. **Generate completion report** with results and compliance status +5. **Lint fixes** follow the linting process before performing quality gates +6. **Generate completion report** with results and compliance status # Reporting diff --git a/.github/agents/implementation.agent.md b/.github/agents/implementation.agent.md index 91b44d7..35cc1c8 100644 --- a/.github/agents/implementation.agent.md +++ b/.github/agents/implementation.agent.md @@ -28,7 +28,7 @@ counting how many retries have occurred. Call the built-in explore sub-agent with: -- **context**: the user's request and any current quality findings +- **context**: the user's request + any previous quality findings + retry context - **goal**: analyze the implementation state and develop a plan to implement the request Once the explore sub-agent finishes, transition to the DEVELOPMENT state. @@ -37,7 +37,7 @@ Once the explore sub-agent finishes, transition to the DEVELOPMENT state. Call the developer sub-agent with: -- **context** the user's request and the current implementation plan +- **context** the user's request + research plan + specific quality issues to address (if retry) - **goal** implement the user's request and any identified quality fixes Once the developer sub-agent finishes: @@ -49,7 +49,7 @@ Once the developer sub-agent finishes: Call the quality sub-agent with: -- **context** the user's request and the current implementation report +- **context** the user's request + development summary + files changed + previous issues (if any) - **goal** check the quality of the work performed for any issues Once the quality sub-agent finishes: @@ -73,7 +73,7 @@ of the project consisting of: ## State Machine Execution - **Research Results**: [Summary of explore agent findings] -- **Development Results**: [Summary of developer agent results] +- **Development Results**: [Summary of developer agent results] - **Quality Results**: [Summary of quality agent results] - **State Transitions**: [Log of state changes and decisions] @@ -86,7 +86,7 @@ of the project consisting of: ## Final Status - **Implementation Success**: [Overall completion status] -- **Quality Compliance**: [Final quality validation status] +- **Quality Compliance**: [Final quality validation status] - **Issues Resolved**: [Problems encountered and resolution attempts] ``` diff --git a/.github/agents/quality.agent.md b/.github/agents/quality.agent.md index a7b57d4..691a17d 100644 --- a/.github/agents/quality.agent.md +++ b/.github/agents/quality.agent.md @@ -26,6 +26,13 @@ This assessment is a quality control system of the project and MUST be performed Upon completion create a summary in `.agent-logs/{agent-name}-{subject}-{unique-id}.md` of the project consisting of: +The **Result** field MUST reflect the quality validation outcome for orchestrator decision-making: + +- **Result: SUCCEEDED** - Only when Overall Grade is PASS (all compliance requirements met) +- **Result: FAILED** - When Overall Grade is FAIL or NEEDS_WORK (compliance failures present) + +This ensures orchestrators properly halt workflows when quality gates fail. + ```markdown # Quality Assessment Report @@ -98,6 +105,13 @@ of the project consisting of: - Do README.md files use absolute URLs and include concrete examples? (PASS|FAIL|N/A) - [Evidence] - Is documentation integrated into ReviewMark review-sets for formal review? (PASS|FAIL|N/A) - [Evidence] +## Software Item Completeness: (PASS|FAIL|N/A) + +- Does every identified software unit have its own requirements file? (PASS|FAIL|N/A) - [Evidence] +- Does every identified software unit have its own design document? (PASS|FAIL|N/A) - [Evidence] +- Does every identified subsystem have its own requirements file? (PASS|FAIL|N/A) - [Evidence] +- Does every identified subsystem have its own design document? (PASS|FAIL|N/A) - [Evidence] + ## Process Compliance: (PASS|FAIL|N/A) - Was Continuous Compliance workflow followed? (PASS|FAIL|N/A) - [Evidence] diff --git a/.github/standards/design-documentation.md b/.github/standards/design-documentation.md index 6312275..e14cd30 100644 --- a/.github/standards/design-documentation.md +++ b/.github/standards/design-documentation.md @@ -23,12 +23,13 @@ design to implementation: ```text docs/design/ -├── introduction.md # Design overview with software structure -├── system.md # System-level design documentation -├── {subsystem-name}/ # Subsystem design documents (kebab-case folder names) -│ ├── {subsystem-name}.md # Subsystem overview and design -│ └── {unit-name}.md # Unit-level design documents -└── {unit-name}.md # Top-level unit design documents (if not in subsystem) +├── introduction.md # Design overview with software structure +└── {system-name}/ # System-level design folder (one per system) + ├── {system-name}.md # System-level design documentation + ├── {subsystem-name}/ # Subsystem design documents (kebab-case folder names) + │ ├── {subsystem-name}.md # Subsystem overview and design + │ └── {unit-name}.md # Unit-level design documents + └── {unit-name}.md # Top-level unit design documents (if not in subsystem) ``` ## introduction.md (MANDATORY) @@ -56,13 +57,16 @@ to understand these classifications before creating this section. Example format: ```text -ProjectName (System) +Project1Name (System) ├── ComponentA (Subsystem) │ ├── ClassX (Unit) │ └── ClassY (Unit) ├── ComponentB (Subsystem) │ └── ClassZ (Unit) └── UtilityClass (Unit) + +Project2Name (System) +└── HelperClass (Unit) ``` ### Folder Layout Section (MANDATORY) @@ -73,24 +77,29 @@ mirror the software structure, with file paths and brief descriptions. Example format: ```text -src/ProjectName/ +src/Project1Name/ ├── ComponentA/ -│ ├── ClassX.cs — brief description -│ └── ClassY.cs — brief description +│ ├── ClassX.cs — Core business logic handler +│ └── ClassY.cs — Data validation service ├── ComponentB/ -│ └── ClassZ.cs — brief description -└── UtilityClass.cs — brief description +│ └── ClassZ.cs — Integration interface +└── UtilityClass.cs — Common utility functions + +src/Project2Name/ +└── HelperClass.cs — Helper functions ``` -## system.md (MANDATORY) +## System Design Documentation (MANDATORY) -The `system.md` file contains system-level design documentation including: +For each system identified in the repository: -- System architecture and major components -- External interfaces and dependencies -- Data flow and control flow -- System-wide design constraints and decisions -- Integration patterns and communication protocols +- Create a kebab-case folder matching the system name +- Include `{system-name}.md` with system-level design documentation such as: + - System architecture and major components + - External interfaces and dependencies + - Data flow and control flow + - System-wide design constraints and decisions + - Integration patterns and communication protocols ## Subsystem and Unit Design Documents @@ -98,9 +107,9 @@ For each subsystem identified in the software structure: - Create a kebab-case folder matching the subsystem name (enables automated tooling) - Include `{subsystem-name}.md` with subsystem overview and design -- Include unit design documents for complex units within the subsystem +- Include unit design documents for ALL units within the subsystem -For significant units requiring detailed design: +For every unit identified in the software structure: - Document data models, algorithms, and key methods - Describe interactions with other units @@ -124,8 +133,10 @@ implementation specification for formal code review: - **Implementation Detail**: Provide sufficient detail for code review and implementation - **Architectural Clarity**: Clearly define component boundaries and interfaces - **Traceability**: Link to requirements where applicable using ReqStream patterns -- **Concrete Examples**: Use actual class names, method signatures, and data structures -- **Current Information**: Keep synchronized with code changes and refactoring + +# Mermaid Diagram Integration + +Use Mermaid diagrams to supplement text descriptions (diagrams must not replace text content). # Quality Checks @@ -133,10 +144,10 @@ Before submitting design documentation, verify: - [ ] `introduction.md` includes both Software Structure and Folder Layout sections - [ ] Software structure correctly categorizes items as System/Subsystem/Unit per `software-items.md` -- [ ] Folder layout matches actual source code organization -- [ ] `system.md` provides comprehensive system-level design +- [ ] Folder layout mirrors software structure organization +- [ ] Design documents provide sufficient detail for code review +- [ ] System documentation provides comprehensive system-level design - [ ] Subsystem documentation folders use kebab-case names while mirroring source subsystem names and structure -- [ ] Design documents contain sufficient implementation detail - [ ] All documents follow technical documentation formatting standards - [ ] Content is current with implementation and requirements - [ ] Documents are integrated into ReviewMark review-sets for formal review diff --git a/.github/standards/reqstream-usage.md b/.github/standards/reqstream-usage.md index aa75a1f..bd8c739 100644 --- a/.github/standards/reqstream-usage.md +++ b/.github/standards/reqstream-usage.md @@ -30,15 +30,17 @@ the source code structure because reviewers need clear navigation from requirements to design to implementation: ```text -requirements.yaml # Root file (includes only) +requirements.yaml # Root file (includes only) docs/reqstream/ -├── system.yaml # System-level requirements -├── platform-requirements.yaml # Platform support requirements -├── {subsystem-name}/ # Subsystem requirements (kebab-case folders) -│ └── {subsystem-name}.yaml # Requirements for this subsystem -├── {unit-name}.yaml # Unit requirements (for top-level units) -└── ots/ # OTS software item requirements - └── {ots-name}.yaml # Requirements for OTS components +├── {system-name}/ # System-level requirements folder (one per system) +│ ├── {system-name}.yaml # System-level requirements +│ ├── platform-requirements.yaml # Platform support requirements +│ ├── {subsystem-name}/ # Subsystem requirements (kebab-case folders) +│ │ ├── {subsystem-name}.yaml # Requirements for this subsystem +│ │ └── {unit-name}.yaml # Requirements for units within this subsystem +│ └── {unit-name}.yaml # Requirements for top-level units (outside subsystems) +└── ots/ # OTS software items folder + └── {ots-name}.yaml # Requirements for OTS components ``` The folder structure MUST mirror the source code organization to maintain @@ -62,7 +64,7 @@ for compliance auditing. sections: - title: Functional Requirements requirements: - - id: Project-Subsystem-Feature + - id: System-Subsystem-Feature title: The system shall perform the required function. justification: | Business rationale explaining why this requirement exists. @@ -88,7 +90,7 @@ sections: sections: - title: System.Text.Json requirements: - - id: Project-SystemTextJson-ReadJson + - id: TemplateTool-SystemTextJson-ReadJson title: System.Text.Json shall be able to read JSON files. tests: - JsonReaderTests.TestReadValidJson @@ -96,7 +98,7 @@ sections: # Semantic IDs (MANDATORY) -Use meaningful IDs following `Project-Section-ShortDesc` pattern because +Use meaningful IDs following `System-Section-ShortDesc` pattern because auditors need to understand requirements without cross-referencing: - **Good**: `TemplateTool-Core-DisplayHelp` @@ -127,12 +129,6 @@ dotnet reqstream \ --requirements requirements.yaml \ --lint -# Enforce requirements traceability (use in CI/CD) -dotnet reqstream \ - --requirements requirements.yaml \ - --tests "artifacts/**/*.trx" \ - --enforce - # Generate requirements report dotnet reqstream \ --requirements requirements.yaml \ @@ -154,7 +150,7 @@ dotnet reqstream \ Before submitting requirements, verify: -- [ ] All requirements have semantic IDs (`Project-Section-Feature` pattern) +- [ ] All requirements have semantic IDs (`System-Section-Feature` pattern) - [ ] Every requirement links to at least one passing test - [ ] Platform-specific requirements use source filters (`platform@TestName`) - [ ] Requirements specify observable behavior (WHAT), not implementation (HOW) diff --git a/.github/standards/reviewmark-usage.md b/.github/standards/reviewmark-usage.md index a40179f..2fdaa19 100644 --- a/.github/standards/reviewmark-usage.md +++ b/.github/standards/reviewmark-usage.md @@ -1,16 +1,24 @@ -# ReviewMark File Review Standards +# ReviewMark Usage Standard -This document defines DEMA Consulting standards for managing file reviews using -ReviewMark within Continuous Compliance environments. +## Purpose -# Core Purpose +ReviewMark manages file review status enforcement and formal review processes. It tracks which files need +review, organizes them into review-sets, and generates review plans and reports. -ReviewMark automates file review tracking using cryptographic fingerprints to -ensure: +## Key Commands -- Every file requiring review is covered by a current, valid review -- Reviews become stale when files change, triggering re-review -- Complete audit trail of review coverage for regulatory compliance +- **Lint Configuration**: `dotnet reviewmark --lint` +- **Elaborate Review-Set**: `dotnet reviewmark --elaborate [review-set]` +- **Generate Plan**: `dotnet reviewmark --plan docs/code_review_plan/plan.md` +- **Generate Report**: `dotnet reviewmark --report docs/code_review_report/report.md` + +## Repository Structure + +Required repository items for ReviewMark operation: + +- `.reviewmark.yaml` - Configuration for review-sets, file-patterns, and review evidence-source. +- `docs/code_review_plan/` - Review planning artifacts +- `docs/code_review_report/` - Review status reports # Review Definition Structure @@ -19,36 +27,25 @@ Configure reviews in `.reviewmark.yaml` at repository root: ```yaml # Patterns identifying all files that require review needs-review: - # Include core development artifacts - - "requirements.yaml" # Root requirements file - - "docs/reqstream/**/*.yaml" # Requirements files - - "docs/design/**/*.md" # Design documentation - - "**/*.cs" # All C# source and test files - - # Exclude build output and generated content - - "!**/obj/**" # Exclude build output - - "!**/bin/**" # Exclude binary output - - "!**/generated/**" # Exclude auto-generated files + # Include source code (adjust file extensions for your repo) + - "**/*.cs" # C# source files + - "**/*.cpp" # C++ source files + - "**/*.hpp" # C++ header files + - "!**/bin/**" # Generated source in build outputs + - "!**/obj/**" # Generated source in build intermediates + + # Include requirement files + - "requirements.yaml" # Root requirements file + - "docs/reqstream/**/*.yaml" # Requirements files + + # Include critical documentation files + - "README.md" # Root level README + - "docs/user_guide/**/*.md" # User guide + - "docs/design/**/*.md" # Design documentation # Source of review evidence evidence-source: type: none - -# Named review-sets grouping related files -reviews: - - id: MyProduct-PasswordValidator - title: Password Validator Unit Review - paths: - - "docs/reqstream/authentication/password-validator.yaml" - - "docs/design/authentication/password-validator.md" - - "src/{ProjectName}/Authentication/PasswordValidator.cs" - - "test/{ProjectName}.Tests/Authentication/PasswordValidatorTests.cs" - - - id: MyProduct-AllRequirements - title: All Requirements Review - paths: - - "requirements.yaml" - - "docs/reqstream/**/*.yaml" ``` # Review-Set Organization @@ -56,93 +53,56 @@ reviews: Organize review-sets using standard patterns to ensure comprehensive coverage and consistent review processes: -## [Project]-System Review +## [System]-Architecture Review (one per system) -Reviews system integration and operational validation: +Reviews system architecture and operational validation: -- **Files**: System requirements (`docs/reqstream/system.yaml`), design introduction - (`docs/design/introduction.md`), system design (`docs/design/system.md`), +- **Files**: System requirements (`docs/reqstream/{system-name}/{system-name}.yaml`), design introduction + (`docs/design/introduction.md`), system design (`docs/design/{system-name}/{system-name}.md`), integration tests - **Purpose**: Validates system operates as designed and meets overall requirements -- **Example**: `TemplateTool-System` +- **Example**: `SomeSystem-Architecture` -## [Product]-Design Review +## [System]-Design Review Reviews architectural and design consistency: - **Files**: System requirements, platform requirements, all design documents under `docs/design/` - **Purpose**: Ensures design completeness and architectural coherence -- **Example**: `MyProduct-Design` +- **Example**: `SomeSystem-Design` -## [Product]-AllRequirements Review +## [System]-AllRequirements Review Reviews requirements quality and traceability: -- **Files**: All requirement files including root `requirements.yaml` and all files under `docs/reqstream/` +- **Files**: All requirement files including root `requirements.yaml` and all files under `docs/reqstream/{system-name}/` - **Purpose**: Validates requirements structure, IDs, justifications, and test linkage -- **Example**: `MyProduct-AllRequirements` - -## [Product]-[Unit] Review - -Reviews individual software unit implementation: - -- **Files**: Unit requirements, design documents, source code, unit tests -- **Purpose**: Validates unit meets requirements and is properly implemented -- **File Path Pattern**: - - Requirements: `docs/reqstream/{subsystem-name}/{unit-name}.yaml` or `docs/reqstream/{unit-name}.yaml` - - Design: `docs/design/{subsystem-name}/{unit-name}.md` or `docs/design/{unit-name}.md` - - Source: `src/{ProjectName}/{SubsystemName}/{UnitName}.cs` - - Tests: `test/{ProjectName}.Tests/{SubsystemName}/{UnitName}Tests.cs` -- **Example**: `MyProduct-PasswordValidator`, `MyProduct-ConfigParser` +- **Example**: `SomeSystem-AllRequirements` -## [Product]-[Subsystem] Review +## [System]-[Subsystem] Review Reviews subsystem architecture and interfaces: - **Files**: Subsystem requirements, design documents, integration tests (usually no source code) - **Purpose**: Validates subsystem behavior and interface compliance - **File Path Pattern**: - - Requirements: `docs/reqstream/{subsystem-name}/{subsystem-name}.yaml` - - Design: `docs/design/{subsystem-name}/{subsystem-name}.md` - - Tests: `test/{ProjectName}.Tests/{SubsystemName}Integration/` or similar -- **Example**: `MyProduct-Authentication`, `MyProduct-DataLayer` + - Requirements: `docs/reqstream/{system-name}/{subsystem-name}/{subsystem-name}.yaml` + - Design: `docs/design/{system-name}/{subsystem-name}/{subsystem-name}.md` + - Tests: `test/{SystemName}.Tests/{SubsystemName}/{SubsystemName}*` or similar +- **Example**: `SomeSystem-Authentication`, `SomeSystem-DataLayer` -## [Product]-OTS Review +## [System]-[Subsystem]-[Unit] Review -Reviews OTS (Off-The-Shelf) software integration: +Reviews individual software unit implementation: -- **Files**: OTS requirements and integration test evidence -- **Purpose**: Validates OTS components meet integration requirements +- **Files**: Unit requirements, design documents, source code, unit tests +- **Purpose**: Validates unit meets requirements and is properly implemented - **File Path Pattern**: - - Requirements: `docs/reqstream/ots/{ots-name}.yaml` - - Tests: Integration tests proving OTS functionality -- **Example**: `MyProduct-SystemTextJson`, `MyProduct-EntityFramework` - -# File Pattern Best Practices - -Use "include-then-exclude" approach for `needs-review` patterns because it -ensures comprehensive coverage while removing unwanted files: - -1. **Start broad**: Include all files of potential interest with generous patterns -2. **Exclude overreach**: Use `!` patterns to remove build output, generated files, and temporary files -3. **Test patterns**: Verify patterns match intended files using `dotnet reviewmark --elaborate` - -**Order matters**: Patterns are processed sequentially, excludes override earlier includes. - -# ReviewMark Commands - -Essential ReviewMark commands for Continuous Compliance: - -```bash -# Lint review configuration for issues (run before use) -dotnet reviewmark --lint - -# Generate review plan and report (use in CI/CD) -dotnet reviewmark \ - --plan docs/code_review_plan/plan.md \ - --report docs/code_review_report/report.md \ - --enforce -``` + - Requirements: `docs/reqstream/{system-name}/{subsystem-name}/{unit-name}.yaml` or `docs/reqstream/{system-name}/{unit-name}.yaml` + - Design: `docs/design/{system-name}/{subsystem-name}/{unit-name}.md` or `docs/design/{system-name}/{unit-name}.md` + - Source: `src/{SystemName}/{SubsystemName}/{UnitName}.cs` + - Tests: `test/{SystemName}.Tests/{SubsystemName}/{UnitName}Tests.cs` +- **Example**: `SomeSystem-Authentication-PasswordValidator`, `SomeSystem-DataLayer-ConfigParser` # Quality Checks @@ -155,6 +115,5 @@ Before submitting ReviewMark configuration, verify: - [ ] File paths reflect current naming conventions (kebab-case design/requirements folders, PascalCase source folders) - [ ] Evidence source properly configured (`none` for dev, `url` for production) - [ ] Environment variables used for credentials (never hardcoded) -- [ ] ReviewMark enforcement configured: `dotnet reviewmark --enforce` - [ ] Generated documents accessible for compliance auditing -- [ ] Review-set organization follows standard patterns ([Product]-[Unit], [Product]-Design, etc.) +- [ ] Review-set organization follows standard patterns ([System]-[Subsystem], [System]-Design, etc.) diff --git a/.github/standards/technical-documentation.md b/.github/standards/technical-documentation.md index c117aa2..5bcc937 100644 --- a/.github/standards/technical-documentation.md +++ b/.github/standards/technical-documentation.md @@ -24,36 +24,39 @@ consistency and tool compatibility: ```text docs/ - build_notes.md # Generated by BuildMark - build_notes/ # Auto-generated build notes - versions.md # Generated by VersionMark - code_review_plan/ # Auto-generated review plans - plan.md # Generated by ReviewMark - code_review_report/ # Auto-generated review reports - report.md # Generated by ReviewMark - design/ # Design documentation - introduction.md # Design overview - system.md # System architecture - {subsystem-name}/ # Subsystem design folder - {subsystem-name}.md # Subsystem-specific designs - {unit-name}.md # Unit-specific designs - {unit-name}.md # Top-level unit design - reqstream/ # Requirements source files - system.yaml # System requirements - platform-requirements.yaml # Platform requirements - {subsystem-name}/ # Subsystem requirements folder - {subsystem-name}.yaml # Subsystem requirements - {unit-name}.yaml # Unit requirements - ots/ # OTS requirement files - {ots-name}.yaml # OTS requirements - requirements_doc/ # Auto-generated requirements reports - requirements.md # Generated by ReqStream - justifications.md # Generated by ReqStream - requirements_report/ # Auto-generated trace matrices - trace_matrix.md # Generated by ReqStream - user_guide/ # User-facing documentation - introduction.md # User guide overview - {section}.md # User guide sections + build_notes.md # Generated by BuildMark + build_notes/ # Auto-generated build notes + versions.md # Generated by VersionMark + code_review_plan/ # Auto-generated review plans + plan.md # Generated by ReviewMark + code_review_report/ # Auto-generated review reports + report.md # Generated by ReviewMark + design/ # Design documentation + introduction.md # Design overview + {system-name}/ # System architecture folder + {system-name}.md # System architecture + {subsystem-name}/ # Subsystem design folder + {subsystem-name}.md # Subsystem-specific designs + {unit-name}.md # Unit-specific designs + {unit-name}.md # Top-level unit design + reqstream/ # Requirements source files + {system-name}/ # System requirements folder + {system-name}.yaml # System requirements + platform-requirements.yaml # Platform requirements + {subsystem-name}/ # Subsystem requirements folder + {subsystem-name}.yaml # Subsystem requirements + {unit-name}.yaml # Unit-specific requirements + {unit-name}.yaml # Top-level unit requirements + ots/ # OTS requirement files + {ots-name}.yaml # OTS requirements + requirements_doc/ # Auto-generated requirements reports + requirements.md # Generated by ReqStream + justifications.md # Generated by ReqStream + requirements_report/ # Auto-generated trace matrices + trace_matrix.md # Generated by ReqStream + user_guide/ # User-facing documentation + introduction.md # User guide overview + {section}.md # User guide sections ``` # Pandoc Document Structure (MANDATORY) diff --git a/.reviewmark.yaml b/.reviewmark.yaml index 7c243cd..1143e73 100644 --- a/.reviewmark.yaml +++ b/.reviewmark.yaml @@ -6,12 +6,15 @@ # Patterns identifying all files that require review. # Processed in order; prefix a pattern with '!' to exclude. needs-review: - - "**/*.cs" # All C# source and test files - - "requirements.yaml" # Root requirements file - - "docs/reqstream/**/*.yaml" # Requirements files - - "docs/design/**/*.md" # Design documents - - "!**/obj/**" # Exclude build output - - "!**/bin/**" # Exclude build output + - "**/*.cs" # All C# test files + - "!src/**/*.cs" # Source code reviewed through code review process + - "requirements.yaml" # Root requirements file + - "docs/reqstream/**/*.yaml" # Requirements files + - "docs/design/**/*.md" # Design documents + - "README.md" # Root level README + - "docs/user_guide/**/*.md" # User guide + - "!**/obj/**" # Exclude build output + - "!**/bin/**" # Exclude build output # Evidence source: review data and index.json are located in the 'reviews' branch # of this repository, accessed through the GitHub public HTTPS raw content access. @@ -20,21 +23,23 @@ evidence-source: location: https://raw.githubusercontent.com/demaconsulting/SarifMark/reviews/index.json # Review sets grouping files by logical unit of review. -# Subsystem reviews cover the externally-visible behaviour of each feature subsystem. -# Software unit reviews cover the internal design and implementation of each individual class. +# Subsystem reviews cover the externally-visible behaviour of each feature subsystem: +# subsystem requirements + subsystem design + subsystem test suite. +# Software unit reviews cover the internal design of each individual class: +# unit requirements + unit design + unit test suite. reviews: # --------------------------------------------------------------------------- - # System Review + # Architecture Review # --------------------------------------------------------------------------- - - id: SarifMark-System - title: Review of SarifMark System + - id: SarifMark-Architecture + title: Review of SarifMark Architecture paths: - - "docs/reqstream/system.yaml" - - "docs/reqstream/platform-requirements.yaml" + - "docs/reqstream/sarifmark/sarifmark.yaml" + - "docs/reqstream/sarifmark/platform-requirements.yaml" - "docs/design/introduction.md" - - "docs/design/system.md" + - "docs/design/sarifmark/sarifmark.md" - "test/**/IntegrationTests.cs" - "test/**/Runner.cs" - "test/**/AssemblyInfo.cs" @@ -56,8 +61,9 @@ reviews: - id: SarifMark-Design title: Review of SarifMark Design Documentation paths: - - "docs/reqstream/system.yaml" - - "docs/reqstream/platform-requirements.yaml" + - "README.md" + - "docs/reqstream/sarifmark/sarifmark.yaml" + - "docs/reqstream/sarifmark/platform-requirements.yaml" - "docs/design/**/*.md" # --------------------------------------------------------------------------- @@ -67,43 +73,31 @@ reviews: - id: SarifMark-Cli title: Review of SarifMark Command-Line Interface Subsystem paths: - - "docs/reqstream/cli/subsystem-cli.yaml" - - "docs/design/cli/cli.md" - - "docs/design/cli/context.md" - - "src/**/Cli/Context.cs" - - "test/**/Cli/ContextTests.cs" + - "docs/reqstream/sarifmark/cli/cli.yaml" + - "docs/design/sarifmark/cli/cli.md" + - "test/**/Cli/CliTests.cs" - id: SarifMark-Sarif title: Review of SarifMark SARIF Reading Subsystem paths: - - "docs/reqstream/sarif/subsystem-sarif.yaml" - - "docs/reqstream/sarif/subsystem-report.yaml" - - "docs/design/sarif/sarif.md" - - "docs/design/sarif/sarif-result.md" - - "docs/design/sarif/sarif-results.md" - - "src/**/Sarif/SarifResult.cs" - - "src/**/Sarif/SarifResults.cs" - - "test/**/Sarif/SarifResultsTests.cs" - - "test/**/IntegrationTests.cs" + - "docs/reqstream/sarifmark/sarif/sarif.yaml" + - "docs/reqstream/sarifmark/sarif/report.yaml" + - "docs/design/sarifmark/sarif/sarif.md" + - "test/**/Sarif/SarifTests.cs" - id: SarifMark-SelfTest title: Review of SarifMark Self-Validation Subsystem paths: - - "docs/reqstream/self-test/subsystem-self-test.yaml" - - "docs/design/self-test/self-test.md" - - "docs/design/self-test/validation.md" - - "src/**/SelfTest/Validation.cs" - - "test/**/SelfTest/ValidationTests.cs" - - "test/**/IntegrationTests.cs" + - "docs/reqstream/sarifmark/self-test/self-test.yaml" + - "docs/design/sarifmark/self-test/self-test.md" + - "test/**/SelfTest/SelfTestTests.cs" - id: SarifMark-Utilities title: Review of SarifMark Utilities Subsystem paths: - - "docs/reqstream/utilities/subsystem-utilities.yaml" - - "docs/design/utilities/utilities.md" - - "docs/design/utilities/path-helpers.md" - - "src/**/Utilities/PathHelpers.cs" - - "test/**/Utilities/PathHelpersTests.cs" + - "docs/reqstream/sarifmark/utilities/utilities.yaml" + - "docs/design/sarifmark/utilities/utilities.md" + - "test/**/Utilities/UtilitiesTests.cs" # --------------------------------------------------------------------------- # Software Unit Reviews @@ -112,47 +106,41 @@ reviews: - id: SarifMark-Program title: Review of SarifMark Program Software Unit paths: - - "docs/reqstream/unit-program.yaml" - - "docs/design/program.md" - - "src/**/Program.cs" + - "docs/reqstream/sarifmark/program.yaml" + - "docs/design/sarifmark/program.md" - "test/**/ProgramTests.cs" - - id: SarifMark-Context + - id: SarifMark-Cli-Context title: Review of SarifMark Context Software Unit paths: - - "docs/reqstream/cli/unit-context.yaml" - - "docs/design/cli/context.md" - - "src/**/Cli/Context.cs" + - "docs/reqstream/sarifmark/cli/context.yaml" + - "docs/design/sarifmark/cli/context.md" - "test/**/Cli/ContextTests.cs" - - id: SarifMark-SarifResult + - id: SarifMark-Sarif-SarifResult title: Review of SarifMark SarifResult Software Unit paths: - - "docs/reqstream/sarif/unit-sarif-result.yaml" - - "docs/design/sarif/sarif-result.md" - - "src/**/Sarif/SarifResult.cs" + - "docs/reqstream/sarifmark/sarif/sarif-result.yaml" + - "docs/design/sarifmark/sarif/sarif-result.md" - "test/**/Sarif/SarifResultsTests.cs" - - id: SarifMark-SarifResults + - id: SarifMark-Sarif-SarifResults title: Review of SarifMark SarifResults Software Unit paths: - - "docs/reqstream/sarif/unit-sarif-results.yaml" - - "docs/design/sarif/sarif-results.md" - - "src/**/Sarif/SarifResults.cs" + - "docs/reqstream/sarifmark/sarif/sarif-results.yaml" + - "docs/design/sarifmark/sarif/sarif-results.md" - "test/**/Sarif/SarifResultsTests.cs" - - id: SarifMark-Validation + - id: SarifMark-SelfTest-Validation title: Review of SarifMark Validation Software Unit paths: - - "docs/reqstream/self-test/unit-validation.yaml" - - "docs/design/self-test/validation.md" - - "src/**/SelfTest/Validation.cs" + - "docs/reqstream/sarifmark/self-test/validation.yaml" + - "docs/design/sarifmark/self-test/validation.md" - "test/**/SelfTest/ValidationTests.cs" - - id: SarifMark-PathHelpers + - id: SarifMark-Utilities-PathHelpers title: Review of SarifMark PathHelpers Software Unit paths: - - "docs/reqstream/utilities/unit-path-helpers.yaml" - - "docs/design/utilities/path-helpers.md" - - "src/**/Utilities/PathHelpers.cs" + - "docs/reqstream/sarifmark/utilities/path-helpers.yaml" + - "docs/design/sarifmark/utilities/path-helpers.md" - "test/**/Utilities/PathHelpersTests.cs" diff --git a/AGENTS.md b/AGENTS.md index c884c2e..87fc5c7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,7 +2,43 @@ Comprehensive guidance for AI agents working on repositories following Continuous Compliance practices. -## Standards Application (ALL Agents Must Follow) +# Project Structure + +The following is the basic folder structure of the project. Agents should use this information when searching for +existing files and to know where to make new files. + +```text +├── docs/ +│ ├── build_notes/ +│ ├── code_quality/ +│ ├── code_review_plan/ +│ ├── code_review_report/ +│ ├── design/ +│ ├── requirements_doc/ +│ ├── requirements_report/ +│ └── reqstream/ +├── src/ +│ └── / +└── test/ + └── / +``` + +# Key Configuration Files + +- **`.config/dotnet-tools.json`** - Local tool manifest for Continuous Compliance tools +- **`.editorconfig`** - Code formatting rules +- **`.clang-format`** - C/C++ formatting (if applicable) +- **`.cspell.yaml`** - Spell-check configuration and technical term dictionary +- **`.markdownlint-cli2.yaml`** - Markdown linting rules +- **`.yamllint.yaml`** - YAML linting configuration +- **`.reviewmark.yaml`** - File review definitions and tracking +- **`nuget.config`** - NuGet package sources (if .NET) +- **`package.json`** - Node.js dependencies for linting tools +- **`requirements.yaml`** - Root requirements file with includes +- **`pip-requirements.txt`** - Python dependencies for yamllint +- **`lint.sh` / `lint.bat`** - Cross-platform comprehensive linting scripts + +# Standards Application (ALL Agents Must Follow) Before performing any work, agents must read and apply the relevant standards from `.github/standards/`: @@ -17,24 +53,23 @@ Before performing any work, agents must read and apply the relevant standards fr Load only the standards relevant to your specific task scope and apply their quality checks and guidelines throughout your work. -## Agent Delegation Guidelines +# Agent Delegation Guidelines The default agent should handle simple, straightforward tasks directly. Delegate to specialized agents only for specific scenarios: -- **Light development work** (small fixes, simple features) → Call developer agent -- **Light quality checking** (linting, basic validation) → Call quality agent -- **Formal feature implementation** (complex, multi-step) → Call the `implementation` agent -- **Formal bug resolution** (complex debugging, systematic fixes) → Call the `implementation` agent -- **Formal reviews** (compliance verification, detailed analysis) → Call code-review agent -- **Template consistency** (downstream repository alignment) → Call repo-consistency agent +- **Light development work** (small fixes, simple features) → Call the developer agent +- **Light quality checking** (linting, basic validation) → Call the quality agent +- **Formal feature implementation** (complex, multi-step) → Call the implementation agent +- **Formal bug resolution** (complex debugging, systematic fixes) → Call the implementation agent +- **Formal reviews** (compliance verification, detailed analysis) → Call the code-review agent +- **Template consistency** (downstream repository alignment) → Call the repo-consistency agent ## Available Specialized Agents -- **code-review** - Agent for performing formal reviews using standardized - review processes -- **developer** - General-purpose software development agent that applies - appropriate standards based on the work being performed +- **developer** - General-purpose software development agent that applies appropriate + standards based on the work being performed +- **code-review** - Agent for performing formal reviews using standardized review processes - **implementation** - Orchestrator agent that manages quality implementations through a formal state machine workflow - **quality** - Quality assurance agent that grades developer work against DEMA @@ -42,129 +77,49 @@ Delegate to specialized agents only for specific scenarios: - **repo-consistency** - Ensures downstream repositories remain consistent with the TemplateDotNetTool template patterns and best practices -## Quality Gate Enforcement (ALL Agents Must Verify) +# Linting (Required Before Quality Gates) + +1. **Markdown Auto-fix**: `npx markdownlint-cli2 --fix **/*.md` (fixes most markdown issues except line length) +2. **Dotnet Auto-fix**: `dotnet format` (reformats .NET languages) +3. **Run full check**: `lint.bat` (Windows) or `lint.sh` (Unix) +4. **Fix remaining**: Address line length, spelling, YAML syntax manually +5. **Verify clean**: Re-run until 0 errors before quality validation + +## Linting Tools (ALL Must Pass) + +- **markdownlint-cli2**: Markdown style and formatting enforcement +- **cspell**: Spell-checking across all text files (use `.cspell.yaml` for technical terms) +- **yamllint**: YAML structure and formatting validation +- **Language-specific linters**: Based on repository technology stack + +# Quality Gate Enforcement (ALL Agents Must Verify) Configuration files and scripts are self-documenting with their design intent and modification policies in header comments. -1. **Linting Standards**: `./lint.sh` (Unix) or `lint.bat` (Windows) - comprehensive linting suite -2. **Build Quality**: Zero warnings (`TreatWarningsAsErrors=true`) -3. **Static Analysis**: SonarQube/CodeQL passing with no blockers -4. **Requirements Traceability**: `dotnet reqstream --enforce` passing -5. **Test Coverage**: All requirements linked to passing tests -6. **Documentation Currency**: All docs current and generated -7. **File Review Status**: All reviewable files have current reviews +1. **Build Quality**: Zero warnings (`TreatWarningsAsErrors=true`) +2. **Static Analysis**: SonarQube/CodeQL passing with no blockers +3. **Requirements Traceability**: `dotnet reqstream --enforce` passing +4. **Test Coverage**: All requirements linked to passing tests +5. **Documentation Currency**: All docs current and generated +6. **File Review Status**: All reviewable files have current reviews -## Continuous Compliance Overview +# Continuous Compliance Overview This repository follows the DEMA Consulting Continuous Compliance approach, which enforces quality and compliance gates on every CI/CD run instead of as a last-mile activity. -### Core Principles +## Core Principles - **Requirements Traceability**: Every requirement MUST link to passing tests - **Quality Gates**: All quality checks must pass before merge - **Documentation Currency**: All docs auto-generated and kept current - **Automated Evidence**: Full audit trail generated with every build -## Required Compliance Tools - -### Linting Tools (ALL Must Pass) - -- **markdownlint-cli2**: Markdown style and formatting enforcement -- **cspell**: Spell-checking across all text files (use `.cspell.yaml` for technical terms) -- **yamllint**: YAML structure and formatting validation -- **Language-specific linters**: Based on repository technology stack - -### Quality Analysis - -- **SonarQube/SonarCloud**: Code quality and security analysis -- **CodeQL**: Security vulnerability scanning (produces SARIF output) -- **Static analyzers**: Microsoft.CodeAnalysis.NetAnalyzers, SonarAnalyzer.CSharp, etc. - -### Requirements & Compliance +## Requirements & Compliance - **ReqStream**: Requirements traceability enforcement (`dotnet reqstream --enforce`) - **ReviewMark**: File review status enforcement - **BuildMark**: Tool version documentation - **VersionMark**: Version tracking across CI/CD jobs - -## Project Structure Template - -- `docs/` - Documentation and compliance artifacts - - `design/` - Detailed design documents - - `introduction.md` - System/Subsystem/Unit breakdown for this repository - - `reqstream/` - Subsystem requirements YAML files (included by root requirements.yaml) - - Auto-generated reports (requirements, justifications, trace matrix) -- `src/{ProjectName}/` - Source code projects -- `test/{ProjectName}.Tests/` - Test projects -- `.github/workflows/` - CI/CD pipeline definitions (build.yaml, build_on_push.yaml, release.yaml) -- Configuration files: `.editorconfig`, `.clang-format`, `nuget.config`, `.reviewmark.yaml`, etc. - -## Key Configuration Files - -### Essential Files (Repository-Specific) - -- **`lint.sh` / `lint.bat`** - Cross-platform comprehensive linting scripts -- **`.editorconfig`** - Code formatting rules -- **`.clang-format`** - C/C++ formatting (if applicable) -- **`.cspell.yaml`** - Spell-check configuration and technical term dictionary -- **`.markdownlint-cli2.yaml`** - Markdown linting rules -- **`.yamllint.yaml`** - YAML linting configuration -- **`nuget.config`** - NuGet package sources (if .NET) -- **`package.json`** - Node.js dependencies for linting tools - -### Compliance Files - -- **`requirements.yaml`** - Root requirements file with includes -- **`.reviewmark.yaml`** - File review definitions and tracking -- CI/CD pipeline files with quality gate enforcement - -## Continuous Compliance Workflow - -### CI/CD Pipeline Stages (Standard) - -1. **Lint**: `./lint.sh` or `lint.bat` - comprehensive linting suite -2. **Build**: Compile with warnings as errors -3. **Analyze**: SonarQube/SonarCloud, CodeQL security scanning -4. **Test**: Execute all tests, generate coverage reports -5. **Validate**: Tool self-validation tests -6. **Document**: Generate requirements reports, trace matrix, build notes -7. **Enforce**: Requirements traceability, file review status -8. **Publish**: Generate final documentation (Pandoc → PDF) - -### Quality Gate Enforcement - -All stages must pass before merge. Pipeline fails immediately on: - -- Any linting errors -- Build warnings or errors -- Security vulnerabilities (CodeQL) -- Requirements without test coverage -- Outdated file reviews -- Missing documentation - -## Continuous Compliance Requirements - -This repository follows continuous compliance practices from DEMA Consulting -Continuous Compliance . - -### Core Requirements Traceability Rules - -- **ALL requirements MUST be linked to tests** - Enforced in CI via `dotnet reqstream --enforce` -- **NOT all tests need requirement links** - Tests may exist for corner cases, design validation, failure scenarios -- **Source filters are critical** - Platform/framework requirements need specific test evidence - -For detailed requirements format, test linkage patterns, and ReqStream -integration, call the developer agent with requirements management context. - -## Agent Report Files - -Upon completion, create a report file at `.agent-logs/{agent-name}-{subject}-{unique-id}.md` that includes: - -- A concise summary of the work performed -- Any important decisions made and their rationale -- Follow-up items, open questions, or TODOs - -Store agent logs in the `.agent-logs/` folder so they are ignored via `.gitignore` and excluded from linting and commits. diff --git a/docs/design/definition.yaml b/docs/design/definition.yaml index 54a2856..4d4e357 100644 --- a/docs/design/definition.yaml +++ b/docs/design/definition.yaml @@ -1,25 +1,26 @@ --- resource-path: - docs/design - - docs/design/cli - - docs/design/sarif - - docs/design/self-test - - docs/design/utilities + - docs/design/sarifmark + - docs/design/sarifmark/cli + - docs/design/sarifmark/sarif + - docs/design/sarifmark/self-test + - docs/design/sarifmark/utilities - docs/template input-files: - docs/design/title.txt - docs/design/introduction.md - - docs/design/system.md - - docs/design/program.md - - docs/design/cli/cli.md - - docs/design/cli/context.md - - docs/design/sarif/sarif.md - - docs/design/sarif/sarif-result.md - - docs/design/sarif/sarif-results.md - - docs/design/self-test/self-test.md - - docs/design/self-test/validation.md - - docs/design/utilities/utilities.md - - docs/design/utilities/path-helpers.md + - docs/design/sarifmark/sarifmark.md + - docs/design/sarifmark/program.md + - docs/design/sarifmark/cli/cli.md + - docs/design/sarifmark/cli/context.md + - docs/design/sarifmark/sarif/sarif.md + - docs/design/sarifmark/sarif/sarif-result.md + - docs/design/sarifmark/sarif/sarif-results.md + - docs/design/sarifmark/self-test/self-test.md + - docs/design/sarifmark/self-test/validation.md + - docs/design/sarifmark/utilities/utilities.md + - docs/design/sarifmark/utilities/path-helpers.md template: template.html table-of-contents: true number-sections: true diff --git a/docs/design/cli/cli.md b/docs/design/sarifmark/cli/cli.md similarity index 100% rename from docs/design/cli/cli.md rename to docs/design/sarifmark/cli/cli.md diff --git a/docs/design/cli/context.md b/docs/design/sarifmark/cli/context.md similarity index 100% rename from docs/design/cli/context.md rename to docs/design/sarifmark/cli/context.md diff --git a/docs/design/program.md b/docs/design/sarifmark/program.md similarity index 100% rename from docs/design/program.md rename to docs/design/sarifmark/program.md diff --git a/docs/design/sarif/sarif-result.md b/docs/design/sarifmark/sarif/sarif-result.md similarity index 100% rename from docs/design/sarif/sarif-result.md rename to docs/design/sarifmark/sarif/sarif-result.md diff --git a/docs/design/sarif/sarif-results.md b/docs/design/sarifmark/sarif/sarif-results.md similarity index 100% rename from docs/design/sarif/sarif-results.md rename to docs/design/sarifmark/sarif/sarif-results.md diff --git a/docs/design/sarif/sarif.md b/docs/design/sarifmark/sarif/sarif.md similarity index 100% rename from docs/design/sarif/sarif.md rename to docs/design/sarifmark/sarif/sarif.md diff --git a/docs/design/system.md b/docs/design/sarifmark/sarifmark.md similarity index 97% rename from docs/design/system.md rename to docs/design/sarifmark/sarifmark.md index 8eb1f19..c224db3 100644 --- a/docs/design/system.md +++ b/docs/design/sarifmark/sarifmark.md @@ -82,7 +82,7 @@ subsystem is a stateless helper used by `SelfTest` for path construction. ## System Requirements -System-level requirements are captured in `docs/reqstream/system.yaml` +System-level requirements are captured in `docs/reqstream/sarifmark/sarifmark.yaml` and are validated through integration tests that exercise the published dotnet DLL end-to-end across the supported platforms. diff --git a/docs/design/self-test/self-test.md b/docs/design/sarifmark/self-test/self-test.md similarity index 100% rename from docs/design/self-test/self-test.md rename to docs/design/sarifmark/self-test/self-test.md diff --git a/docs/design/self-test/validation.md b/docs/design/sarifmark/self-test/validation.md similarity index 100% rename from docs/design/self-test/validation.md rename to docs/design/sarifmark/self-test/validation.md diff --git a/docs/design/sarifmark/utilities/path-helpers.md b/docs/design/sarifmark/utilities/path-helpers.md new file mode 100644 index 0000000..3cb7e8d --- /dev/null +++ b/docs/design/sarifmark/utilities/path-helpers.md @@ -0,0 +1,63 @@ +# PathHelpers Class + +## Overview + +`PathHelpers` is a static internal utility class that provides a safe path-combination method. It +protects callers against path-traversal attacks by verifying the resolved combined path stays +within the base directory. Note that `Path.GetFullPath` normalizes `.`/`..` segments but does +not resolve symlinks or reparse points, so this check guards against string-level traversal +only. + +## SafePathCombine Method + +```csharp +internal static string SafePathCombine(string basePath, string relativePath) +``` + +Combines `basePath` and `relativePath` safely, ensuring the resulting path remains within +the base directory. It is used by the `TemporaryDirectory` helper inside `Validation` when +constructing paths inside a temporary directory from `Guid`-based file names. + +### Null Checks + +Both `basePath` and `relativePath` are validated with `ArgumentNullException.ThrowIfNull` before +any other processing. This satisfies requirement `SarifMark-PathHelpers-NullCheck`. + +### Path Combination + +`Path.Combine(basePath, relativePath)` is called to produce the candidate path, preserving +the caller's relative/absolute style. + +### Post-Combination Security Check + +The method resolves both `basePath` and the candidate to absolute form with `Path.GetFullPath`, +then calls `Path.GetRelativePath(absoluteBase, absoluteCombined)` and rejects the input if +the result is exactly `".."`, starts with `".."` followed by `Path.DirectorySeparatorChar` +or `Path.AltDirectorySeparatorChar`, or is itself rooted (absolute). These conditions indicate +the combined path escapes the base directory. This satisfies requirement +`SarifMark-PathHelpers-PostCombineCheck`. + +### Return Value + +On success, the non-resolved combined path (the direct result of `Path.Combine`) is returned. +This satisfies requirement `SarifMark-PathHelpers-SafeCombine`. + +## Design Decisions + +- **`Path.GetRelativePath` for containment check**: Using `GetRelativePath` to verify + containment handles root paths (e.g. `/`, `C:\`), platform case-sensitivity, and + directory-separator normalization natively. The containment test treats `..` as an + escaping segment only when it is the entire relative result or is followed by a directory + separator, avoiding false positives for valid in-base names such as `..data`. +- **Post-combine canonical-path check**: Resolving paths after combining handles all traversal + patterns — `../`, embedded `/../`, absolute-path overrides, and platform edge cases — + without fragile pre-combine string inspection of `relativePath`. +- **ArgumentException on invalid input**: Callers receive a specific `ArgumentException` + identifying `relativePath` as the problematic parameter, making debugging straightforward. +- **No logging or error accumulation**: `SafePathCombine` is a pure utility method that throws + on invalid input; it does not interact with the `Context` or any output mechanism. + +## Cross-References + +See the Self-Validation document for the `TemporaryDirectory` nested class that calls +`SafePathCombine`. diff --git a/docs/design/utilities/utilities.md b/docs/design/sarifmark/utilities/utilities.md similarity index 100% rename from docs/design/utilities/utilities.md rename to docs/design/sarifmark/utilities/utilities.md diff --git a/docs/design/utilities/path-helpers.md b/docs/design/utilities/path-helpers.md deleted file mode 100644 index 75d1cf1..0000000 --- a/docs/design/utilities/path-helpers.md +++ /dev/null @@ -1,54 +0,0 @@ -# PathHelpers Class - -## Overview - -The `PathHelpers` class (`PathHelpers.cs`) is a static internal utility class that provides safe -path operations for use within the tool. It guards against path-traversal vulnerabilities when -constructing file paths from input that may not be fully trusted. - -## SafePathCombine Method - -`SafePathCombine(string basePath, string relativePath)` combines a base directory path with a -relative path and returns the result, subject to two layers of validation. It is used by the -`TemporaryDirectory` helper inside `Validation` when constructing paths inside a temporary -directory from `Guid`-based file names. - -### Null Checks - -Both `basePath` and `relativePath` are validated with `ArgumentNullException.ThrowIfNull` before -any other processing. This satisfies requirement `SarifMark-PathHelpers-NullCheck`. - -### Pre-Combination Check - -Before calling `Path.Combine`, the method inspects `relativePath` directly: - -- If `relativePath` contains the string `".."`, `ArgumentException` is thrown. -- If `Path.IsPathRooted(relativePath)` returns `true`, `ArgumentException` is thrown. - -These checks reject the most common path-traversal patterns before any file-system operations -are performed. This satisfies requirements `SarifMark-PathHelpers-PreCombineCheck` and -`SarifMark-PathHelpers-RootedCheck`. - -### Path Combination - -`Path.Combine(basePath, relativePath)` is called after the pre-combination checks pass. Because -`relativePath` has been verified to contain no `".."` segments and is not rooted, the combined -path is expected to remain under `basePath`. - -### Post-Combination Check - -As a defense-in-depth measure, the method resolves both paths using `Path.GetFullPath` and then -calls `Path.GetRelativePath(fullBasePath, fullCombinedPath)`. If the resulting relative string -starts with `".."` or is itself rooted, `ArgumentException` is thrown. This second check catches -any edge cases that might survive the pre-combination string test on a given operating system. -This satisfies requirement `SarifMark-PathHelpers-PostCombineCheck`. - -### Return Value - -On success, the non-resolved combined path (the direct result of `Path.Combine`) is returned. -This satisfies requirement `SarifMark-PathHelpers-SafeCombine`. - -## Cross-References - -See the Self-Validation document for the `TemporaryDirectory` nested class that calls -`SafePathCombine`. diff --git a/docs/reqstream/cli/subsystem-cli.yaml b/docs/reqstream/sarifmark/cli/cli.yaml similarity index 87% rename from docs/reqstream/cli/subsystem-cli.yaml rename to docs/reqstream/sarifmark/cli/cli.yaml index 89eac9b..c29787f 100644 --- a/docs/reqstream/cli/subsystem-cli.yaml +++ b/docs/reqstream/sarifmark/cli/cli.yaml @@ -13,8 +13,8 @@ sections: children: - SarifMark-Context-Create tests: - - IntegrationTest_VersionFlag_OutputsVersion - - IntegrationTest_HelpFlag_OutputsUsageInformation + - Cli_VersionFlag_OutputsVersion + - Cli_HelpFlag_OutputsUsageInformation - id: SarifMark-Cli-Version title: The tool shall display version information when requested. @@ -25,7 +25,7 @@ sections: children: - SarifMark-Context-VersionFlag tests: - - IntegrationTest_VersionFlag_OutputsVersion + - Cli_VersionFlag_OutputsVersion - Context_Create_VersionFlag_SetsVersionTrue - id: SarifMark-Cli-Help @@ -37,7 +37,7 @@ sections: children: - SarifMark-Context-HelpFlag tests: - - IntegrationTest_HelpFlag_OutputsUsageInformation + - Cli_HelpFlag_OutputsUsageInformation - Context_Create_HelpFlag_SetsHelpTrue - id: SarifMark-Cli-Silent @@ -49,7 +49,7 @@ sections: children: - SarifMark-Context-SilentFlag tests: - - IntegrationTest_SilentFlag_SuppressesOutput + - Cli_SilentFlag_SuppressesOutput - id: SarifMark-Cli-Log title: The tool shall support writing output to a log file. @@ -60,7 +60,7 @@ sections: children: - SarifMark-Context-LogParam tests: - - IntegrationTest_LogFile_WritesOutputToFile + - Cli_LogFile_WritesOutputToFile - id: SarifMark-Cli-Enforce title: The tool shall support enforcing quality gate checks. @@ -72,7 +72,7 @@ sections: - SarifMark-Context-EnforceFlag tests: - SarifMark_Enforcement - - IntegrationTest_EnforceFlagWithIssues_ReturnsError + - Cli_EnforceFlagWithIssues_ReturnsError - id: SarifMark-Cli-InvalidArgs title: The tool shall reject unknown command-line arguments. @@ -83,4 +83,4 @@ sections: children: - SarifMark-Context-UnknownArgs tests: - - IntegrationTest_UnknownArgument_ShowsError + - Cli_UnknownArgument_ShowsError diff --git a/docs/reqstream/cli/unit-context.yaml b/docs/reqstream/sarifmark/cli/context.yaml similarity index 100% rename from docs/reqstream/cli/unit-context.yaml rename to docs/reqstream/sarifmark/cli/context.yaml diff --git a/docs/reqstream/platform-requirements.yaml b/docs/reqstream/sarifmark/platform-requirements.yaml similarity index 100% rename from docs/reqstream/platform-requirements.yaml rename to docs/reqstream/sarifmark/platform-requirements.yaml diff --git a/docs/reqstream/unit-program.yaml b/docs/reqstream/sarifmark/program.yaml similarity index 95% rename from docs/reqstream/unit-program.yaml rename to docs/reqstream/sarifmark/program.yaml index 5d08677..9a96b46 100644 --- a/docs/reqstream/unit-program.yaml +++ b/docs/reqstream/sarifmark/program.yaml @@ -76,6 +76,6 @@ sections: tags: [internal] tests: - Program_Main_NoArguments_ReturnsError - - IntegrationTest_ValidSarifFile_ProcessesSuccessfully - - IntegrationTest_EnforceFlagWithIssues_ReturnsError - - IntegrationTest_GenerateReport_CreatesReportFile + - Program_Main_ValidSarifFile_ProcessesSuccessfully + - Program_Main_EnforceFlagWithIssues_ReturnsError + - Program_Main_ReportFile_CreatesReport diff --git a/docs/reqstream/sarif/subsystem-report.yaml b/docs/reqstream/sarifmark/sarif/report.yaml similarity index 96% rename from docs/reqstream/sarif/subsystem-report.yaml rename to docs/reqstream/sarifmark/sarif/report.yaml index c24256a..2739946 100644 --- a/docs/reqstream/sarif/subsystem-report.yaml +++ b/docs/reqstream/sarifmark/sarif/report.yaml @@ -13,7 +13,7 @@ sections: children: - SarifMark-SarifResults-ToMarkdown tests: - - IntegrationTest_GenerateReport_CreatesReportFile + - Sarif_GenerateReport_CreatesReportFile - SarifMark_MarkdownReportGeneration - id: SarifMark-Report-Depth @@ -25,7 +25,7 @@ sections: children: - SarifMark-SarifResults-ValidateDepth tests: - - IntegrationTest_ReportDepth_IsConfigurable + - Sarif_ReportDepth_IsConfigurable - id: SarifMark-Report-Counts title: The tool shall display result counts in reports. diff --git a/docs/reqstream/sarif/unit-sarif-result.yaml b/docs/reqstream/sarifmark/sarif/sarif-result.yaml similarity index 100% rename from docs/reqstream/sarif/unit-sarif-result.yaml rename to docs/reqstream/sarifmark/sarif/sarif-result.yaml diff --git a/docs/reqstream/sarif/unit-sarif-results.yaml b/docs/reqstream/sarifmark/sarif/sarif-results.yaml similarity index 100% rename from docs/reqstream/sarif/unit-sarif-results.yaml rename to docs/reqstream/sarifmark/sarif/sarif-results.yaml diff --git a/docs/reqstream/sarif/subsystem-sarif.yaml b/docs/reqstream/sarifmark/sarif/sarif.yaml similarity index 96% rename from docs/reqstream/sarif/subsystem-sarif.yaml rename to docs/reqstream/sarifmark/sarif/sarif.yaml index 68f3fe0..43ef483 100644 --- a/docs/reqstream/sarif/subsystem-sarif.yaml +++ b/docs/reqstream/sarifmark/sarif/sarif.yaml @@ -14,7 +14,7 @@ sections: - SarifMark-SarifResults-ParseResults tests: - SarifMark_SarifReading - - IntegrationTest_ValidSarifFile_ProcessesSuccessfully + - Sarif_ValidSarifFile_ProcessesSuccessfully - id: SarifMark-Sarif-Validation title: The tool shall validate SARIF file format. @@ -95,7 +95,7 @@ sections: children: - SarifMark-SarifResults-ValidatePath tests: - - IntegrationTest_MissingSarifParameter_ShowsError + - Sarif_MissingSarifParameter_ShowsError - id: SarifMark-Sarif-Processing title: The tool shall process valid SARIF files successfully. @@ -106,4 +106,4 @@ sections: children: - SarifMark-SarifResults-ParseResults tests: - - IntegrationTest_ValidSarifFile_ProcessesSuccessfully + - Sarif_ValidSarifFile_ProcessesSuccessfully diff --git a/docs/reqstream/system.yaml b/docs/reqstream/sarifmark/sarifmark.yaml similarity index 100% rename from docs/reqstream/system.yaml rename to docs/reqstream/sarifmark/sarifmark.yaml diff --git a/docs/reqstream/self-test/subsystem-self-test.yaml b/docs/reqstream/sarifmark/self-test/self-test.yaml similarity index 94% rename from docs/reqstream/self-test/subsystem-self-test.yaml rename to docs/reqstream/sarifmark/self-test/self-test.yaml index ea749b6..64cf3ff 100644 --- a/docs/reqstream/self-test/subsystem-self-test.yaml +++ b/docs/reqstream/sarifmark/self-test/self-test.yaml @@ -13,7 +13,7 @@ sections: children: - SarifMark-Validation-Run tests: - - IntegrationTest_ValidateFlag_RunsSelfValidation + - SelfTest_ValidateFlag_RunsSelfValidation - id: SarifMark-Validate-ResultFiles title: The tool shall write validation results to test result files. @@ -60,7 +60,7 @@ sections: children: - SarifMark-Validation-EnforcementTest tests: - - IntegrationTest_EnforceFlagWithIssues_ReturnsError + - Cli_EnforceFlagWithIssues_ReturnsError - SarifMark_Enforcement - id: SarifMark-Enforce-ExitCode @@ -72,5 +72,5 @@ sections: children: - SarifMark-Validation-EnforcementTest tests: - - IntegrationTest_EnforceFlagWithIssues_ReturnsError + - Cli_EnforceFlagWithIssues_ReturnsError - SarifMark_Enforcement diff --git a/docs/reqstream/self-test/unit-validation.yaml b/docs/reqstream/sarifmark/self-test/validation.yaml similarity index 100% rename from docs/reqstream/self-test/unit-validation.yaml rename to docs/reqstream/sarifmark/self-test/validation.yaml diff --git a/docs/reqstream/utilities/unit-path-helpers.yaml b/docs/reqstream/sarifmark/utilities/path-helpers.yaml similarity index 63% rename from docs/reqstream/utilities/unit-path-helpers.yaml rename to docs/reqstream/sarifmark/utilities/path-helpers.yaml index 5118551..8d028e1 100644 --- a/docs/reqstream/utilities/unit-path-helpers.yaml +++ b/docs/reqstream/sarifmark/utilities/path-helpers.yaml @@ -16,6 +16,7 @@ sections: - PathHelpers_SafePathCombine_SimpleFilename_CombinesSuccessfully - PathHelpers_SafePathCombine_PathWithSubdirectories_CombinesSuccessfully - PathHelpers_SafePathCombine_GuidBasedFilename_CombinesSuccessfully + - PathHelpers_SafePathCombine_FilenameWithEmbeddedDots_CombinesSuccessfully - id: SarifMark-PathHelpers-NullCheck title: The SafePathCombine method shall throw ArgumentNullException when either argument is null. @@ -28,34 +29,17 @@ sections: - PathHelpers_SafePathCombine_NullBasePath_ThrowsArgumentNullException - PathHelpers_SafePathCombine_NullRelativePath_ThrowsArgumentNullException - - id: SarifMark-PathHelpers-PreCombineCheck - title: The SafePathCombine method shall reject relative paths containing ".." sequences or rooted paths. + - id: SarifMark-PathHelpers-PostCombineCheck + title: The SafePathCombine method shall verify the combined path remains under the base path. justification: >- - Pre-combination checks catch the most common path traversal patterns before any file system - operations are performed, providing a fast and reliable first line of defense against - directory traversal attacks. + A post-combination containment check using Path.GetRelativePath handles all traversal + patterns — directory traversal sequences, absolute-path overrides, and platform edge + cases — without fragile pre-combine string inspection. Treating ".." as an escape segment + only when it is the entire result or is followed by a directory separator avoids false + positives for valid names such as "..data" or "v1..0.sarif". tags: [internal] tests: - PathHelpers_SafePathCombine_PathWithParentDirectory_ThrowsArgumentException - PathHelpers_SafePathCombine_PathWithDoubleDots_ThrowsArgumentException - - PathHelpers_SafePathCombine_FilenameWithEmbeddedDots_ThrowsArgumentException - - - id: SarifMark-PathHelpers-RootedCheck - title: The SafePathCombine method shall reject relative paths that are rooted or absolute. - justification: >- - Rejecting rooted relative paths prevents callers from accidentally bypassing the base path - restriction by supplying an absolute path as the relative argument, closing a potential path - traversal vector. - tags: [internal] - tests: - PathHelpers_SafePathCombine_AbsolutePath_ThrowsArgumentException - - - id: SarifMark-PathHelpers-PostCombineCheck - title: The SafePathCombine method shall verify the combined path remains under the base path. - justification: >- - A post-combination check adds a second layer of defense by verifying the combined path - regardless of how the operating system resolves it, ensuring that no combination of legal - segments can escape the intended base directory. - tags: [internal] - tests: - PathHelpers_SafePathCombine_ValidPaths_CombinesSuccessfully diff --git a/docs/reqstream/utilities/subsystem-utilities.yaml b/docs/reqstream/sarifmark/utilities/utilities.yaml similarity index 81% rename from docs/reqstream/utilities/subsystem-utilities.yaml rename to docs/reqstream/sarifmark/utilities/utilities.yaml index 52df4ea..339bca8 100644 --- a/docs/reqstream/utilities/subsystem-utilities.yaml +++ b/docs/reqstream/sarifmark/utilities/utilities.yaml @@ -13,8 +13,7 @@ sections: tags: [internal] children: - SarifMark-PathHelpers-SafeCombine - - SarifMark-PathHelpers-PreCombineCheck - - SarifMark-PathHelpers-RootedCheck + - SarifMark-PathHelpers-NullCheck - SarifMark-PathHelpers-PostCombineCheck tests: - - PathHelpers_SafePathCombine_ValidPaths_CombinesSuccessfully + - Utilities_SafePathHandling_ValidPaths_CombinesSuccessfully diff --git a/lint.bat b/lint.bat index c7440d4..433421b 100644 --- a/lint.bat +++ b/lint.bat @@ -2,7 +2,7 @@ setlocal REM Comprehensive Linting Script -REM +REM REM PURPOSE: REM - Run ALL lint checks when executed (no options or modes) REM - Output lint failures directly for agent parsing @@ -11,30 +11,55 @@ REM - Agents execute this script to identify files needing fixes set "LINT_ERROR=0" -REM Install npm dependencies -call npm install --silent +REM === PYTHON SECTION === + +REM Create python venv if necessary +if not exist ".venv\Scripts\activate.bat" python -m venv .venv +if errorlevel 1 goto skip_python -REM Create Python virtual environment (for yamllint) if missing -if not exist ".venv\Scripts\activate.bat" ( - python -m venv .venv -) +REM Activate python venv call .venv\Scripts\activate.bat +if errorlevel 1 goto skip_python + +REM Install python tools pip install -r pip-requirements.txt --quiet --disable-pip-version-check +if errorlevel 1 goto skip_python + +REM Run yamllint +yamllint . +if errorlevel 1 set "LINT_ERROR=1" +goto npm_section + +:skip_python +set "LINT_ERROR=1" + +REM === NPM SECTION === -REM Run spell check +:npm_section + +REM Install npm dependencies +call npm install --silent +if errorlevel 1 goto skip_npm + +REM Run cspell call npx cspell --no-progress --no-color --quiet "**/*.{md,yaml,yml,json,cs,cpp,hpp,h,txt}" if errorlevel 1 set "LINT_ERROR=1" -REM Run markdownlint check +REM Run markdownlint-cli2 call npx markdownlint-cli2 "**/*.md" if errorlevel 1 set "LINT_ERROR=1" +goto dotnet_section -REM Run yamllint check -yamllint . -if errorlevel 1 set "LINT_ERROR=1" +:skip_npm +set "LINT_ERROR=1" + +REM === DOTNET SECTION === + +:dotnet_section -REM Run .NET formatting check (verifies no changes are needed) +REM Run dotnet format dotnet format --verify-no-changes if errorlevel 1 set "LINT_ERROR=1" +REM Report result exit /b %LINT_ERROR% diff --git a/lint.sh b/lint.sh index c567e09..13ac584 100755 --- a/lint.sh +++ b/lint.sh @@ -1,7 +1,7 @@ #!/bin/bash # Comprehensive Linting Script -# +# # PURPOSE: # - Run ALL lint checks when executed (no options or modes) # - Output lint failures directly for agent parsing @@ -10,26 +10,47 @@ lint_error=0 -# Install npm dependencies -npm install --silent +# === PYTHON SECTION === -# Create Python virtual environment (for yamllint) +# Create python venv if necessary if [ ! -d ".venv" ]; then - python -m venv .venv + python -m venv .venv || { lint_error=1; skip_python=1; } +fi + +# Activate python venv +if [ "$skip_python" != "1" ]; then + source .venv/bin/activate || { lint_error=1; skip_python=1; } +fi + +# Install python tools +if [ "$skip_python" != "1" ]; then + pip install -r pip-requirements.txt --quiet --disable-pip-version-check || { lint_error=1; skip_python=1; } +fi + +# Run yamllint +if [ "$skip_python" != "1" ]; then + yamllint . || lint_error=1 fi -source .venv/bin/activate -pip install -r pip-requirements.txt --quiet --disable-pip-version-check -# Run spell check -npx cspell --no-progress --no-color --quiet "**/*.{md,yaml,yml,json,cs,cpp,hpp,h,txt}" || lint_error=1 +# === NPM SECTION === + +# Install npm dependencies +npm install --silent || { lint_error=1; skip_npm=1; } + +# Run cspell +if [ "$skip_npm" != "1" ]; then + npx cspell --no-progress --no-color --quiet "**/*.{md,yaml,yml,json,cs,cpp,hpp,h,txt}" || lint_error=1 +fi -# Run markdownlint check -npx markdownlint-cli2 "**/*.md" || lint_error=1 +# Run markdownlint-cli2 +if [ "$skip_npm" != "1" ]; then + npx markdownlint-cli2 "**/*.md" || lint_error=1 +fi -# Run yamllint check -yamllint . || lint_error=1 +# === DOTNET SECTION === -# Run .NET formatting check (verifies no changes are needed) +# Run dotnet format dotnet format --verify-no-changes || lint_error=1 +# Report result exit $lint_error diff --git a/requirements.yaml b/requirements.yaml index 3855d11..0dcb734 100644 --- a/requirements.yaml +++ b/requirements.yaml @@ -13,17 +13,17 @@ # --- includes: - - docs/reqstream/system.yaml - - docs/reqstream/cli/subsystem-cli.yaml - - docs/reqstream/cli/unit-context.yaml - - docs/reqstream/sarif/subsystem-sarif.yaml - - docs/reqstream/sarif/subsystem-report.yaml - - docs/reqstream/sarif/unit-sarif-result.yaml - - docs/reqstream/sarif/unit-sarif-results.yaml - - docs/reqstream/self-test/subsystem-self-test.yaml - - docs/reqstream/self-test/unit-validation.yaml - - docs/reqstream/utilities/subsystem-utilities.yaml - - docs/reqstream/utilities/unit-path-helpers.yaml - - docs/reqstream/platform-requirements.yaml + - docs/reqstream/sarifmark/sarifmark.yaml + - docs/reqstream/sarifmark/cli/cli.yaml + - docs/reqstream/sarifmark/cli/context.yaml + - docs/reqstream/sarifmark/sarif/sarif.yaml + - docs/reqstream/sarifmark/sarif/report.yaml + - docs/reqstream/sarifmark/sarif/sarif-result.yaml + - docs/reqstream/sarifmark/sarif/sarif-results.yaml + - docs/reqstream/sarifmark/self-test/self-test.yaml + - docs/reqstream/sarifmark/self-test/validation.yaml + - docs/reqstream/sarifmark/utilities/utilities.yaml + - docs/reqstream/sarifmark/utilities/path-helpers.yaml + - docs/reqstream/sarifmark/platform-requirements.yaml - docs/reqstream/ots/ots-software.yaml - - docs/reqstream/unit-program.yaml + - docs/reqstream/sarifmark/program.yaml diff --git a/src/DemaConsulting.SarifMark/Utilities/PathHelpers.cs b/src/DemaConsulting.SarifMark/Utilities/PathHelpers.cs index 6dbea08..254fa65 100644 --- a/src/DemaConsulting.SarifMark/Utilities/PathHelpers.cs +++ b/src/DemaConsulting.SarifMark/Utilities/PathHelpers.cs @@ -26,40 +26,37 @@ namespace DemaConsulting.SarifMark; internal static class PathHelpers { /// - /// Safely combines two paths, ensuring the second path doesn't contain path traversal sequences. + /// Safely combines two paths, ensuring the resolved combined path stays within the base directory. /// /// The base path. /// The relative path to combine. /// The combined path. - /// Thrown when relativePath contains path traversal sequences or is a rooted path. - /// Thrown when basePath or relativePath is null. + /// Thrown when or is . + /// + /// Thrown when the resolved combined path escapes the base directory, or when a supplied path is invalid. + /// + /// Thrown when a supplied path contains an unsupported format. + /// Thrown when the combined or resolved path exceeds the system-defined maximum length. internal static string SafePathCombine(string basePath, string relativePath) { // Validate inputs ArgumentNullException.ThrowIfNull(basePath); ArgumentNullException.ThrowIfNull(relativePath); - // Ensure the relative path doesn't contain path traversal sequences - if (relativePath.Contains("..") || Path.IsPathRooted(relativePath)) - { - throw new ArgumentException($"Invalid path component: {relativePath}", nameof(relativePath)); - } - - // This call to Path.Combine is safe because we've validated that: - // 1. relativePath doesn't contain ".." (path traversal) - // 2. relativePath is not an absolute path (IsPathRooted check) - // This ensures the combined path will always be under basePath + // Combine the paths (preserves the caller's relative/absolute style) var combinedPath = Path.Combine(basePath, relativePath); - // Additional security validation: ensure the combined path is still under the base path. - // This defense-in-depth approach protects against edge cases that might bypass the - // initial validation, ensuring the final path stays within the intended directory. - var fullBasePath = Path.GetFullPath(basePath); - var fullCombinedPath = Path.GetFullPath(combinedPath); + // Security check: resolve both paths to absolute form and verify the combined + // path is still inside the base directory. Path.GetRelativePath handles root + // paths, platform case-sensitivity, and directory-separator normalization natively. + var absoluteBase = Path.GetFullPath(basePath); + var absoluteCombined = Path.GetFullPath(combinedPath); + var checkRelative = Path.GetRelativePath(absoluteBase, absoluteCombined); - // Use GetRelativePath to verify the relationship between paths - var relativeCheck = Path.GetRelativePath(fullBasePath, fullCombinedPath); - if (relativeCheck.StartsWith("..") || Path.IsPathRooted(relativeCheck)) + if (string.Equals(checkRelative, "..", StringComparison.Ordinal) + || checkRelative.StartsWith(".." + Path.DirectorySeparatorChar, StringComparison.Ordinal) + || checkRelative.StartsWith(".." + Path.AltDirectorySeparatorChar, StringComparison.Ordinal) + || Path.IsPathRooted(checkRelative)) { throw new ArgumentException($"Invalid path component: {relativePath}", nameof(relativePath)); } diff --git a/test/DemaConsulting.SarifMark.Tests/Cli/CliTests.cs b/test/DemaConsulting.SarifMark.Tests/Cli/CliTests.cs new file mode 100644 index 0000000..58f374f --- /dev/null +++ b/test/DemaConsulting.SarifMark.Tests/Cli/CliTests.cs @@ -0,0 +1,199 @@ +// Copyright (c) DEMA Consulting +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +namespace DemaConsulting.SarifMark.Tests; + +/// +/// Subsystem tests for the Command-Line Interface subsystem. +/// +[TestClass] +public class CliTests +{ + private string _dllPath = string.Empty; + private string _testDataPath = string.Empty; + + /// + /// Initialize test by locating the SarifMark DLL and test data. + /// + [TestInitialize] + public void TestInitialize() + { + var baseDir = AppContext.BaseDirectory; + _dllPath = PathHelpers.SafePathCombine(baseDir, "DemaConsulting.SarifMark.dll"); + _testDataPath = PathHelpers.SafePathCombine(baseDir, "TestData"); + + Assert.IsTrue(File.Exists(_dllPath), $"Could not find SarifMark DLL at {_dllPath}"); + } + + /// + /// Test that version flag outputs version information. + /// + [TestMethod] + public void Cli_VersionFlag_OutputsVersion() + { + // Arrange - No special setup needed + + // Act + var exitCode = Runner.Run( + out var output, + "dotnet", + _dllPath, + "--version"); + + // Assert + Assert.AreEqual(0, exitCode); + Assert.IsFalse(string.IsNullOrWhiteSpace(output)); + Assert.DoesNotContain("Error", output); + Assert.DoesNotContain("Copyright", output); + } + + /// + /// Test that help flag outputs usage information. + /// + [TestMethod] + public void Cli_HelpFlag_OutputsUsageInformation() + { + // Arrange - No special setup needed + + // Act + var exitCode = Runner.Run( + out var output, + "dotnet", + _dllPath, + "--help"); + + // Assert + Assert.AreEqual(0, exitCode); + Assert.Contains("Usage: sarifmark", output); + Assert.Contains("Options:", output); + Assert.Contains("--version", output); + Assert.Contains("--help", output); + Assert.Contains("--sarif", output); + Assert.MatchesRegex(@"--report(?!-)", output); + } + + /// + /// Test that silent flag suppresses console output. + /// + [TestMethod] + public void Cli_SilentFlag_SuppressesOutput() + { + // Arrange + var sarifFile = PathHelpers.SafePathCombine(_testDataPath, "sample.sarif"); + Assert.IsTrue(File.Exists(sarifFile), $"Test SARIF file not found at {sarifFile}"); + + // Act + var exitCode = Runner.Run( + out var output, + "dotnet", + _dllPath, + "--silent", + "--sarif", sarifFile); + + // Assert + Assert.AreEqual(0, exitCode); + Assert.DoesNotContain("SarifMark version", output); + Assert.DoesNotContain("Copyright", output); + } + + /// + /// Test that log file parameter writes output to file. + /// + [TestMethod] + public void Cli_LogFile_WritesOutputToFile() + { + // Arrange + var sarifFile = PathHelpers.SafePathCombine(_testDataPath, "sample.sarif"); + Assert.IsTrue(File.Exists(sarifFile), $"Test SARIF file not found at {sarifFile}"); + + var logFile = PathHelpers.SafePathCombine(Path.GetTempPath(), $"test-log-{Guid.NewGuid()}.log"); + + try + { + // Act + var exitCode = Runner.Run( + out _, + "dotnet", + _dllPath, + "--log", logFile, + "--sarif", sarifFile); + + // Assert + Assert.AreEqual(0, exitCode); + Assert.IsTrue(File.Exists(logFile), "Log file was not created"); + + var logContent = File.ReadAllText(logFile); + Assert.Contains("SarifMark version", logContent); + Assert.Contains("SARIF File:", logContent); + Assert.Contains("Tool: TestTool", logContent); + } + finally + { + if (File.Exists(logFile)) + { + File.Delete(logFile); + } + } + } + + /// + /// Test that enforce flag with issues returns error exit code. + /// + [TestMethod] + public void Cli_EnforceFlagWithIssues_ReturnsError() + { + // Arrange + var sarifFile = PathHelpers.SafePathCombine(_testDataPath, "sample.sarif"); + Assert.IsTrue(File.Exists(sarifFile), $"Test SARIF file not found at {sarifFile}"); + + // Act + var exitCode = Runner.Run( + out var output, + "dotnet", + _dllPath, + "--sarif", sarifFile, + "--enforce"); + + // Assert + Assert.AreEqual(1, exitCode); + Assert.Contains("Issues found in SARIF file", output); + } + + /// + /// Test that unknown arguments are rejected with error. + /// + [TestMethod] + public void Cli_UnknownArgument_ShowsError() + { + // Arrange - No special setup needed + + // Act + var exitCode = Runner.Run( + out var output, + "dotnet", + _dllPath, + "--unknown-flag"); + + // Assert + Assert.AreEqual(1, exitCode); + Assert.Contains("Error:", output); + Assert.Contains("unknown-flag", output); + } +} diff --git a/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs b/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs index e36f477..ade2db8 100644 --- a/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs +++ b/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs @@ -149,4 +149,93 @@ public void Program_Main_UnknownArgument_ReturnsError() Console.SetError(originalError); } } + + /// + /// Test that Main processes a valid SARIF file successfully. + /// + [TestMethod] + public void Program_Main_ValidSarifFile_ProcessesSuccessfully() + { + // Arrange + var sarifFile = Path.Combine(AppContext.BaseDirectory, "TestData", "sample.sarif"); + var originalOut = Console.Out; + try + { + using var outWriter = new StringWriter(); + Console.SetOut(outWriter); + + // Act + var result = Program.Main(["--sarif", sarifFile]); + + // Assert + Assert.AreEqual(0, result); + Assert.Contains("Tool: TestTool", outWriter.ToString()); + } + finally + { + Console.SetOut(originalOut); + } + } + + /// + /// Test that enforce flag returns error exit code when issues are found. + /// + [TestMethod] + public void Program_Main_EnforceFlagWithIssues_ReturnsError() + { + // Arrange + var sarifFile = Path.Combine(AppContext.BaseDirectory, "TestData", "sample.sarif"); + var originalOut = Console.Out; + var originalError = Console.Error; + try + { + using var outWriter = new StringWriter(); + using var errWriter = new StringWriter(); + Console.SetOut(outWriter); + Console.SetError(errWriter); + + // Act + var result = Program.Main(["--sarif", sarifFile, "--enforce"]); + + // Assert + Assert.AreEqual(1, result); + } + finally + { + Console.SetOut(originalOut); + Console.SetError(originalError); + } + } + + /// + /// Test that Main with report file creates the report. + /// + [TestMethod] + public void Program_Main_ReportFile_CreatesReport() + { + // Arrange + var sarifFile = Path.Combine(AppContext.BaseDirectory, "TestData", "sample.sarif"); + var reportFile = Path.Combine(Path.GetTempPath(), $"test-report-{Guid.NewGuid()}.md"); + var originalOut = Console.Out; + try + { + using var outWriter = new StringWriter(); + Console.SetOut(outWriter); + + // Act + var result = Program.Main(["--sarif", sarifFile, "--report", reportFile]); + + // Assert + Assert.AreEqual(0, result); + Assert.IsTrue(File.Exists(reportFile), "Report file was not created"); + } + finally + { + Console.SetOut(originalOut); + if (File.Exists(reportFile)) + { + File.Delete(reportFile); + } + } + } } diff --git a/test/DemaConsulting.SarifMark.Tests/Sarif/SarifTests.cs b/test/DemaConsulting.SarifMark.Tests/Sarif/SarifTests.cs new file mode 100644 index 0000000..a54ab32 --- /dev/null +++ b/test/DemaConsulting.SarifMark.Tests/Sarif/SarifTests.cs @@ -0,0 +1,188 @@ +// Copyright (c) DEMA Consulting +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +namespace DemaConsulting.SarifMark.Tests; + +/// +/// Subsystem tests for the SARIF reading subsystem. +/// +[TestClass] +public class SarifTests +{ + private string _dllPath = string.Empty; + private string _testDataPath = string.Empty; + + /// + /// Initialize test by locating the SarifMark DLL and test data. + /// + [TestInitialize] + public void TestInitialize() + { + var baseDir = AppContext.BaseDirectory; + _dllPath = PathHelpers.SafePathCombine(baseDir, "DemaConsulting.SarifMark.dll"); + _testDataPath = PathHelpers.SafePathCombine(baseDir, "TestData"); + + Assert.IsTrue(File.Exists(_dllPath), $"Could not find SarifMark DLL at {_dllPath}"); + } + + /// + /// Test that missing sarif parameter shows error. + /// + [TestMethod] + public void Sarif_MissingSarifParameter_ShowsError() + { + // Arrange - No special setup needed + + // Act + var exitCode = Runner.Run( + out var output, + "dotnet", + _dllPath); + + // Assert + Assert.AreEqual(1, exitCode); + Assert.Contains("--sarif parameter is required", output); + } + + /// + /// Test that processing a valid SARIF file succeeds. + /// + [TestMethod] + public void Sarif_ValidSarifFile_ProcessesSuccessfully() + { + // Arrange + var sarifFile = PathHelpers.SafePathCombine(_testDataPath, "sample.sarif"); + Assert.IsTrue(File.Exists(sarifFile), $"Test SARIF file not found at {sarifFile}"); + + // Act + var exitCode = Runner.Run( + out var output, + "dotnet", + _dllPath, + "--sarif", sarifFile); + + // Assert + Assert.AreEqual(0, exitCode); + Assert.Contains("SarifMark version", output); + Assert.Contains("SARIF File:", output); + Assert.Contains("Reading SARIF file...", output); + Assert.Contains("Tool: TestTool", output); + Assert.Contains("Results: 1", output); + } + + /// + /// Test that processing a non-existent SARIF file shows error. + /// + [TestMethod] + public void Sarif_NonExistentSarifFile_ShowsError() + { + // Arrange - No special setup needed + + // Act + var exitCode = Runner.Run( + out var output, + "dotnet", + _dllPath, + "--sarif", "nonexistent.sarif"); + + // Assert + Assert.AreEqual(1, exitCode); + Assert.Contains("Error:", output); + } + + /// + /// Test that generating a report file succeeds. + /// + [TestMethod] + public void Sarif_GenerateReport_CreatesReportFile() + { + // Arrange + var sarifFile = PathHelpers.SafePathCombine(_testDataPath, "sample.sarif"); + Assert.IsTrue(File.Exists(sarifFile), $"Test SARIF file not found at {sarifFile}"); + + var reportFile = PathHelpers.SafePathCombine(Path.GetTempPath(), $"test-report-{Guid.NewGuid()}.md"); + + try + { + // Act + var exitCode = Runner.Run( + out var output, + "dotnet", + _dllPath, + "--sarif", sarifFile, + "--report", reportFile); + + // Assert + Assert.AreEqual(0, exitCode); + Assert.Contains("Writing report to", output); + Assert.Contains("Report generated successfully", output); + Assert.IsTrue(File.Exists(reportFile), "Report file was not created"); + + var reportContent = File.ReadAllText(reportFile); + Assert.Contains("# TestTool Analysis", reportContent); + } + finally + { + if (File.Exists(reportFile)) + { + File.Delete(reportFile); + } + } + } + + /// + /// Test that report depth parameter is configurable. + /// + [TestMethod] + public void Sarif_ReportDepth_IsConfigurable() + { + // Arrange + var sarifFile = PathHelpers.SafePathCombine(_testDataPath, "sample.sarif"); + Assert.IsTrue(File.Exists(sarifFile), $"Test SARIF file not found at {sarifFile}"); + + var reportFile = PathHelpers.SafePathCombine(Path.GetTempPath(), $"test-report-depth-{Guid.NewGuid()}.md"); + + try + { + // Act + var exitCode = Runner.Run( + out _, + "dotnet", + _dllPath, + "--sarif", sarifFile, + "--report", reportFile, + "--report-depth", "3"); + + // Assert + Assert.AreEqual(0, exitCode); + Assert.IsTrue(File.Exists(reportFile), "Report file was not created"); + + var reportContent = File.ReadAllText(reportFile); + Assert.Contains("### TestTool Analysis", reportContent); + } + finally + { + if (File.Exists(reportFile)) + { + File.Delete(reportFile); + } + } + } +} diff --git a/test/DemaConsulting.SarifMark.Tests/SelfTest/SelfTestTests.cs b/test/DemaConsulting.SarifMark.Tests/SelfTest/SelfTestTests.cs new file mode 100644 index 0000000..a3373b5 --- /dev/null +++ b/test/DemaConsulting.SarifMark.Tests/SelfTest/SelfTestTests.cs @@ -0,0 +1,63 @@ +// Copyright (c) DEMA Consulting +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +namespace DemaConsulting.SarifMark.Tests; + +/// +/// Subsystem tests for the Self-Validation subsystem. +/// +[TestClass] +public class SelfTestTests +{ + private string _dllPath = string.Empty; + + /// + /// Initialize test by locating the SarifMark DLL. + /// + [TestInitialize] + public void TestInitialize() + { + var baseDir = AppContext.BaseDirectory; + _dllPath = PathHelpers.SafePathCombine(baseDir, "DemaConsulting.SarifMark.dll"); + + Assert.IsTrue(File.Exists(_dllPath), $"Could not find SarifMark DLL at {_dllPath}"); + } + + /// + /// Test that validate flag runs self-validation. + /// + [TestMethod] + public void SelfTest_ValidateFlag_RunsSelfValidation() + { + // Arrange - No special setup needed + + // Act + var exitCode = Runner.Run( + out var output, + "dotnet", + _dllPath, + "--validate"); + + // Assert + Assert.AreEqual(0, exitCode); + Assert.Contains("SarifMark version", output); + Assert.Contains("Total Tests:", output); + } +} diff --git a/test/DemaConsulting.SarifMark.Tests/Utilities/PathHelpersTests.cs b/test/DemaConsulting.SarifMark.Tests/Utilities/PathHelpersTests.cs index ae10037..44300d9 100644 --- a/test/DemaConsulting.SarifMark.Tests/Utilities/PathHelpersTests.cs +++ b/test/DemaConsulting.SarifMark.Tests/Utilities/PathHelpersTests.cs @@ -102,23 +102,23 @@ public void PathHelpers_SafePathCombine_PathWithDoubleDots_ThrowsArgumentExcepti } /// - /// Test that SafePathCombine throws ArgumentException for a filename that contains ".." as a substring. - /// The check uses a strict Contains("..") comparison, so names like "v1..0.sarif" are rejected - /// even though they do not represent path traversal. This is intentional: the design favours - /// rejecting a small set of unusual but valid names over risking traversal edge-cases. + /// Test that SafePathCombine accepts a filename that contains ".." as an embedded substring. + /// The post-combine check uses which treats ".." as an + /// escape segment only when it is the full result or is followed by a directory separator, + /// so names like "v1..0.sarif" are accepted as valid in-base filenames. /// [TestMethod] - public void PathHelpers_SafePathCombine_FilenameWithEmbeddedDots_ThrowsArgumentException() + public void PathHelpers_SafePathCombine_FilenameWithEmbeddedDots_CombinesSuccessfully() { // Arrange - "v1..0.sarif" contains ".." but is not a path traversal var basePath = "/home/user"; var relativePath = "v1..0.sarif"; - // Act & Assert - the strict substring check rejects this name - var exception = Assert.Throws(() => - PathHelpers.SafePathCombine(basePath, relativePath)); - Assert.Contains("Invalid path component", exception.Message); - Assert.AreEqual("relativePath", exception.ParamName); + // Act + var result = PathHelpers.SafePathCombine(basePath, relativePath); + + // Assert + Assert.AreEqual(Path.Combine(basePath, relativePath), result); } /// diff --git a/test/DemaConsulting.SarifMark.Tests/Utilities/UtilitiesTests.cs b/test/DemaConsulting.SarifMark.Tests/Utilities/UtilitiesTests.cs new file mode 100644 index 0000000..2ae2ad7 --- /dev/null +++ b/test/DemaConsulting.SarifMark.Tests/Utilities/UtilitiesTests.cs @@ -0,0 +1,45 @@ +// Copyright (c) DEMA Consulting +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +namespace DemaConsulting.SarifMark.Tests; + +/// +/// Subsystem tests for the Utilities subsystem. +/// +[TestClass] +public class UtilitiesTests +{ + /// + /// Test that the Utilities subsystem provides safe path-handling that combines valid paths. + /// + [TestMethod] + public void Utilities_SafePathHandling_ValidPaths_CombinesSuccessfully() + { + // Arrange + var basePath = Path.GetTempPath(); + var relativePath = "subdirectory/file.txt"; + + // Act + var result = PathHelpers.SafePathCombine(basePath, relativePath); + + // Assert + Assert.AreEqual(Path.Combine(basePath, relativePath), result); + } +}