-
Notifications
You must be signed in to change notification settings - Fork 193
feat: Add support for custom guidance and appendices in HTML reports #467
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
feat: Add support for custom guidance and appendices in HTML reports #467
Conversation
- Add TemplateConfig class to detect and process custom HTML content files - Support custom-guidance.html and custom-appendices.html files in working directory - Implement proper HTML escaping for JavaScript injection safety - Add navigation visibility controls based on content availability - Empty custom files hide corresponding navigation sections - Fall back to default content when custom files don't exist - Include comprehensive test coverage for all scenarios
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for custom guidance and appendices content in Cloudsplaining HTML reports by allowing users to provide custom HTML files that override default content sections.
- Adds
TemplateConfig
class to detect and process custom HTML files (custom-guidance.html
andcustom-appendices.html
) - Implements conditional navigation visibility based on content availability
- Integrates custom content rendering in Vue.js components with proper HTML escaping
Reviewed Changes
Copilot reviewed 8 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
test/shared/test_template_config.py |
Comprehensive test suite covering all template configuration scenarios |
cloudsplaining/shared/template_config.py |
Core logic for detecting, loading, and processing custom content files |
cloudsplaining/output/template.html |
JavaScript variable definitions for template configuration |
cloudsplaining/output/src/views/Guidance.vue |
Vue component updated to render custom or default guidance content |
cloudsplaining/output/src/views/Appendices.vue |
Vue component updated to render custom or default appendices content |
cloudsplaining/output/src/routes/routes.js |
Dynamic route registration based on navigation visibility flags |
cloudsplaining/output/src/App.vue |
Navigation menu updates with conditional visibility and code formatting |
cloudsplaining/output/report.py |
Integration of TemplateConfig into the report generation process |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gave some feedback. Some additional requests:
- In your PR description, can you give instructions on how others would use these changes? It seems like it does not affect any command line args, right? Only if you are wrapping Cloudsplaining as a library?
- It would be great if you would include these changes in the documentation as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see these two new chunk files. I haven't looked in a while, but I believe in the template.html we are specifically injecting index.js
and chunk-vendors.js
. Are these chunk files intended to be injected into the HTML template or should they be part of the chunk-vendors.js?
I want to make sure we aren't breaking anyone. I realize we don't have any Playwright tests to validate the html file works as expected, but you can test this by Installing the package from a different virtual environment.
# Remove those chunk files
# Navigate to a different directory
cd ../different-project
python3 -m venv venv
source venv/bin/activate
pip3 install /path/to/cloudsplaining/dev
# run cloudsplaining commands to generate a report
# view the report and see if there are errors
Can you let me know how that goes?
</b-navbar-nav> | ||
</b-collapse> | ||
</b-navbar> | ||
|
||
<b-container class="mt-3 pb-3 report"> | ||
<b-tabs nav-class="d-none"> | ||
<router-view /> | ||
<!-- <b-tab key="task-table">--> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like a lot of these changes are formatting changes that don't reflect functionality changes which makes it difficult to review. If possible, it would be great if you could revert changes that are only related to formatting.
{path: '/iam-principals', component: IamPrincipals}, | ||
]; | ||
|
||
if (typeof window !== 'undefined' && window.show_guidance_nav === "True") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain the rationale/approach here and any potential side effects?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typeof window !== 'undefined'
: prevents errors during server-side rendering or node.js environments where window doesn't exist.
window.show_guidance_nav === "True"
: checks if the python backend set this variable to true
The purpose is to conditionally add routes only only when the custom guidance or appendices should be shown.
The potential side effects:
This approach conditionally registers routes only when needed (potentially reducing bundle size), safely handles server-side rendering environments where window is undefined, and works reliably since the window.show_guidance_nav
value is set once during report generation and never changes during the user session.
Alternatively I could move this logic to the Vue component level instead of the route level but in my humble opinion the current approach is cleaner for navigation.
What does this PR do?
This PR adds support for customizable guidance and appendices content in Cloudsplaining HTML reports. Users can now provide their own HTML content files to customize the guidance and appendices sections of their reports.
Key features:
custom-guidance.html
andcustom-appendices.html
files in their working directoryHow to use this feature
Use Case 1: Custom Guidance Content
Result: Report shows your custom guidance content instead of default AWS guidance.
Use Case 2: Custom Appendices with Additional Resources
Result: Report shows custom appendices content instead of default AWS appendices
Use Case 3: Hide Sections Completely
Result: No Guidance or Appendices tabs appear in the navigation
Use Case 4: Mixed Configuration
Result: Custom guidance tab + default AWS appendices tab
Use Case 5: Default Behavior (No Changes)
# Don't create any custom files - works exactly as before cloudsplaining scan --input-file default.json --output reports/
Result: Standard AWS guidance and appendices content (existing behavior)
File Requirements
custom-guidance.html
andcustom-appendices.html
cloudsplaining scan
commandWhat gif best describes this PR or how it makes you feel?
Completion checklist
make test
,make lint
,make security-test
,make test-js
)