8000 feat: enhance docs-analysis action security and error handling · coder/coder@ea54314 · GitHub
[go: up one dir, main page]
Skip to content

Commit ea54314

Browse files
EdwardAngertclaude
EdwardAngert
and
claude
committed
feat: enhance docs-analysis action security and error handling
- Implement strict whitelist input validation for branch references - Add path traversal detection to sanitize_path function - Replace eval with direct command execution in git_with_retry - Add error tracing with line numbers for better debugging - Add performance monitoring and metrics generation - Update README with security enhancements 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent a4d3d94 commit ea54314

File tree

2 files changed

+169
-19
lines changed

2 files changed

+169
-19
lines changed

‎.github/actions/docs-analysis/README.md

Lines changed: 101 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ A composite GitHub Action to analyze documentation changes in pull requests and
99
- Tracks image modifications with detailed reporting
1010
- Analyzes document structure (headings, titles)
1111
- Identifies the most significantly changed files
12+
- Integrates with other doc workflows (weekly checks, PR previews)
1213
- Provides standardized outputs that can be used by any workflow
1314

1415
## Usage
@@ -28,6 +29,28 @@ It only runs on PRs that modify files in the docs directory or markdown files el
2829
base-ref: 'main'
2930
```
3031
32+
### Integration with tj-actions/changed-files (Recommended)
33+
34+
For optimal performance and reliability, we recommend using with `tj-actions/changed-files`:
35+
36+
```yaml
37+
- uses: tj-actions/changed-files@v45
38+
id: changed-files
39+
with:
40+
files: |
41+
docs/**
42+
**.md
43+
separator: ","
44+
45+
- name: Analyze Documentation Changes
46+
id: docs-analysis
47+
uses: ./.github/actions/docs-analysis
48+
with:
49+
docs-path: 'docs/'
50+
changed-files: ${{ steps.changed-files.outputs.all_changed_files }}
51+
files-pattern: 'docs/**|**.md'
52+
```
53+
3154
### Complete Example with Conditionals
3255

3356
```yaml
@@ -40,13 +63,20 @@ jobs:
4063
with:
4164
fetch-depth: 0
4265
66+
- uses: tj-actions/changed-files@v45
67+
id: changed-files
68+
with:
69+
files: |
70+
docs/**
71+
**.md
72+
separator: ","
73+
4374
- name: Analyze Documentation Changes
4475
uses: ./.github/actions/docs-analysis
4576
id: docs-analysis
4677
with:
4778
docs-path: 'docs/'
48-
pr-ref: ${{ github.event.pull_request.head.ref }}
49-
base-ref: 'main'
79+
changed-files: ${{ steps.changed-files.outputs.all_changed_files }}
5080
significant-words-threshold: '100'
5181
skip-if-no-docs: 'true'
5282
debug-mode: 'false'
@@ -71,15 +101,18 @@ jobs:
71101
| Name | Description | Required | Default |
72102
|------|-------------|----------|---------|
73103
| `docs-path` | Path to the documentation directory | No | `docs/` |
104+
| `files-pattern` | Glob pattern(s) for documentation files (use vertical bar \| to separate multiple patterns) | No | `**.md\|docs/**` |
105+
| `changed-files` | Comma-separated list of changed files (from tj-actions/changed-files) | No | `` |
74106
| `pr-ref` | PR reference to analyze | No | `github.event.pull_request.head.ref` |
75107
| `base-ref` | Base reference to compare against | No | `main` |
76-
| `files-changed` | Comma-separated list of files changed (alternative to git diff) | No | `` |
108+
| `files-changed` | Comma-separated list of files changed (legacy input, use `changed-files` instead) | No | `` |
77109
| `max-scan-files` | Maximum number of files to scan | No | `100` |
78110
| `max-files-to-analyze` | Maximum files to analyze in detail (for performance) | No | `20` |
79111
| `throttle-large-repos` | Enable throttling for large repositories | No | `true` |
80112
| `significant-words-threshold` | Threshold for significant text changes | No | `100` |
81113
| `skip-if-no-docs` | Whether to skip if no docs files are changed | No | `true` |
82114
| `debug-mode` | Enable verbose debugging output | No | `false` |
115+
| `use-changed-files-action` | Whether to use tj-actions/changed-files instead of git commands | No | `false` |
83116

84117
## Outputs
85118

@@ -107,12 +140,14 @@ jobs:
107140

108141
## Security Features
109142

110-
- Input validation to prevent command injection
111-
- Path sanitization for safer file operations
112-
- Git command retry logic for improved reliability
143+
- Stronger input validation with whitelist approach for branch references
144+
- Enhanced path sanitization with traversal detection
145+
- Secure command execution (no eval) in git retry operations
146+
- Error tracing with line numbers for better debugging
113147
- Cross-platform compatibility with fallbacks
114148
- Repository size detection with adaptive throttling
115149
- Python integration for safer JSON handling (with bash fallbacks)
150+
- Performance monitoring with execution metrics
116151

117152
## Performance Optimization
118153

@@ -155,4 +190,63 @@ jobs:
155190
with:
156191
docs-path: 'docs/'
157192
debug-mode: 'true'
158-
```
193+
```
194+
195+
## Unified Documentation Workflows
196+
197+
This action is designed to work seamlessly with Coder's other documentation-related workflows:
198+
199+
### How to Use with docs-ci.yaml
200+
201+
The `docs-ci.yaml` workflow uses this action to analyze documentation changes for linting and formatting:
202+
203+
```yaml
204+
# From .github/workflows/docs-ci.yaml
205+
- uses: tj-actions/changed-files@v45
206+
id: changed-files
207+
with:
208+
files: |
209+
docs/**
210+
**.md
211+
separator: ","
212+
213+
- name: Analyze documentation changes
214+
id: docs-analysis
215+
uses: ./.github/actions/docs-analysis
216+
with:
217+
docs-path: "docs/"
218+
changed-files: ${{ steps.changed-files.outputs.all_changed_files }}
219+
files-pattern: "docs/**|**.md"
220+
```
221+
222+
### How to Use with docs-preview-link.yml
223+
224+
This action can be used in the `docs-preview-link.yml` workflow to analyze documentation changes for preview generation:
225+
226+
```yaml
227+
# Example integration with docs-preview-link.yml
228+
- name: Analyze documentation changes
229+
id: docs-analysis
230+
uses: ./.github/actions/docs-analysis
231+
with:
232+
docs-path: "docs/"
233+
pr-ref: ${{ steps.pr_info.outputs.branch_name }}
234+
base-ref: 'main'
235+
```
236+
237+
### How to Use with weekly-docs.yaml
238+
239+
This action can be used to enhance the weekly documentation checks:
240+
241+
```yaml
242+
# Example integration with weekly-docs.yaml
243+
- name: Analyze documentation structure
244+
id: docs-analysis
245+
uses: ./.github/actions/docs-analysis
246+
with:
247+
docs-path: "docs/"
248+
files-pattern: "docs/**"
249+
max-scan-files: "500" # Higher limit for full repo scan
250+
```
251+
252+
By using this shared action across all documentation workflows, you ensure consistent analysis, metrics, and reporting for all documentation-related tasks.

‎.github/actions/docs-analysis/action.yml

Lines changed: 68 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -146,14 +146,14 @@ runs:
146146
echo "::warning::Documentation path '${{ inputs.docs-path }}' does not exist - some functions may not work correctly"
147147
fi
148148
149-
# Validate branch references for command injection prevention
150-
if [[ "${{ inputs.pr-ref }}" =~ [;&|$"'`] ]]; then
151-
echo "::error::Invalid characters in pr-ref"
149+
# Validate branch references with strict whitelist approach for better security
150+
if [[ ! "${{ inputs.pr-ref }}" =~ ^[a-zA-Z0-9_\-\.\/]+$ ]]; then
151+
echo "::error::Invalid characters in pr-ref - only alphanumeric, underscore, hyphen, dot, and forward slash are allowed"
152152
exit 1
153153
fi
154154
155-
if [[ "${{ inputs.base-ref }}" =~ [;&|$"'`] ]]; then
156-
echo "::error::Invalid characters in base-ref"
155+
if [[ ! "${{ inputs.base-ref }}" =~ ^[a-zA-Z0-9_\-\.\/]+$ ]]; then
156+
echo "::error::Invalid characters in base-ref - only alphanumeric, underscore, hyphen, dot, and forward slash are allowed"
157157
exit 1
158158
fi
159159
@@ -224,25 +224,37 @@ runs:
224224
id: verify
225225
shell: bash
226226
run: |
227+
# Add error tracing for better debugging and recovery
228+
trap 'echo "::error::Error occurred in verify docs changes at line $LINENO"' ERR
227229
# Helper functions for better error handling and path sanitization
228230
function handle_error() {
229231
echo "::error::$1"
230232
echo "docs_changed=false" >> $GITHUB_OUTPUT
231233
exit 1
232234
}
233235
236+
# More secure path sanitization with validation
234237
function sanitize_path() {
235-
echo "$1" | sed 's/[;&|"`$]/\\&/g'
238+
local path="$1"
239+
240+
# Check for path traversal attempts or absolute paths if needed
241+
if [[ "$path" == *".."* || "$path" == "/"* ]]; then
242+
echo "::error::Invalid path containing directory traversal patterns or absolute reference"
243+
return 1
244+
fi
245+
246+
# Sanitize the path - escape special characters
247+
echo "$path" | sed 's/[;&|"`$]/\\&/g'
236248
}
237249
238250
# Retry function for git operations to handle potential rate limiting
251+
# Uses direct command execution instead of eval for better security
239252
function git_with_retry() {
240253
local max_retries=3
241-
local cmd="$@"
242254
local retry_count=0
243255
244256
while [[ $retry_count -lt $max_retries ]]; do
245-
if eval "$cmd"; then
257+
if "$@"; then # Direct execution instead of eval
246258
return 0
247259
fi
248260
@@ -251,7 +263,7 @@ runs:
251263
sleep $((retry_count * 2))
252264
done
253265
254-
echo "::warning::Git operation failed after $max_retries retries: $cmd"
266+
echo "::warning::Git operation failed after $max_retries retries"
255267
return 1
256268
}
257269
@@ -503,9 +515,22 @@ runs:
503515
set -x
504516
fi
505517
518+
# Add error tracing for better debugging and recovery
519+
trap 'echo "::error::Error occurred in document structure analysis at line $LINENO"' ERR
520+
506521
# Helper functions
522+
# More secure path sanitization with validation
507523
function sanitize_path() {
508-
echo "$1" | sed 's/[;&|"`$]/\\&/g'
524+
local path="$1"
525+
526+
# Check for path traversal attempts or absolute paths if needed
527+
if [[ "$path" == *".."* || "$path" == "/"* ]]; then
528+
echo "::error::Invalid path containing directory traversal patterns or absolute reference"
529+
return 1
530+
fi
531+
532+
# Sanitize the path - escape special characters
533+
echo "$path" | sed 's/[;&|"`$]/\\&/g'
509534
}
510535
511536
function json_escape() {
@@ -656,9 +681,22 @@ print(json.dumps(doc_structure))
656681
if: steps.verify.outputs.docs_changed == 'true'
657682
shell: bash
658683
run: |
684+
# Add error tracing for better debugging and recovery
685+
trap 'echo "::error::Error occurred in finding changed files at line $LINENO"' ERR
686+
659687
# Helper functions
688+
# More secure path sanitization with validation
660689
function sanitize_path() {
661-
echo "$1" | sed 's/[;&|"`$]/\\&/g'
690+
local path="$1"
691+
692+
# Check for path traversal attempts or absolute paths if needed
693+
if [[ "$path" == *".."* || "$path" == "/"* ]]; then
694+
echo "::error::Invalid path containing directory traversal patterns or absolute reference"
695+
return 1
696+
fi
697+
698+
# Sanitize the path - escape special characters
699+
echo "$path" | sed 's/[;&|"`$]/\\&/g'
662700
}
663701
664702
# Enable debug output if requested
@@ -815,4 +853,22 @@ print(json.dumps(doc_structure))
815853
START_TIME="${{ steps.timing.outputs.start_time }}"
816854
DURATION=$((END_TIME - START_TIME))
817855
echo "duration=$DURATION" >> $GITHUB_OUTPUT
818-
echo "Docs analysis completed in ${DURATION}s"
856+
857+
# Output for CI monitoring systems
858+
if [[ $DURATION -gt 30 ]]; then
859+
echo "::warning::Docs analysis took ${DURATION}s to complete - consider optimizing"
860+
else
861+
echo "::notice::Docs analysis completed in ${DURATION}s"
862+
fi
863+
864+
# Create execution metrics JSON for potential monitoring integration
865+
mkdir -p .github/temp
866+
cat > .github/temp/docs-analysis-metrics.json << EOF
867+
{
868+
"execution_time": $DURATION,
869+
"timestamp": "$(date -u +"%Y-%m-%dT%H:%M:%SZ")",
870+
"repository": "${{ github.repository }}",
871+
"workflow": "${{ github.workflow }}",
872+
"action": "docs-analysis"
873+
}
874+
EOF

0 commit comments

Comments
 (0)
0