fix(policy): clarify Jira curl validation after reopen#5172
Conversation
Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR clarifies Jira preset validation guidance: it updates the warning message and its test to state that empty ChangesJira Policy Validation Guidance
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-pr-5172.docs.buildwithfern.com/nemoclaw |
E2E Advisor RecommendationRequired E2E: None Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: None Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/network-policy/integration-policy-examples.mdx (1)
2-3:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the required SPDX header format for MDX.
For
.mdx, the SPDX header must use Markdown/HTML comment form per repo rule; the current#lines in frontmatter do not match the required format.As per coding guidelines,
*.md/*.mdxfiles must carry SPDX headers and Markdown-family files should use HTML comments for that header.🤖 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 `@docs/network-policy/integration-policy-examples.mdx` around lines 2 - 3, The SPDX frontmatter in integration-policy-examples.mdx uses '#' lines; update it to the required Markdown/HTML comment form by replacing the two header lines (the SPDX-FileCopyrightText and SPDX-License-Identifier) with an HTML comment block such as <!-- SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. --> and <!-- SPDX-License-Identifier: Apache-2.0 --> so the SPDX-... symbols remain identical but are wrapped in HTML comments for MDX.Source: Coding guidelines
.agents/skills/nemoclaw-user-overview/references/release-notes.md (1)
1-2:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd required SPDX header for this Markdown file.
This release-notes
.mdfile is missing the mandatory SPDX copyright and license header.As per coding guidelines, all
*.md/*.mdxsources must include SPDX headers using HTML comments for Markdown files.🤖 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 @.agents/skills/nemoclaw-user-overview/references/release-notes.md around lines 1 - 2, The release-notes.md file is missing the required SPDX header; add an HTML comment SPDX header at the very top of the file (for example an HTML comment containing SPDX-FileCopyrightText and SPDX-License-Identifier) so this Markdown follows the project's *.md/*.mdx SPDX header requirement; update the top of release-notes.md to include the correct SPDX header lines.Source: Coding guidelines
.agents/skills/nemoclaw-user-manage-policy/references/integration-policy-examples.md (1)
1-2:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd required SPDX header for Markdown files.
This
.mdfile is missing the required SPDX copyright and license header in Markdown comment form.As per coding guidelines, all
*.md/*.mdxfiles must include SPDX headers, using HTML comments for Markdown files.🤖 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 @.agents/skills/nemoclaw-user-manage-policy/references/integration-policy-examples.md around lines 1 - 2, This Markdown file lacks the required SPDX header; add an HTML comment SPDX header at the very top of the file (above the existing "# Common NemoClaw Integration Policy Examples" heading) that includes the SPDX-FileCopyrightText line with the appropriate year/owner and the SPDX-License-Identifier line with the proper license identifier (e.g., Apache-2.0), ensuring the header is in HTML comment form so it remains a Markdown comment.Source: Coding guidelines
🧹 Nitpick comments (1)
docs/about/release-notes.mdx (1)
156-156: Consider breaking this into one sentence per line.This bullet point contains three sentences on the same line. The docs style guide requires one sentence per line to make diffs more readable. However, this compact style is used consistently throughout the release notes file.
Suggested format
-- Jira policy validation guidance now matches the maintained preset. Use a Node HTTPS status probe for Atlassian API access and the body-visible `api.atlassian.com/oauth/token/accessible-resources` curl probe when validating approved requests manually. Plain `curl -s` against `auth.atlassian.com` can return empty output even when reachable, so it is not a pass/fail signal. +- Jira policy validation guidance now matches the maintained preset. + Use a Node HTTPS status probe for Atlassian API access and the body-visible `api.atlassian.com/oauth/token/accessible-resources` curl probe when validating approved requests manually. + Plain `curl -s` against `auth.atlassian.com` can return empty output even when reachable, so it is not a pass/fail signal.🤖 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 `@docs/about/release-notes.mdx` at line 156, The release-notes bullet currently contains three sentences on a single line; split that bullet into one sentence per line so each sentence in the Jira policy validation guidance becomes its own line. Locate the bullet text starting with "Jira policy validation guidance now matches the maintained preset." and break it into three separate lines: one for the statement about matching the preset, one for using a Node HTTPS status probe and the body-visible curl probe for Atlassian API access, and one noting that plain `curl -s` against `auth.atlassian.com` can return empty output and is not a pass/fail signal.Source: Coding guidelines
🤖 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.
Outside diff comments:
In
@.agents/skills/nemoclaw-user-manage-policy/references/integration-policy-examples.md:
- Around line 1-2: This Markdown file lacks the required SPDX header; add an
HTML comment SPDX header at the very top of the file (above the existing "#
Common NemoClaw Integration Policy Examples" heading) that includes the
SPDX-FileCopyrightText line with the appropriate year/owner and the
SPDX-License-Identifier line with the proper license identifier (e.g.,
Apache-2.0), ensuring the header is in HTML comment form so it remains a
Markdown comment.
In @.agents/skills/nemoclaw-user-overview/references/release-notes.md:
- Around line 1-2: The release-notes.md file is missing the required SPDX
header; add an HTML comment SPDX header at the very top of the file (for example
an HTML comment containing SPDX-FileCopyrightText and SPDX-License-Identifier)
so this Markdown follows the project's *.md/*.mdx SPDX header requirement;
update the top of release-notes.md to include the correct SPDX header lines.
In `@docs/network-policy/integration-policy-examples.mdx`:
- Around line 2-3: The SPDX frontmatter in integration-policy-examples.mdx uses
'#' lines; update it to the required Markdown/HTML comment form by replacing the
two header lines (the SPDX-FileCopyrightText and SPDX-License-Identifier) with
an HTML comment block such as <!-- SPDX-FileCopyrightText: Copyright (c) 2026
NVIDIA CORPORATION & AFFILIATES. All rights reserved. --> and <!--
SPDX-License-Identifier: Apache-2.0 --> so the SPDX-... symbols remain identical
but are wrapped in HTML comments for MDX.
---
Nitpick comments:
In `@docs/about/release-notes.mdx`:
- Line 156: The release-notes bullet currently contains three sentences on a
single line; split that bullet into one sentence per line so each sentence in
the Jira policy validation guidance becomes its own line. Locate the bullet text
starting with "Jira policy validation guidance now matches the maintained
preset." and break it into three separate lines: one for the statement about
matching the preset, one for using a Node HTTPS status probe and the
body-visible curl probe for Atlassian API access, and one noting that plain
`curl -s` against `auth.atlassian.com` can return empty output and is not a
pass/fail signal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f128f4ab-03e9-4961-8e9e-6422ef8fd748
📒 Files selected for processing (6)
.agents/skills/nemoclaw-user-manage-policy/references/integration-policy-examples.md.agents/skills/nemoclaw-user-overview/references/release-notes.mddocs/about/release-notes.mdxdocs/network-policy/integration-policy-examples.mdxsrc/lib/policy/index.tstest/policies.test.ts
PR Review AdvisorFindings: 1 needs attention, 3 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Consider writing more tests for
Since last review detailsCurrent findings:
This is an automated advisory review. A human maintainer must make the final merge decision. |
prekshivyas
left a comment
There was a problem hiding this comment.
Reviewed (code + 9-cat security). Adds one clarifying sentence to the Jira getPresetValidationWarning (mirrored to docs + generated skills + release-notes) plus a test-assertion update. No enforcement logic changes.
✅ Approve — purely additive guidance, does not weaken policy enforcement. Verified: the jira preset allowlists only node (no curl), and test-network-policy.sh TC-NET-08 proves curl is blocked pre-approval (CURL_APPCONNECT_0) and works only after an explicit policy update --binary curl — so #3758's "presets permit any binary" is refuted; the reopen is a QA-observability artifact, not an enforcement gap. The dropped toContain("curl -s") assertion is safe (string still contains it twice). Security: all 9 pass.
Note (pre-existing, not this PR): the legacy skills/ tree lags the maintained .agents/skills/ repo-wide.
|
@chengjiew the only red check here is Use the same name/email as your commits. Once it's in the PR body, |
|
✨ Related open issues: |
Summary
curl -s https://auth.atlassian.comempty output is inconclusive before or after approval.api.atlassian.com/oauth/token/accessible-resources.Root Cause
This does not change Jira policy enforcement. #4579 already made the runtime validation observable and covered TC-NET-08 with network-policy E2E. The reopen happened because QA re-tested v0.0.56 with the older
auth.atlassian.comcurl -sprobe; that endpoint can return an empty redirect body even when reachable, so the old probe makes blocked and allowed cases look the same.Tests
npm run build:clinpx vitest run test/policies.test.tsnpm run docs:check-agent-variantsnpm run test-size:checkpython3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix nemoclaw-user --doc-platform fern-mdx --dry-rungit diff --checkLocal hook note
git commit/git pushhooks were attempted, but the full local CLI coverage hook timed out across unrelated CLI suites on this machine. The focused validation above passed, and this PR is docs/CLI-warning scoped.Fixes #3758
Signed-off-by: Chengjie Wang chengjiew@nvidia.com
Summary by CodeRabbit