222 lines
14 KiB
Markdown
222 lines
14 KiB
Markdown
# Codebase Concerns
|
|
|
|
**Analysis Date:** 2026-04-02
|
|
|
|
## Tech Debt
|
|
|
|
**Silent Error Handling (Widespread):**
|
|
- Issue: 38 empty `catch` blocks that suppress errors without logging
|
|
- Files: `Sharepoint_ToolBox.ps1` (lines 1018, 1020, 1067, 1068, 1144, 2028, 2030, etc.)
|
|
- Impact: Failures go unnoticed, making debugging difficult. Users don't know why operations fail. Error conditions are hidden from logs.
|
|
- Fix approach: Add logging to all `catch` blocks. Use `BgLog` for background tasks, `Write-Log` for UI threads. Example: `catch { BgLog "Folder enumeration failed: $_" "DarkGray" }` instead of `catch {}`
|
|
|
|
**Resource Cleanup Issues:**
|
|
- Issue: Runspace and PowerShell objects created in background jobs may not be properly disposed if exceptions occur
|
|
- Files: `Sharepoint_ToolBox.ps1` lines 1040-1052, 4564-4577, 5556-5577
|
|
- Impact: Memory leaks possible if UI interactions are interrupted. Zombie runspaces could accumulate over multiple operations.
|
|
- Fix approach: Wrap all runspace/PS object creation in try-finally blocks. Ensure `$rs.Dispose()` and `$ps.Dispose()` are called in finally block, not just in success path
|
|
|
|
**Overly Broad Error Suppression:**
|
|
- Issue: 27 instances of `-ErrorAction SilentlyContinue` spread throughout code
|
|
- Files: `Sharepoint_ToolBox.ps1` (e.g., lines 1142, 1188, 2070, 4436, etc.)
|
|
- Impact: Real failures indistinguishable from expected failures (e.g., list doesn't exist vs. connection failed). Masks bugs.
|
|
- Fix approach: Use selective error suppression. Only suppress when you've explicitly checked for the condition (e.g., "if list doesn't exist, create it"). Otherwise use `-ErrorAction Stop` with explicit try-catch.
|
|
|
|
**Inconsistent JSON Error Handling:**
|
|
- Issue: JSON parsing in `Load-Profiles`, `Load-Settings`, `Load-Templates` uses empty catch blocks
|
|
- Files: `Sharepoint_ToolBox.ps1` lines 61, 140, 568-570
|
|
- Impact: Corrupted JSON files silently fail and return empty defaults, losing user data silently
|
|
- Fix approach: Log actual error message. Implement validation schema. Create backup of corrupted files.
|
|
|
|
## Known Bugs
|
|
|
|
**Blank Client ID Warning Not Actionable:**
|
|
- Symptoms: "WARNING: No Client ID returned" appears but doesn't prevent further operations or clear user input
|
|
- Files: `Sharepoint_ToolBox.ps1` line 4237
|
|
- Trigger: Azure AD app registration completes but returns null ClientId (can happen with certain tenant configurations)
|
|
- Workaround: Manually register app via Azure Portal and paste Client ID
|
|
- Fix approach: Check for null ClientId before continuing, clear the warning state properly
|
|
|
|
**Group Member Addition Silent Failures:**
|
|
- Symptoms: Members appear not to be added to sites, but no error shown in UI
|
|
- Files: `Sharepoint_ToolBox.ps1` lines 1222-1225, 5914, 5922 (try-catch with SilentlyContinue)
|
|
- Trigger: User exists but cannot be added to group (permissions, licensing, or source-specific SP group issues)
|
|
- Workaround: Manual group membership assignment
|
|
- Fix approach: Replace SilentlyContinue with explicit logging of why Add-PnPGroupMember failed
|
|
|
|
**Folder Metadata Loss in Template Application:**
|
|
- Symptoms: Folder permissions captured correctly but not reapplied when permissions=true but structure already exists
|
|
- Files: `Sharepoint_ToolBox.ps1` lines 1229-1292 (folder-level permission application depends on library structure map being built first)
|
|
- Trigger: Target library created by Apply-FolderTree but permissions application logic expects library to already exist in template structure
|
|
- Workaround: Delete and recreate target library, or manually apply permissions via SharePoint UI
|
|
- Fix approach: Build library map before applying folder tree, or add validation that all referenced libraries exist
|
|
|
|
**CSV Import for Bulk Operations Not Validated:**
|
|
- Symptoms: Invalid CSV format silently fails, users see no clear error, form appears unresponsive
|
|
- Files: `Sharepoint_ToolBox.ps1` lines 5765-5800 (CSV parsing with inadequate error context)
|
|
- Trigger: CSV with missing headers, wrong delimiter, or invalid format
|
|
- Workaround: Edit CSV manually to match expected format, restart tool
|
|
- Fix approach: Add CSV schema validation before processing, show specific validation errors
|
|
|
|
## Security Considerations
|
|
|
|
**Client ID and Tenant URL Hardcoded in Temp Files:**
|
|
- Risk: Temp registration script contains unencrypted Client ID and Tenant ID in plaintext
|
|
- Files: `Sharepoint_ToolBox.ps1` lines 4210-4245 (temp file creation)
|
|
- Current mitigation: Temp file cleanup attempted but not guaranteed if process crashes
|
|
- Recommendations: Use SecureString to pass credentials, delete temp file with -Force in finally block, or use named pipes instead of files
|
|
|
|
**No Validation of Tenant URL Format:**
|
|
- Risk: Arbitrary URLs accepted, could be typos leading to authentication against wrong tenant
|
|
- Files: `Sharepoint_ToolBox.ps1` lines 4306-4317, 30-43 (Validate-Inputs)
|
|
- Current mitigation: URL used as-is, relies on PnP authentication failure to catch issues
|
|
- Recommendations: Add regex validation for SharePoint tenant URLs, warn on suspicious patterns
|
|
|
|
**Profile File Contains Credentials in Plaintext:**
|
|
- Risk: `Sharepoint_Export_profiles.json` contains Client ID and Tenant URL in plaintext on disk
|
|
- Files: `Sharepoint_ToolBox.ps1` lines 50-72 (profile persistence)
|
|
- Current mitigation: File located in user home directory (Windows ACL protection), but still plaintext
|
|
- Recommendations: Consider encrypting profile file with DPAPI, or move to Windows Credential Manager
|
|
|
|
**PnP PowerShell Module Trust Not Validated:**
|
|
- Risk: Module imported without version pinning, could load compromised version
|
|
- Files: `Sharepoint_ToolBox.ps1` lines 151, 1151, 4218, 4521, 5833 (Import-Module PnP.PowerShell)
|
|
- Current mitigation: None
|
|
- Recommendations: Pin module version in manifest, use `-MinimumVersion` parameter, check module signature
|
|
|
|
## Performance Bottlenecks
|
|
|
|
**Synchronous UI Freezes During Large Operations:**
|
|
- Problem: File search with 50,000 result limit processes all results at once, building HTML string in memory
|
|
- Files: `Sharepoint_ToolBox.ps1` lines 2112-2133 (Export-SearchResultsToHTML builds entire table in string)
|
|
- Cause: All results concatenated into single `$rows` string before sending to UI
|
|
- Improvement path: Implement pagination in HTML reports, stream results rather than buffering all in memory. For large datasets, chunk exports into multiple files.
|
|
|
|
**Folder Storage Recursion Not Depth-Limited by Default:**
|
|
- Problem: `Collect-FolderStorage` recurses unlimited depth unless explicitly capped, can take hours on deep folder structures
|
|
- Files: `Sharepoint_ToolBox.ps1` lines 2009-2032, 4432-4455
|
|
- Cause: CurrentDepth compared against FolderDepth limit, but FolderDepth defaults to 999 if not set
|
|
- Improvement path: Default to depth 3-4, show estimated scan time based on depth, implement cancellation token
|
|
|
|
**No Parallel Processing for Multiple Sites:**
|
|
- Problem: Sites processed sequentially in Permissions/Storage reports, one site blocks all others
|
|
- Files: `Sharepoint_ToolBox.ps1` lines 4379-4401 (foreach loop in permissions scan)
|
|
- Cause: Single-threaded approach with `Connect-PnPOnline` context switches
|
|
- Improvement path: Queue-based processing for multiple sites (partially done for storage scans), implement async context management
|
|
|
|
**HTML Report Generation for Large Duplicates List:**
|
|
- Problem: Export-DuplicatesToHTML builds entire HTML in memory, slow for 10,000+ duplicates
|
|
- Files: `Sharepoint_ToolBox.ps1` lines 2235-2400 (HTML string concatenation in loop)
|
|
- Cause: All groups converted to HTML before writing to file
|
|
- Improvement path: Stream HTML generation, write to file incrementally, implement lazy-loading tables in browser
|
|
|
|
## Fragile Areas
|
|
|
|
**Language System (T() function):**
|
|
- Files: `Sharepoint_ToolBox.ps1` (translation lookups throughout, ~15 hardcoded English fallbacks like "Veuillez renseigner")
|
|
- Why fragile: Language loading can fail silently, UI control updates hardcoded at multiple locations, no fallback chain for missing translations
|
|
- Safe modification: Add validation that all UI strings have corresponding translation keys before form creation. Create helper function that returns English default if key missing.
|
|
- Test coverage: No tests for translation system. Gaps: Missing translations for error messages, hardcoded "Veuillez renseigner" strings that bypass T() function
|
|
|
|
**Profile Management:**
|
|
- Files: `Sharepoint_ToolBox.ps1` lines 50-127
|
|
- Why fragile: Profile list is in-memory array that syncs with JSON file. If Save-Profiles fails, changes are lost. No transaction semantics.
|
|
- Safe modification: Implement write-lock pattern. Create backup before write. Validate JSON before replacing file.
|
|
- Test coverage: No validation that profile save actually persists. Race condition if opened in multiple instances.
|
|
|
|
**Runspace State Machine:**
|
|
- Files: `Sharepoint_ToolBox.ps1` lines 1034-1095, 4580-4650 (runspace creation, async timer polling)
|
|
- Why fragile: UI state (`btnGenPerms.Enabled = false`) set before runspace begins, but no explicit state reset if runspace crashes or hangs
|
|
- Safe modification: Implement state enum (Idle, Running, Done, Error). Always reset state in finally block. Set timeout on runspace execution.
|
|
- Test coverage: No timeout tests. Gaps: What happens if runspace hangs indefinitely? Button remains disabled forever.
|
|
|
|
**Site Picker List View:**
|
|
- Files: `Sharepoint_ToolBox.ps1` lines 157-250 (_Pkl-* functions)
|
|
- Why fragile: AllSites list updates while UI may be reading it (SuppressCheck flag used but incomplete synchronization)
|
|
- Safe modification: Use proper locking or rebuild entire list atomically. Current approach relies on flag which may miss updates.
|
|
- Test coverage: No concurrent access tests. Gaps: What if site is checked while sort is happening?
|
|
|
|
## Scaling Limits
|
|
|
|
**Permissions Report HTML with Large Item Counts:**
|
|
- Current capacity: Tested up to ~5,000 items, performance degrades significantly
|
|
- Limit: HTML table becomes unusable in browser above 10,000 rows (sorting, filtering slow)
|
|
- Scaling path: Implement client-side virtual scrolling in HTML template, paginate into multiple reports, add server-side filtering before export
|
|
|
|
**File Search Result Limit:**
|
|
- Current capacity: 50,000 result maximum hardcoded
|
|
- Limit: Beyond 50,000 files, results truncated without warning
|
|
- Scaling path: Implement pagination in SharePoint Search API, show "more results available" warning, allow user to refine search
|
|
|
|
**Runspace Queue Processing:**
|
|
- Current capacity: Single queue per scan, sequential processing
|
|
- Limit: If background job produces messages faster than timer dequeues, queue could grow unbounded
|
|
- Scaling path: Implement back-pressure (slow producer if queue > 1000 items), implement priority queue
|
|
|
|
**Profile JSON File Size:**
|
|
- Current capacity: Profiles loaded entirely into memory, no limit on file size
|
|
- Limit: If user creates 1,000+ profiles, JSON file becomes slow to load/save
|
|
- Scaling path: Implement profile paging, index file by profile name, lazy-load profile details
|
|
|
|
## Dependencies at Risk
|
|
|
|
**PnP.PowerShell Module Version Mismatch:**
|
|
- Risk: Module API changes between major versions, cmdlet parameter changes
|
|
- Impact: Features relying on specific cmdlet parameters break silently
|
|
- Migration plan: Pin to stable version range in script header. Create version compatibility matrix. Test against 2-3 stable versions.
|
|
|
|
**System.Windows.Forms Dependency:**
|
|
- Risk: WinForms support in PowerShell 7 is deprecated, future versions may not ship it
|
|
- Impact: GUI completely broken on future PowerShell versions
|
|
- Migration plan: Consider migrating to WPF or cross-platform GUI framework (Avalonia). Current WinForms code is tied to Assembly loading.
|
|
|
|
## Missing Critical Features
|
|
|
|
**No Operation Cancellation:**
|
|
- Problem: Running operations (permissions scan, storage metrics, file search) cannot be stopped mid-execution
|
|
- Blocks: User stuck waiting for slow operations to complete, no way to abort except kill process
|
|
|
|
**No Audit Log:**
|
|
- Problem: No record of who ran what operation, what results were exported, when last backup occurred
|
|
- Blocks: Compliance, troubleshooting
|
|
|
|
**No Dry-Run for Most Operations:**
|
|
- Problem: Only version cleanup has dry-run. Permission changes, site creation applied immediately without preview
|
|
- Blocks: Prevents risk assessment before making changes
|
|
|
|
## Test Coverage Gaps
|
|
|
|
**PnP Connection Failures:**
|
|
- What's not tested: Connection timeouts, intermittent network issues, authentication failures mid-operation
|
|
- Files: `Sharepoint_ToolBox.ps1` lines 36, 157, 170, 2036, 4458, etc. (Connect-PnPOnline calls)
|
|
- Risk: Tool may hang indefinitely if connection drops. No retry logic.
|
|
- Priority: High
|
|
|
|
**Malformed JSON Resilience:**
|
|
- What's not tested: Templates.json, Profiles.json, Settings.json with invalid JSON, missing fields, type mismatches
|
|
- Files: `Sharepoint_ToolBox.ps1` lines 61, 140, 568 (ConvertFrom-Json)
|
|
- Risk: Tool fails to start or loses user data
|
|
- Priority: High
|
|
|
|
**Large-Scale Operations:**
|
|
- What's not tested: Permissions scan on site with 50,000+ items, storage metrics on 10,000+ folders, file search returning 40,000+ results
|
|
- Files: Bulk scanning functions throughout
|
|
- Risk: Memory exhaustion, timeout, UI freeze
|
|
- Priority: Medium
|
|
|
|
**Runspace Cleanup on Error:**
|
|
- What's not tested: Runspace exception handling, cleanup if UI window closes during background operation
|
|
- Files: `Sharepoint_ToolBox.ps1` lines 1040-1052, 4564-4577, 5556-5577
|
|
- Risk: Zombie processes, resource leaks
|
|
- Priority: Medium
|
|
|
|
**CSV Format Validation:**
|
|
- What's not tested: Invalid column headers, wrong delimiter, missing required columns in bulk operations
|
|
- Files: `Sharepoint_ToolBox.ps1` lines 5765-5800
|
|
- Risk: Silent failures, partial data import
|
|
- Priority: Medium
|
|
|
|
---
|
|
|
|
*Concerns audit: 2026-04-02*
|