fix: force WBS task level to secondary#1611
Open
Mxturl wants to merge 1 commit into
Open
Conversation
|
杜励承 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
📝 WalkthroughWalkthroughThe PR rewrites the standup-report skill into a daily/weekly/WBS workflow, adds rule docs and test prompts, and introduces cross-platform scripts for Lark source collection, local agent evidence gathering, candidate review generation, and document publishing. ChangesLark Standup Report Workflow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/lark-workflow-standup-report/scripts/collect_agent_evidence.ps1`:
- Around line 229-233: The path exclusion in collect_agent_evidence.ps1 is too
specific and only matches a nested dist/cache segment, so standalone dist or
cache directories still get counted. Update the Where-Object filter in the
recentFiles pipeline to exclude dist and cache as separate path segments,
alongside node_modules, .git, and __pycache__, so generated files from either
directory do not affect has_today_files.
- Around line 221-223: The project discovery loop in collect_agent_evidence.ps1
only inspects subdirectories from each $ProjectRoots entry, so a root path
itself is never considered a project candidate. Update the logic around the
foreach over $ProjectRoots and Get-ChildItem so each root path is first checked
and added as a candidate when appropriate, then continue scanning its child
directories as before. Use the existing project_candidates collection logic in
the same block to keep behavior consistent for single-repo invocations.
In `@skills/lark-workflow-standup-report/scripts/collect_agent_evidence.sh`:
- Around line 34-35: The project root handling in collect_agent_evidence.sh
skips the supplied root itself and only scans its child directories, which can
drop root-level work from the evidence set. Update the PROJECT_ROOTS traversal
logic so the directory passed in via the default "$PWD" or a direct repo path is
treated as a candidate project and scanned before iterating into its
subdirectories, using the existing project-root discovery flow around
PROJECT_ROOTS and the scanning logic near the referenced block.
In `@skills/lark-workflow-standup-report/scripts/collect_lark_daily.ps1`:
- Around line 426-427: The manifest is being rewritten after the redaction pass,
so the final artifact still contains raw sensitive fields. Update the flow
around Protect-JsonFiles and Save-Manifest in collect_lark_daily.ps1 so the
manifest is either saved before redaction or excluded from the redaction step
and then sanitized before completion. Make sure the Save-Manifest output is
protected the same way as the other JSON artifacts, preserving the redacted
current_user_* fields and error text.
- Around line 258-266: The current_user parsing in collect_lark_daily.ps1 only
reads the nested contact +get-user shape, so it misses the flat legacy response
and leaves currentUserOpenId null. Update the logic in the current_user block to
mirror the Bash fallback used for the same response, using the data.user fields
first and falling back to the flat data fields for both open_id and name so the
im_self branch still runs when the CLI returns the legacy shape.
In `@skills/lark-workflow-standup-report/scripts/collect_lark_daily.sh`:
- Around line 145-152: The batching logic in collect_lark_daily.sh is splitting
the joined meeting ID list by character width using the chunk loop that feeds
try_lark vc_notes, which can break IDs mid-token. Replace the paste/fold
pipeline with token-aware batching that reads individual meeting IDs and
accumulates them into comma-separated chunks based on length limits, ensuring
each --meeting-ids argument contains only complete IDs. Apply the same fix to
the duplicated chunking logic referenced in the later section as well.
In `@skills/lark-workflow-standup-report/scripts/create_daily_doc.ps1`:
- Around line 49-55: The create_daily_doc.ps1 publish flow is not matching the
Bash contract: it uses $WikiNode directly instead of normalizing /wiki/... URLs
to a parent token, and it only checks $LASTEXITCODE instead of the API response
payload. Update the lark-cli docs +create call in this script to use the same
arguments as the Bash path (including --api-version v2 and --format json),
normalize the wiki URL before passing the parent token, and parse the returned
JSON to fail when .ok is false even if the process exits successfully.
In `@skills/lark-workflow-standup-report/scripts/lark_common.ps1`:
- Line 75: The PowerShell helper in lark_common.ps1 writes $OutFile directly
with Set-Content, so fresh work/... paths can fail if the parent directory does
not exist. Update the script to create the output directory first in the same
flow that builds $payload and writes $OutFile, using the existing path variables
so the directory is ensured before Set-Content runs.
- Around line 104-114: Protect-Text is not redacting bearer-style Authorization
headers, so update the redaction logic in Protect-Text to catch Authorization
values that use the Bearer scheme as well as the existing token patterns. Add or
adjust the regex handling near the current authorization-token replacement so
any “Authorization: Bearer …” or similar header value is replaced with
[REDACTED] before artifacts are saved.
- Around line 77-94: The helper in lark_common.ps1 only raises on timeout or
JSON ok=false, so non-zero lark-cli exits can still be treated as success.
Update the command execution flow in the script helper that returns the
pscustomobject so it also checks the process exit code from $proc.ExitCode and
throws on any non-zero result unless it is the expected ok=false case already
handled. Keep the existing JSON parsing behavior in the same helper, and make
sure stderr is surfaced in the thrown error for debugging.
In `@skills/lark-workflow-standup-report/scripts/lark_common.sh`:
- Around line 97-101: manifest_set_files_array is passing the manifest path
after jq --args, which causes jq to treat the manifest filename as positional
data instead of an input file. Update the jq invocation in
manifest_set_files_array so the manifest is provided as jq input and only the
array values are passed as positional arguments, preserving the .files[$key]
assignment and keeping the temp-file mv flow unchanged.
- Around line 61-70: The redaction logic in redact_file() is missing
bearer-style Authorization headers, so add a replacement pattern for
Authorization: Bearer credentials alongside the existing token and secret masks.
Update the perl redaction list in lark_common.sh so any Authorization header
values emitted by lark-cli are replaced with [REDACTED], while keeping the
current handling for api_key, access/refresh tokens, secrets, passwords, and
private keys intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 86f4ac07-846f-4829-88a4-da58adf89794
📒 Files selected for processing (18)
skills/lark-workflow-standup-report/.gitattributesskills/lark-workflow-standup-report/.gitignoreskills/lark-workflow-standup-report/SKILL.mdskills/lark-workflow-standup-report/agents/openai.yamlskills/lark-workflow-standup-report/references/daily-report-rules.mdskills/lark-workflow-standup-report/references/lark-source-commands.mdskills/lark-workflow-standup-report/references/weekly-wbs-rules.mdskills/lark-workflow-standup-report/scripts/build_candidate_review.ps1skills/lark-workflow-standup-report/scripts/build_candidate_review.shskills/lark-workflow-standup-report/scripts/collect_agent_evidence.ps1skills/lark-workflow-standup-report/scripts/collect_agent_evidence.shskills/lark-workflow-standup-report/scripts/collect_lark_daily.ps1skills/lark-workflow-standup-report/scripts/collect_lark_daily.shskills/lark-workflow-standup-report/scripts/create_daily_doc.ps1skills/lark-workflow-standup-report/scripts/create_daily_doc.shskills/lark-workflow-standup-report/scripts/lark_common.ps1skills/lark-workflow-standup-report/scripts/lark_common.shskills/lark-workflow-standup-report/test-prompts.json
Comment on lines
+221
to
+223
| foreach ($root in $ProjectRoots) { | ||
| if (-not (Test-Path -LiteralPath $root)) { continue } | ||
| Get-ChildItem -LiteralPath $root -Directory -ErrorAction SilentlyContinue | ForEach-Object { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Treat each $ProjectRoots entry as a candidate project too.
This only scans child directories under each root. If the caller passes a single repo path, changes at the repo root never produce a project_candidates entry, so single-project invocations miss evidence.
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] Missing BOM encoding for non-ASCII encoded file 'collect_agent_evidence.ps1'
(PSUseBOMForUnicodeEncodedFile)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/lark-workflow-standup-report/scripts/collect_agent_evidence.ps1`
around lines 221 - 223, The project discovery loop in collect_agent_evidence.ps1
only inspects subdirectories from each $ProjectRoots entry, so a root path
itself is never considered a project candidate. Update the logic around the
foreach over $ProjectRoots and Get-ChildItem so each root path is first checked
and added as a candidate when appropriate, then continue scanning its child
directories as before. Use the existing project_candidates collection logic in
the same block to keep behavior consistent for single-repo invocations.
Comment on lines
+229
to
+233
| $recentFiles = @(Get-ChildItem -LiteralPath $_.FullName -Recurse -File -ErrorAction SilentlyContinue | | ||
| Where-Object { | ||
| (Test-InRange $_.LastWriteTime) -and | ||
| ($_.FullName -notmatch '\\node_modules\\|\\.git\\|\\dist\\cache\\|\\__pycache__\\') | ||
| } | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The exclusion pattern never filters dist or cache on their own.
\\dist\\cache\\ matches a literal \dist\cache\ segment, not \dist\ or \cache\ individually. Generated files from either directory can therefore be counted as “today files” and incorrectly promote a project to has_today_files.
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] Missing BOM encoding for non-ASCII encoded file 'collect_agent_evidence.ps1'
(PSUseBOMForUnicodeEncodedFile)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/lark-workflow-standup-report/scripts/collect_agent_evidence.ps1`
around lines 229 - 233, The path exclusion in collect_agent_evidence.ps1 is too
specific and only matches a nested dist/cache segment, so standalone dist or
cache directories still get counted. Update the Where-Object filter in the
recentFiles pipeline to exclude dist and cache as separate path segments,
alongside node_modules, .git, and __pycache__, so generated files from either
directory do not affect has_today_files.
Comment on lines
+34
to
+35
| [[ ${#PROJECT_ROOTS[@]} -eq 0 ]] && PROJECT_ROOTS=("$PWD") | ||
| CODEX_HOME_DIR="${CODEX_HOME:-$HOME/.codex}" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Scan the supplied project root itself, not only its children.
Right now a directory entry in PROJECT_ROOTS is treated as a container of projects. With the default "$PWD" or a direct repo path, the root project itself is skipped and only its subdirectories are considered, so root-level work can disappear from the evidence set.
Also applies to: 215-223
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/lark-workflow-standup-report/scripts/collect_agent_evidence.sh` around
lines 34 - 35, The project root handling in collect_agent_evidence.sh skips the
supplied root itself and only scans its child directories, which can drop
root-level work from the evidence set. Update the PROJECT_ROOTS traversal logic
so the directory passed in via the default "$PWD" or a direct repo path is
treated as a candidate project and scanned before iterating into its
subdirectories, using the existing project-root discovery flow around
PROJECT_ROOTS and the scanning logic near the referenced block.
Comment on lines
+258
to
+266
| $currentUserOpenId = $null | ||
| try { | ||
| $file = Join-Path $OutDir "current_user_$Date.json" | ||
| Invoke-LarkCliJson -Args @('contact', '+get-user', '--as', 'user', '--format', 'json') -OutFile $file -TimeoutSeconds $RequestTimeoutSeconds | Out-Null | ||
| $manifest.files.current_user = $file | ||
| $j = Read-JsonFileOrNull -Path $file | ||
| $currentUserOpenId = $j.data.user.open_id | ||
| $manifest.current_user_name = $j.data.user.name | ||
| $manifest.current_user_open_id = $currentUserOpenId |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
Add fallback handling for flat contact +get-user response shapes
The Bash collector correctly handles both the nested contact +get-user shape (.data.user.open_id) and the flat legacy shape (.data.open_id) using the // operator. The PowerShell implementation at lines 264–266 only accesses the nested path. If the CLI returns the flat shape, $currentUserOpenId remains null, causing the im_self branch to be skipped entirely.
Update the code to mirror the Bash fallback logic:
$currentUserOpenId = $j.data.user.open_id // $j.data.open_id
$manifest.current_user_name = $j.data.user.name // $j.data.name🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/lark-workflow-standup-report/scripts/collect_lark_daily.ps1` around
lines 258 - 266, The current_user parsing in collect_lark_daily.ps1 only reads
the nested contact +get-user shape, so it misses the flat legacy response and
leaves currentUserOpenId null. Update the logic in the current_user block to
mirror the Bash fallback used for the same response, using the data.user fields
first and falling back to the flat data fields for both open_id and name so the
im_self branch still runs when the CLI returns the legacy shape.
Comment on lines
+426
to
+427
| Protect-JsonFiles -Directory $OutDir | ||
| Save-Manifest |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
The final manifest write undoes the redaction pass.
Protect-JsonFiles redacts every *.json in $OutDir, including source_manifest.json, and then Save-Manifest immediately rewrites the raw manifest. That leaves the final artifact with unprotected current_user_* fields and raw error text.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/lark-workflow-standup-report/scripts/collect_lark_daily.ps1` around
lines 426 - 427, The manifest is being rewritten after the redaction pass, so
the final artifact still contains raw sensitive fields. Update the flow around
Protect-JsonFiles and Save-Manifest in collect_lark_daily.ps1 so the manifest is
either saved before redaction or excluded from the redaction step and then
sanitized before completion. Make sure the Save-Manifest output is protected the
same way as the other JSON artifacts, preserving the redacted current_user_*
fields and error text.
| $stderr = $stderrTask.GetAwaiter().GetResult() | ||
| $payload = if ($stdout.Trim()) { $stdout } else { $stderr } | ||
|
|
||
| Set-Content -LiteralPath $OutFile -Value $payload -Encoding UTF8 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Create the output directory before writing $OutFile.
The Bash helper does mkdir -p first, but this PowerShell path writes directly with Set-Content. Any fresh work/... target will fail on Windows before the payload is persisted.
Suggested fix
+ $outDir = Split-Path -Parent $OutFile
+ if ($outDir) {
+ New-Item -ItemType Directory -Path $outDir -Force | Out-Null
+ }
Set-Content -LiteralPath $OutFile -Value $payload -Encoding UTF8📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Set-Content -LiteralPath $OutFile -Value $payload -Encoding UTF8 | |
| $outDir = Split-Path -Parent $OutFile | |
| if ($outDir) { | |
| New-Item -ItemType Directory -Path $outDir -Force | Out-Null | |
| } | |
| Set-Content -LiteralPath $OutFile -Value $payload -Encoding UTF8 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/lark-workflow-standup-report/scripts/lark_common.ps1` at line 75, The
PowerShell helper in lark_common.ps1 writes $OutFile directly with Set-Content,
so fresh work/... paths can fail if the parent directory does not exist. Update
the script to create the output directory first in the same flow that builds
$payload and writes $OutFile, using the existing path variables so the directory
is ensured before Set-Content runs.
Comment on lines
+77
to
+94
| try { | ||
| $json = $payload | ConvertFrom-Json | ||
| if ($json -and $json.PSObject.Properties['ok'] -and $json.ok -eq $false) { | ||
| $errorType = if ($json.PSObject.Properties['error'] -and $json.error.PSObject.Properties['type']) { $json.error.type } else { 'unknown' } | ||
| $errorMessage = if ($json.PSObject.Properties['error'] -and $json.error.PSObject.Properties['message']) { $json.error.message } else { 'lark-cli returned ok=false' } | ||
| throw "lark-cli returned ok=false ($errorType): $errorMessage" | ||
| } | ||
| } catch { | ||
| if ($_.Exception.Message -like 'lark-cli returned ok=false*') { throw } | ||
| } | ||
|
|
||
| return [pscustomobject]@{ | ||
| ExitCode = $proc.ExitCode | ||
| Stdout = $stdout | ||
| Stderr = $stderr | ||
| OutFile = $OutFile | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Non-zero lark-cli exits are treated as success.
This helper only throws on timeout or JSON ok=false. A plain exit code failure with stderr text still returns normally, so downstream Windows scripts can keep going with an error file while the Bash path would already have failed.
Suggested fix
Set-Content -LiteralPath $OutFile -Value $payload -Encoding UTF8
+
+ if ($proc.ExitCode -ne 0) {
+ throw "lark-cli failed with exit code $($proc.ExitCode): $($Args -join ' ')"
+ }
try {
$json = $payload | ConvertFrom-Json📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try { | |
| $json = $payload | ConvertFrom-Json | |
| if ($json -and $json.PSObject.Properties['ok'] -and $json.ok -eq $false) { | |
| $errorType = if ($json.PSObject.Properties['error'] -and $json.error.PSObject.Properties['type']) { $json.error.type } else { 'unknown' } | |
| $errorMessage = if ($json.PSObject.Properties['error'] -and $json.error.PSObject.Properties['message']) { $json.error.message } else { 'lark-cli returned ok=false' } | |
| throw "lark-cli returned ok=false ($errorType): $errorMessage" | |
| } | |
| } catch { | |
| if ($_.Exception.Message -like 'lark-cli returned ok=false*') { throw } | |
| } | |
| return [pscustomobject]@{ | |
| ExitCode = $proc.ExitCode | |
| Stdout = $stdout | |
| Stderr = $stderr | |
| OutFile = $OutFile | |
| } | |
| } | |
| if ($proc.ExitCode -ne 0) { | |
| throw "lark-cli failed with exit code $($proc.ExitCode): $($Args -join ' ')" | |
| } | |
| try { | |
| $json = $payload | ConvertFrom-Json | |
| if ($json -and $json.PSObject.Properties['ok'] -and $json.ok -eq $false) { | |
| $errorType = if ($json.PSObject.Properties['error'] -and $json.error.PSObject.Properties['type']) { $json.error.type } else { 'unknown' } | |
| $errorMessage = if ($json.PSObject.Properties['error'] -and $json.error.PSObject.Properties['message']) { $json.error.message } else { 'lark-cli returned ok=false' } | |
| throw "lark-cli returned ok=false ($errorType): $errorMessage" | |
| } | |
| } catch { | |
| if ($_.Exception.Message -like 'lark-cli returned ok=false*') { throw } | |
| } | |
| return [pscustomobject]@{ | |
| ExitCode = $proc.ExitCode | |
| Stdout = $stdout | |
| Stderr = $stderr | |
| OutFile = $OutFile | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/lark-workflow-standup-report/scripts/lark_common.ps1` around lines 77
- 94, The helper in lark_common.ps1 only raises on timeout or JSON ok=false, so
non-zero lark-cli exits can still be treated as success. Update the command
execution flow in the script helper that returns the pscustomobject so it also
checks the process exit code from $proc.ExitCode and throws on any non-zero
result unless it is the expected ok=false case already handled. Keep the
existing JSON parsing behavior in the same helper, and make sure stderr is
surfaced in the thrown error for debugging.
Comment on lines
+104
to
+114
| function Protect-Text { | ||
| param([Parameter(Mandatory = $true)][AllowEmptyString()][string] $Text) | ||
|
|
||
| $safe = $Text | ||
| $safe = $safe -replace '(?i)(Key:\s*)[A-Za-z0-9+/=_-]{16,}', '$1[REDACTED]' | ||
| $safe = $safe -replace '(?i)(api[_ -]?key["''\s:=]+)[A-Za-z0-9+/=_-]{12,}', '$1[REDACTED]' | ||
| $safe = $safe -replace '(?i)((?:access|refresh|tenant|user|app|authorization)[_-]?token["''\s:=]+)[A-Za-z0-9._+/=_-]{16,}', '$1[REDACTED]' | ||
| $safe = $safe -replace '(?i)(secret["''\s:=]+)[A-Za-z0-9._+/=_-]{12,}', '$1[REDACTED]' | ||
| $safe = $safe -replace '(?i)(password["''\s:=]+)[^\s,''"}]{6,}', '$1[REDACTED]' | ||
| $safe = $safe -replace '(?i)(private[-_ ]?key["''\s:=]+)[A-Za-z0-9._+/=_-]{16,}', '$1[REDACTED]' | ||
| return $safe |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Redaction misses bearer-style Authorization headers.
Protect-Text still allows Authorization: Bearer ... values through, so a failed CLI call can leave live access tokens in the saved JSON artifacts.
Suggested fix
$safe = $safe -replace '(?i)(Key:\s*)[A-Za-z0-9+/=_-]{16,}', '$1[REDACTED]'
$safe = $safe -replace '(?i)(api[_ -]?key["''\s:=]+)[A-Za-z0-9+/=_-]{12,}', '$1[REDACTED]'
+ $safe = $safe -replace '(?i)(authorization["''\s:=]+bearer\s+)[A-Za-z0-9._~+/=-]{16,}', '$1[REDACTED]'
$safe = $safe -replace '(?i)((?:access|refresh|tenant|user|app|authorization)[_-]?token["''\s:=]+)[A-Za-z0-9._+/=_-]{16,}', '$1[REDACTED]'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function Protect-Text { | |
| param([Parameter(Mandatory = $true)][AllowEmptyString()][string] $Text) | |
| $safe = $Text | |
| $safe = $safe -replace '(?i)(Key:\s*)[A-Za-z0-9+/=_-]{16,}', '$1[REDACTED]' | |
| $safe = $safe -replace '(?i)(api[_ -]?key["''\s:=]+)[A-Za-z0-9+/=_-]{12,}', '$1[REDACTED]' | |
| $safe = $safe -replace '(?i)((?:access|refresh|tenant|user|app|authorization)[_-]?token["''\s:=]+)[A-Za-z0-9._+/=_-]{16,}', '$1[REDACTED]' | |
| $safe = $safe -replace '(?i)(secret["''\s:=]+)[A-Za-z0-9._+/=_-]{12,}', '$1[REDACTED]' | |
| $safe = $safe -replace '(?i)(password["''\s:=]+)[^\s,''"}]{6,}', '$1[REDACTED]' | |
| $safe = $safe -replace '(?i)(private[-_ ]?key["''\s:=]+)[A-Za-z0-9._+/=_-]{16,}', '$1[REDACTED]' | |
| return $safe | |
| function Protect-Text { | |
| param([Parameter(Mandatory = $true)][AllowEmptyString()][string] $Text) | |
| $safe = $Text | |
| $safe = $safe -replace '(?i)(Key:\s*)[A-Za-z0-9+/=_-]{16,}', '$1[REDACTED]' | |
| $safe = $safe -replace '(?i)(api[_ -]?key["''\s:=]+)[A-Za-z0-9+/=_-]{12,}', '$1[REDACTED]' | |
| $safe = $safe -replace '(?i)(authorization["''\s:=]+bearer\s+)[A-Za-z0-9._~+/=-]{16,}', '$1[REDACTED]' | |
| $safe = $safe -replace '(?i)((?:access|refresh|tenant|user|app|authorization)[_-]?token["''\s:=]+)[A-Za-z0-9._+/=_-]{16,}', '$1[REDACTED]' | |
| $safe = $safe -replace '(?i)(secret["''\s:=]+)[A-Za-z0-9._+/=_-]{12,}', '$1[REDACTED]' | |
| $safe = $safe -replace '(?i)(password["''\s:=]+)[^\s,''"}]{6,}', '$1[REDACTED]' | |
| $safe = $safe -replace '(?i)(private[-_ ]?key["''\s:=]+)[A-Za-z0-9._+/=_-]{16,}', '$1[REDACTED]' | |
| return $safe |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/lark-workflow-standup-report/scripts/lark_common.ps1` around lines 104
- 114, Protect-Text is not redacting bearer-style Authorization headers, so
update the redaction logic in Protect-Text to catch Authorization values that
use the Bearer scheme as well as the existing token patterns. Add or adjust the
regex handling near the current authorization-token replacement so any
“Authorization: Bearer …” or similar header value is replaced with [REDACTED]
before artifacts are saved.
Comment on lines
+61
to
+70
| redact_file() { | ||
| local file=$1 | ||
| perl -0pi \ | ||
| -e 's/(Key:\s*)[A-Za-z0-9+\/=_-]{16,}/$1[REDACTED]/gi' \ | ||
| -e 's/(api[_ -]?key["'\''\s:=]+)[A-Za-z0-9+\/=_-]{12,}/$1[REDACTED]/gi' \ | ||
| -e 's/((?:access|refresh|tenant|user|app|authorization)[_-]?token["'\''\s:=]+)[A-Za-z0-9._+\/=_-]{16,}/$1[REDACTED]/gi' \ | ||
| -e 's/(secret["'\''\s:=]+)[A-Za-z0-9._+\/=_-]{12,}/$1[REDACTED]/gi' \ | ||
| -e 's/(password["'\''\s:=]+)[^\s,'\''"}]{6,}/$1[REDACTED]/gi' \ | ||
| -e 's/(private[-_ ]?key["'\''\s:=]+)[A-Za-z0-9._+\/=_-]{16,}/$1[REDACTED]/gi' \ | ||
| "$file" 2>/dev/null || true |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Redaction misses bearer-style Authorization headers.
If lark-cli emits HTTP headers or debug payloads, Authorization: Bearer ... tokens are currently persisted unredacted. That leaks reusable credentials into the work artifacts this helper is supposed to sanitize.
Suggested fix
perl -0pi \
-e 's/(Key:\s*)[A-Za-z0-9+\/=_-]{16,}/$1[REDACTED]/gi' \
-e 's/(api[_ -]?key["'\''\s:=]+)[A-Za-z0-9+\/=_-]{12,}/$1[REDACTED]/gi' \
+ -e 's/(authorization["'\''\s:=]+bearer\s+)[A-Za-z0-9._~+\/=-]{16,}/$1[REDACTED]/gi' \
-e 's/((?:access|refresh|tenant|user|app|authorization)[_-]?token["'\''\s:=]+)[A-Za-z0-9._+\/=_-]{16,}/$1[REDACTED]/gi' \📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| redact_file() { | |
| local file=$1 | |
| perl -0pi \ | |
| -e 's/(Key:\s*)[A-Za-z0-9+\/=_-]{16,}/$1[REDACTED]/gi' \ | |
| -e 's/(api[_ -]?key["'\''\s:=]+)[A-Za-z0-9+\/=_-]{12,}/$1[REDACTED]/gi' \ | |
| -e 's/((?:access|refresh|tenant|user|app|authorization)[_-]?token["'\''\s:=]+)[A-Za-z0-9._+\/=_-]{16,}/$1[REDACTED]/gi' \ | |
| -e 's/(secret["'\''\s:=]+)[A-Za-z0-9._+\/=_-]{12,}/$1[REDACTED]/gi' \ | |
| -e 's/(password["'\''\s:=]+)[^\s,'\''"}]{6,}/$1[REDACTED]/gi' \ | |
| -e 's/(private[-_ ]?key["'\''\s:=]+)[A-Za-z0-9._+\/=_-]{16,}/$1[REDACTED]/gi' \ | |
| "$file" 2>/dev/null || true | |
| redact_file() { | |
| local file=$1 | |
| perl -0pi \ | |
| -e 's/(Key:\s*)[A-Za-z0-9+\/=_-]{16,}/$1[REDACTED]/gi' \ | |
| -e 's/(api[_ -]?key["'\''\s:=]+)[A-Za-z0-9+\/=_-]{12,}/$1[REDACTED]/gi' \ | |
| -e 's/(authorization["'\''\s:=]+bearer\s+)[A-Za-z0-9._~+\/=-]{16,}/$1[REDACTED]/gi' \ | |
| -e 's/((?:access|refresh|tenant|user|app|authorization)[_-]?token["'\''\s:=]+)[A-Za-z0-9._+\/=_-]{16,}/$1[REDACTED]/gi' \ | |
| -e 's/(secret["'\''\s:=]+)[A-Za-z0-9._+\/=_-]{12,}/$1[REDACTED]/gi' \ | |
| -e 's/(password["'\''\s:=]+)[^\s,'\''"}]{6,}/$1[REDACTED]/gi' \ | |
| -e 's/(private[-_ ]?key["'\''\s:=]+)[A-Za-z0-9._+\/=_-]{16,}/$1[REDACTED]/gi' \ | |
| "$file" 2>/dev/null || true |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/lark-workflow-standup-report/scripts/lark_common.sh` around lines 61 -
70, The redaction logic in redact_file() is missing bearer-style Authorization
headers, so add a replacement pattern for Authorization: Bearer credentials
alongside the existing token and secret masks. Update the perl redaction list in
lark_common.sh so any Authorization header values emitted by lark-cli are
replaced with [REDACTED], while keeping the current handling for api_key,
access/refresh tokens, secrets, passwords, and private keys intact.
Comment on lines
+97
to
+101
| manifest_set_files_array() { | ||
| local manifest=$1 key=$2 | ||
| shift 2 | ||
| jq --arg key "$key" --args '.files[$key]=$ARGS.positional' "$manifest" "$@" >"${manifest}.tmp" | ||
| mv "${manifest}.tmp" "$manifest" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
manifest_set_files_array fails to read the manifest file because jq --args consumes the filename as data.
The --args flag treats all subsequent arguments, including $manifest, as positional arguments available via $ARGS.positional. Consequently, jq receives no input file path and attempts to read from empty standard input, causing the operation to fail.
Suggested fix
manifest_set_files_array() {
local manifest=$1 key=$2
shift 2
- jq --arg key "$key" --args '.files[$key]=$ARGS.positional' "$manifest" "$@" >"${manifest}.tmp"
+ local values_json
+ values_json=$(printf '%s\n' "$@" | jq -R . | jq -s .)
+ jq --arg key "$key" --argjson values "$values_json" '.files[$key]=$values' "$manifest" >"${manifest}.tmp"
mv "${manifest}.tmp" "$manifest"
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| manifest_set_files_array() { | |
| local manifest=$1 key=$2 | |
| shift 2 | |
| jq --arg key "$key" --args '.files[$key]=$ARGS.positional' "$manifest" "$@" >"${manifest}.tmp" | |
| mv "${manifest}.tmp" "$manifest" | |
| manifest_set_files_array() { | |
| local manifest=$1 key=$2 | |
| shift 2 | |
| local values_json | |
| values_json=$(printf '%s\n' "$@" | jq -R . | jq -s .) | |
| jq --arg key "$key" --argjson values "$values_json" '.files[$key]=$values' "$manifest" >"${manifest}.tmp" | |
| mv "${manifest}.tmp" "$manifest" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/lark-workflow-standup-report/scripts/lark_common.sh` around lines 97 -
101, manifest_set_files_array is passing the manifest path after jq --args,
which causes jq to treat the manifest filename as positional data instead of an
input file. Update the jq invocation in manifest_set_files_array so the manifest
is provided as jq input and only the array values are passed as positional
arguments, preserving the .files[$key] assignment and keeping the temp-file mv
flow unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
Summary by CodeRabbit
New Features
Bug Fixes