Skip to content

Conversation

@lukehunterdev
Copy link

Added new threats and service accounts page.

@github-actions
Copy link
Contributor

PR Review: Threat Manager 3.1 Documentation

Thank you for this substantial documentation addition! This PR adds comprehensive documentation for Threat Manager v3.1, including 9,442 additions across 96 files. Below is my review feedback:

✅ Strengths

  1. Comprehensive Coverage: Excellent breadth of documentation covering administration, configuration, investigations, playbooks, and threat management
  2. Consistent Structure: Files follow a consistent frontmatter pattern with title, description, and sidebar_position
  3. Good Use of Documentation Features: Appropriate use of admonitions (:::note, :::info, :::tip), code blocks, and links
  4. Well-Organized Hierarchy: Logical folder structure with proper category.json files for navigation

🔍 Issues Found

1. Image Path Issues (Priority: High)

Multiple files reference images with inconsistent paths:

  • serviceaccounts.md:13 uses /images/threatmanager/3.1/... (correct for v3.1)
  • threats.md:19 uses /images/threatmanager/3.0/... (references v3.0 instead of v3.1)
  • overview.md:11 uses /images/threatmanager/3.0/... (references v3.0 instead of v3.1)
  • activedirectorysync.md:60 uses /images/threatprevention/7.5/... (wrong product entirely!)

Recommendation: Audit all image paths to ensure they reference the correct product version. According to the repository README, images should be at /img/product_docs/productname/. The current paths use /images/ instead of /img/. Please verify:

  • Are these images committed in the correct location?
  • Should paths use /img/ or /images/?
  • All v3.1 documentation should reference v3.1 images, not v3.0

2. Typo in activedirectorysync.md:10 (Priority: Medium)

to theThreat Manager database

Should be: to the Threat Manager database (missing space)

3. Undefined Variable in playbooks/overview.md:114 (Priority: High)

This allows a (Undefined variable: SD.Product Short Name) administrator

This appears to be a template variable that wasn't properly replaced. Should reference "Threat Manager" directly.

4. Link Formatting (Priority: Low)

Some links are correctly using absolute paths from /docs/, which is good for a Docusaurus site. Verify all internal links resolve correctly after deployment.

5. Inconsistent Heading Hierarchy (Priority: Low)

Some files use proper heading hierarchy (h1 → h2 → h3), while others jump levels. Maintain consistent hierarchy for better accessibility and SEO.

📋 Recommendations

  1. Test Image Paths: Run a local build (npm run build) to verify all images load correctly
  2. Fix Template Variables: Search for any remaining template variables like (Undefined variable: across all files
  3. Version Consistency: Ensure all v3.1 documentation references v3.1 resources, not v3.0
  4. Cross-Reference Validation: Test all internal links to ensure they resolve correctly
  5. Consider Adding:
    • Alt text for images (accessibility)
    • More code examples where applicable
    • Troubleshooting sections for common issues

🔒 Security & Performance

  • ✅ No security concerns identified
  • ✅ No performance issues expected (documentation only)
  • ✅ Using .webp format for images (good for performance)

📝 Test Coverage

This is documentation-only, but recommend:

  • Verifying all links work in the built site
  • Testing navigation through the sidebar structure
  • Ensuring search functionality indexes the new content properly

Summary

This is a well-structured documentation PR with good content organization. The main issues are:

  1. Image path inconsistencies (some reference v3.0, wrong product, or potentially wrong base path)
  2. One undefined template variable that needs fixing
  3. Minor typo to correct

Please address the high-priority issues (image paths and undefined variable) before merging. The documentation quality is otherwise excellent!


Review conducted by Claude Code

@github-actions
Copy link
Contributor

Pull Request Review: Threat Manager 3.1 Documentation

Overview

This PR adds comprehensive documentation for Threat Manager version 3.1, including 97 new markdown files totaling over 9,400 additions. The documentation covers administration, configuration, integrations, playbooks, threats, and installation topics. Additionally, the products.js configuration has been correctly updated to set version 3.1 as the latest version.

✅ Strengths

Documentation Structure

  • Well-organized hierarchy: The documentation follows a logical structure with clear categorization (administration, threats, investigations, playbooks, configuration)
  • Consistent formatting: All files use proper YAML frontmatter with title, description, and sidebar_position
  • Comprehensive coverage: Documentation covers all major features including new Service Accounts dashboard (v3.1)
  • Good use of Docusaurus features: Proper use of admonitions (:::note, :::tip, :::info), cross-references, and image links

Content Quality

  • Detailed procedural documentation: Step-by-step instructions are clear and numbered
  • Rich visual support: Extensive use of screenshots with proper alt text and descriptive paths
  • Cross-referencing: Good internal linking between related topics
  • New features documented: Service Accounts dashboard properly documented with column definitions

Configuration

  • Correct version setup: Version 3.1 properly configured as latest in products.js (lines 567-570)
  • Sidebar configuration: Proper sidebar file reference (./sidebars/threatmanager/3.1.js)

⚠️ Issues Found

1. Missing Spaces After Punctuation (High Priority)

Location: docs/threatmanager/3.1/administration/configuration/integrations/activedirectorysync.md:11

environment (users, groups, hosts, etc).See the

Fix: Add space before "See"

environment (users, groups, hosts, etc). See the

Location: docs/threatmanager/3.1/administration/investigations/reports.md:127

Click the link to view target details.See the [Host Details Page]

Fix: Add space before "See"

2. Insecure HTTP Links (Medium Priority)

Found 5 files with http:// links. While some may be example URLs or localhost references, they should be reviewed:

  • docs/threatmanager/3.1/install/firstlaunch/login.md
  • docs/threatmanager/3.1/install/secure.md
  • docs/threatmanager/3.1/install/actionservice.md
  • docs/threatmanager/3.1/administration/configuration/integrations/email.md (line 64: http://localhost:8080/)
  • docs/threatmanager/3.1/administration/configuration/integrations/netwrixintegrations.md

Recommendation:

  • For http://localhost:8080/, this is acceptable as an example/default value
  • Other HTTP links should be verified and converted to HTTPS where applicable, especially for external documentation links

3. Typos (Low Priority)

Location: src/config/products.js:185

description: 'Access custsomer resources and training materials',

Fix: "custsomer" → "customer"

Location: docs/threatmanager/3.1/administration/configuration/integrations/email.md:27

use a semicolon as a seperator

Fix: "seperator" → "separator" (also on line 60)

4. Image Path Consistency

Many files reference images from the 3.0 version directory rather than 3.1:

![headerbarleft](/images/threatmanager/3.0/administration/headerbarleft.webp)

Impact: This may be intentional if images are reused, but should be verified:

  • If 3.1 has UI changes, images should be updated to reflect the new version
  • If images are identical, the current approach is acceptable for maintainability

📋 Recommendations

High Priority (Before Merge)

  1. Fix spacing issues after punctuation (2 occurrences found)
  2. Fix typo in products.js ("custsomer" → "customer")

Medium Priority (Consider Before Merge)

  1. Review HTTP links - Verify if any external documentation links should use HTTPS
  2. Verify image accuracy - Confirm that 3.0 images accurately represent 3.1 UI, or update paths if needed

Low Priority (Future Improvements)

  1. Fix typo in email.md ("seperator" → "separator")
  2. Consider adding:
    • A "What's New in 3.1" section highlighting Service Accounts and other new features
    • Comparison table between 3.0 and 3.1 features
    • Migration guide for users upgrading from 3.0 to 3.1

🔒 Security Considerations

No security concerns identified:

  • Credential handling documentation properly references secure practices
  • No hardcoded credentials or sensitive data exposed
  • SSL/TLS configuration properly documented
  • Authentication and authorization topics well covered

📊 Testing Coverage

Recommendation: Before merge, verify:

  • All internal documentation links resolve correctly
  • All image paths are valid
  • Sidebar navigation works as expected
  • Search indexing includes new content

Summary

This is a substantial and well-structured documentation update. The content is comprehensive and follows good documentation practices. The issues identified are minor and mostly cosmetic (spacing, typos). The most critical fix is the missing space after punctuation in 2 locations.

Recommendation: ✅ Approve with minor fixes - Address the spacing issues and typo in products.js before merging.


Reviewed by: Claude Sonnet 4.5 🤖

@github-actions
Copy link
Contributor

Pull Request Review: Threat Manager 3.1 Documentation

Overview

This PR adds substantial documentation for Threat Manager 3.1, including a new service accounts page and comprehensive threats documentation. The PR adds 9,442 lines across 98 files, primarily markdown documentation and JSON configuration files.


✅ Strengths

Documentation Structure

  • Well-organized hierarchy: The documentation follows a logical structure with clear categories (administration, configuration, threats, investigations, playbooks, etc.)
  • Consistent frontmatter: All markdown files use proper YAML frontmatter with title, description, and sidebar_position
  • Category configuration: All _category_.json files are properly structured with labels, positions, and links

Content Quality

  • Comprehensive coverage: The documentation thoroughly covers all aspects of Threat Manager 3.1, including:
    • Administration and configuration interfaces
    • Integration pages (Active Directory Sync, Entra ID Sync, SIEM, Email, etc.)
    • Threat detection and response mechanisms
    • Investigations and playbooks
    • System health and settings
  • Clear writing: Documentation is generally well-written with step-by-step instructions
  • Good use of admonitions: Appropriate use of :::note and :::info blocks for important information

Best Practices

  • Consistent naming conventions: File and folder names follow kebab-case convention
  • Proper image references: Images use the /images/ prefix for static assets
  • Link structure: Internal links use proper /docs/ paths with version numbers

⚠️ Issues Found

1. Image Path Versioning (Medium Priority)

Many files reference images from version 3.0 instead of 3.1:

Examples:

  • docs/threatmanager/3.1/administration/overview.md:11/images/threatmanager/3.0/administration/headerbarleft.webp
  • docs/threatmanager/3.1/administration/threats/threats.md:19/images/threatmanager/3.0/administration/threatspage.webp
  • Multiple files in administration/playbooks/, administration/investigations/, etc.

Recommendation:

  • Verify if these 3.0 images are correct for 3.1 documentation or if new 3.1 screenshots are needed
  • If the UI hasn't changed, this is acceptable, but should be documented
  • If UI has changed, update images and paths to /images/threatmanager/3.1/

2. HTTP URLs (Low Priority - Informational)

Several files contain http:// URLs for localhost references:

Files:

  • administration/configuration/integrations/email.md:64
  • administration/configuration/integrations/netwrixintegrations.md:69
  • install/firstlaunch/login.md:14-15

Recommendation:

  • These appear to be legitimate localhost URLs for configuration examples
  • Consider noting that HTTPS should be used in production environments

3. Typo in activedirectorysync.md (Low Priority)

Line 88: "recheck your entries for error" → should be "for errors" (plural)

4. Step Numbering Inconsistency (Low Priority)

In administration/configuration/integrations/activedirectorysync.md, the section "Active Directory Sync Policy Details" starts at Step 7 instead of Step 1. This appears to be continuing from the previous section's numbering, which may confuse readers.

Recommendation: Each section should start numbering from Step 1.


🔍 Code Quality Assessment

Documentation Standards

  • ✅ Proper markdown syntax throughout
  • ✅ Consistent heading hierarchy (H1 → H2 → H3)
  • ✅ Well-formatted tables and lists
  • ✅ Appropriate use of code blocks and inline code

Accessibility

  • ✅ All images have alt text descriptions
  • ✅ Links have descriptive text (not "click here")
  • ⚠️ Some images could benefit from more descriptive alt text beyond filename descriptions

Maintainability

  • ✅ Clear file organization makes updates straightforward
  • ✅ JSON configuration files are properly formatted
  • ✅ No hardcoded values that should be variables

🔒 Security Considerations

No Security Issues Found

  • ✅ No exposed credentials or API keys
  • ✅ Password fields are appropriately described without actual passwords
  • ✅ Security best practices are mentioned in relevant sections
  • ✅ Proper guidance on credential profiles and authentication

🎯 Performance Considerations

Documentation Size

  • The PR is quite large (9,442 additions) but this is expected for comprehensive product documentation
  • All images use .webp format, which is optimal for web performance ✅
  • No performance concerns identified

🧪 Test Coverage

Documentation Testing Recommendations

  1. Build Test: Verify the documentation builds successfully with Docusaurus
  2. Link Validation: Run a link checker to ensure all internal links resolve correctly
  3. Image Validation: Verify all referenced images exist in the static/images/ directory
  4. Version Navigation: Test that version switcher works correctly between 3.0 and 3.1

📋 Recommendations

Before Merging

  1. Verify image paths: Confirm whether 3.0 images are intentional or need updating to 3.1
  2. Fix step numbering: Start each major section at Step 1 for clarity
  3. Fix minor typo: "for error" → "for errors" in activedirectorysync.md

Post-Merge

  1. Create visual regression tests: Consider screenshots for major UI changes
  2. User feedback: Monitor for user questions that might indicate unclear documentation
  3. Version migration guide: Consider adding a "What's New in 3.1" page highlighting changes from 3.0

🎉 Summary

This is high-quality, comprehensive documentation that significantly improves the Threat Manager 3.1 product documentation. The structure is excellent, the content is thorough, and there are only minor issues to address.

Overall Assessment

  • Code Quality: ⭐⭐⭐⭐⭐ (5/5)
  • Content Quality: ⭐⭐⭐⭐⭐ (5/5)
  • Best Practices: ⭐⭐⭐⭐½ (4.5/5)
  • Ready to Merge: ✅ Yes, with minor improvements recommended

Great work on this documentation update! 🚀

@lukehunterdev lukehunterdev marked this pull request as ready for review January 27, 2026 17:38
@lukehunterdev lukehunterdev requested a review from a team as a code owner January 27, 2026 17:38
Updated the description of the Service Accounts dashboard to clarify its purpose and functionality.
Copy link
Contributor

@kdejoyce kdejoyce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed and approved with 1 minor change. Do not merge until release.

@github-actions
Copy link
Contributor

Pull Request Review: Threat Manager 3.1 Documentation

Thank you for this comprehensive documentation addition! This PR adds substantial new content for Threat Manager 3.1, including the new Service Accounts page and Threats page along with extensive administration documentation.

Summary

  • Files Changed: 140 files
  • Additions: 9,440 lines
  • Deletions: 2 lines
  • Scope: Complete administration documentation for Threat Manager 3.1

Positive Aspects

Documentation Structure

  • Well-organized hierarchical structure with clear categorization
  • Proper use of _category_.json files for Docusaurus navigation
  • Consistent use of frontmatter (title, description, sidebar_position)
  • Good separation of concerns across different sections (Configuration, Investigations, Playbooks, Threats, etc.)

Content Quality

  • Clear and comprehensive: Documentation provides good coverage of features with detailed explanations
  • User-focused: Includes step-by-step instructions and helpful visual aids
  • Contextual information: Good use of notes, warnings, and cross-references to related topics
  • Professional tone: Consistent, clear technical writing throughout

Technical Implementation

  • Proper Docusaurus configuration with correct JSON schema for categories
  • Appropriate use of MDX features and markdown formatting
  • Good navigation structure with logical positioning values

Issues Found

Critical: Mixed Image Path Versions

Issue: The documentation uses image paths from two different versions (3.0 and 3.1):

  • 266 occurrences of /images/threatmanager/3.0/ paths across 85 files
  • 1 occurrence of /images/threatmanager/3.1/ paths across 1 file

Impact: This creates version inconsistency and may lead to:

  • Incorrect or outdated screenshots being displayed
  • Confusion for users if images don't match the current version's UI
  • Maintenance issues when updating to future versions

Example locations:

  • docs/threatmanager/3.1/administration/overview.md:11 - Uses 3.0 images
  • docs/threatmanager/3.1/administration/threats/threats.md:19 - Uses 3.0 images
  • docs/threatmanager/3.1/administration/serviceaccounts.md:11 - Uses 3.1 images (correct)

Recommendation:

  1. Verify if v3.1 has UI changes that require new screenshots
  2. If UI is identical to 3.0, document this decision explicitly
  3. If UI has changed, update all image paths to use /images/threatmanager/3.1/ and ensure corresponding images exist
  4. Add a comment in the documentation about image versioning strategy

Minor: Description Repetition

Issue: Many files have identical title and description in frontmatter.

Example:

---
title: "Service Accounts"
description: "Service Accounts"
---

Recommendation: Enhance descriptions to be more SEO-friendly and informative:

---
title: "Service Accounts"
description: "Manage and monitor service account usage across your environment with the Service Accounts dashboard"
---

This improves:

  • Search engine optimization (SEO)
  • Search result snippets
  • User understanding before clicking through

Minor: External Link References

Issue: Some external documentation links reference specific URLs that may change:

  • Microsoft documentation links with long URLs
  • Other Netwrix product documentation links

Recommendation: Consider whether these links should:

  1. Use relative paths for internal Netwrix docs (if they're part of the same doc site)
  2. Be validated periodically to ensure they don't break
  3. Include archived/versioned links where appropriate

Style & Best Practices

Strengths

  • Consistent heading hierarchy
  • Good use of admonitions (notes, warnings)
  • Clear table formatting for column definitions
  • Proper markdown list formatting
  • Good use of bold for UI elements and field names

Suggestions

  1. Consistency in step formatting: Some steps use "Step 1 –" while others might benefit from ordered lists
  2. Alt text for images: Most images have basic descriptions, but could be more descriptive for accessibility
  3. Code blocks: Consider adding language identifiers to any code snippets (though minimal in this PR)

Security Considerations

No security concerns identified. The documentation:

  • Does not expose credentials or sensitive information
  • Appropriately references credential profiles without showing actual credentials
  • Includes proper guidance on permissions and security configurations

Test Coverage

N/A - This is documentation only (no code changes requiring tests)

Recommendation: Consider adding:

  • A documentation review checklist
  • Link validation tests in CI/CD
  • Spell-check automation
  • Markdown linting rules

Performance Considerations

No performance concerns. The documentation:

  • Uses appropriate image formats (.webp for most images)
  • Has reasonable file sizes
  • Follows Docusaurus best practices

Recommendations Summary

Must Fix

  1. Resolve image path versioning: Decide on strategy for 3.0 vs 3.1 image paths and standardize

Should Fix

  1. Enhance frontmatter descriptions for better SEO
  2. Validate external links don't 404

Nice to Have

  1. Improve image alt text for accessibility
  2. Consider adding a docs contribution guide
  3. Add automated link checking to CI/CD

Overall Assessment

This is a high-quality documentation PR that adds significant value to the Threat Manager product. The content is well-written, properly structured, and follows Docusaurus conventions. The primary concern is the image path versioning inconsistency which should be addressed before merging.

Recommendation: ✅ Approve with minor revisions - Address the image path versioning issue, then this is ready to merge.

Great work on this comprehensive documentation update! 🎉

@kdejoyce kdejoyce deployed to development January 27, 2026 19:52 — with GitHub Actions Active
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants