feat(isms-change-management): add ISMS Change Management compliance GitHub Action - #59
feat(isms-change-management): add ISMS Change Management compliance GitHub Action#59yanjost wants to merge 3 commits into
Conversation
…ction
Implements a new GitHub Action that validates changes to ISMS documents
against the ISMS Change Management Policy (ISMS_CMPOL).
Checks performed on each changed ISMS markdown file:
1. Author ≠ Validator: no person may appear in both authors and validators
2. Version bump: version must be strictly greater than on the base branch
3. Double validation: minor or major bumps require at least 2 validators
4. RSSI role (optional): RSSI must validate, unless RSSI is the author
in which case CTO or CEO must validate instead
Written in Ruby (allowed scripting language at Scalingo).
Includes 29 unit tests covering all validation rules and edge cases.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…herence Add two new checks to the ISMS change management action: Check 5 - Commit authors coherence (no API required): Every git commit author touching an ISMS document must be listed in the document's authors field. Prevents undeclared authorship. Check 6 - PR reviewer coherence (optional, requires github-token + pr-number): YAML validators must exactly match the set of actual GitHub PR approvers (bidirectional). Prevents phantom validators and undocumented approvers. Also adds normalize_name/name_in_list? helpers for case-insensitive name matching across all checks.
![semgrep-code-scalingo[bot]](https://avatars.githubusercontent.com/u/4868969?s=60&v=4){kind=small}
| end | ||
|
|
||
| def read_base_content(file, base_ref) | ||
| `git show "#{base_ref}:#{file}" 2>/dev/null` |
There was a problem hiding this comment.
Detected non-static command inside .... If unverified user data can reach this call site, this is a code injection vulnerability. A malicious actor can inject a malicious script to execute arbitrary code.
🧁 Fixed in commit d7e19ae 🧁
| end | ||
|
|
||
| def changed_files(base_ref) | ||
| `git diff --name-only "#{base_ref}" HEAD 2>/dev/null`.split("\n").map(&:strip).reject(&:empty?) |
There was a problem hiding this comment.
Detected non-static command inside .... If unverified user data can reach this call site, this is a code injection vulnerability. A malicious actor can inject a malicious script to execute arbitrary code.
🧹 Fixed in commit d7e19ae 🧹
| end | ||
|
|
||
| def commit_authors_for_file(file, base_ref) | ||
| `git log "#{base_ref}..HEAD" --format="%an" -- "#{file}" 2>/dev/null` |
There was a problem hiding this comment.
Detected non-static command inside .... If unverified user data can reach this call site, this is a code injection vulnerability. A malicious actor can inject a malicious script to execute arbitrary code.
⭐ Fixed in commit d7e19ae ⭐
| end | ||
|
|
||
| def file_exists_in_base?(file, base_ref) | ||
| system("git cat-file -e \"#{base_ref}:#{file}\" 2>/dev/null") |
There was a problem hiding this comment.
Detected non-static command inside system. Audit the input to 'system'. If unverified user data can reach this call site, this is a code injection vulnerability. A malicious actor can inject a malicious script to execute arbitrary code.
🧹 Fixed in commit d7e19ae 🧹
…n policy, post PR comment Add three new capabilities: Check 7 — Version matches content change: Classifies each changed ISMS document body as patch/minor/major according to the security policy (>50% lines changed → major, heading added/removed → minor, cosmetic only → patch) and fails when the declared version bump is lower than the content warrants. PR comment: When github-token + pr-number are provided, the action posts (or updates in place) a markdown comment on the PR summarising every check result per file with ✅/❌ emojis and an overall verdict. Security fixes (semgrep): Replace shell-interpolated backtick/system calls with array-form IO.popen and system to eliminate shell injection risk. Two remaining findings (git revision-range and object-specifier arguments that must be a single token) are annotated with nosemgrep and an explanation. Also add Python __pycache__ and *.pyc to .gitignore. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
f97f476 to
4c6b502
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a new “ISMS Change Management Compliance” composite GitHub Action implemented in Ruby, with documentation, CI wiring, and a comprehensive Minitest suite to validate changes to ISMS Markdown documents (front matter + content) against the policy.
Changes:
- Introduces
isms-change-managementcomposite action + Ruby validator script with multiple checks (authors/validators, versioning, roles, PR approvers, content-change classification). - Adds Minitest test suite and a test runner script; wires tests into CI.
- Updates action documentation and repo metadata to reflect the new Ruby-based action.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
isms-change-management/scripts/check_isms_change.rb |
Core implementation of the ISMS change validation and optional PR comment posting. |
isms-change-management/tests/test_check_isms_change.rb |
Unit tests covering the script helpers and checks. |
isms-change-management/tests/run.sh |
Convenience script to run the Ruby test suite. |
isms-change-management/action.yml |
Composite action definition exposing inputs and running the Ruby script. |
isms-change-management/README.md |
Full documentation of checks, inputs, and example PR comment output. |
.github/workflows/ci.yml |
Adds a CI job to run the action’s Ruby tests. |
.sclng/metadata.toml |
Updates declared languages to include Ruby. |
.gitignore |
Adds Python cache ignores (unrelated to the Ruby action). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def check_author_validator_coherence(file, front_matter) | ||
| authors = Array(front_matter["authors"]).map(&:to_s) | ||
| validators = Array(front_matter["validators"]).map(&:to_s) | ||
|
|
||
| overlap = authors & validators | ||
| return [] if overlap.empty? |
| def check_rssi_role(file, front_matter, rssi_names, cto_names, ceo_names) | ||
| return [] if rssi_names.empty? | ||
|
|
||
| authors = Array(front_matter["authors"]).map(&:to_s) | ||
| validators = Array(front_matter["validators"]).map(&:to_s) | ||
|
|
||
| rssi_is_author = (rssi_names & authors).any? | ||
| rssi_is_validator = (rssi_names & validators).any? |
| def bump_kind(old_ver, new_ver) | ||
| parse = lambda do |v| | ||
| m = v.to_s.match(/\A(\d+)\.(\d+)\.(\d+)/) | ||
| m ? [m[1].to_i, m[2].to_i, m[3].to_i] : [0, 0, 0] | ||
| end | ||
|
|
||
| old = parse.call(old_ver) | ||
| new_v = parse.call(new_ver) | ||
|
|
||
| return :major if new_v[0] > old[0] | ||
| return :minor if new_v[1] > old[1] | ||
| return :patch if new_v[2] > old[2] | ||
|
|
||
| :none | ||
| end |
| github_token = ENV.fetch("GITHUB_TOKEN", "").strip | ||
| pr_number = ENV.fetch("PR_NUMBER", "").strip | ||
| repository = ENV.fetch("GITHUB_REPOSITORY", "").strip | ||
|
|
||
| all_changed = changed_files(base_ref) | ||
| isms_files = all_changed.select { |f| f.end_with?(".md") && matches_pattern?(f, files_pattern) } |
|
|
||
| puts "Checking #{isms_files.size} changed ISMS document(s) against base #{base_ref}..." | ||
|
|
||
| can_post_comment = !github_token.empty? && !pr_number.empty? && !repository.empty? |
| def check_double_validation(file, old_front_matter, new_front_matter) | ||
| old_version = old_front_matter["version"].to_s | ||
| new_version = new_front_matter["version"].to_s | ||
| validators = Array(new_front_matter["validators"]).map(&:to_s).reject(&:empty?) | ||
|
|
||
| kind = bump_kind(old_version, new_version) | ||
| return [] unless %i[minor major].include?(kind) |
Dismissing approval — known bugs identified in the ISMS Change Management action; re-testing against real specifications PRs before re-requesting review.
Testing against real specifications PRs — 3 issues foundI ran 1. (Critical) Front matter parsing silently fails on ~92% of real ISMS docs → false "all checks passed"
2. (Critical) Wrong default base branch assumption
3. (Medium) Changed-files diff uses two-dot instead of merge-base semantics
Marking as draft until these are addressed — the 69 unit tests pass, but they don't exercise real front matter (with dates) or the real base-branch/diff scenario, which is how all three issues slipped through. |
|
@yanjost is this PR still under your radar? |
| base-ref: main | ||
| rssi: "Yannick Jost" | ||
| cto: "Léo Unbekandt" | ||
| ceo: "Frédéric Harper" |
|
@EtienneM I think yes, but it was postponed due to other topics |
Summary
Implements a new composite GitHub Action
isms-change-managementthat validates changes to ISMS documents (markdown files with YAML front matter) against the ISMS Change Management Policy.Written in Ruby (allowed scripting language at Scalingo).
Checks performed
For each changed ISMS markdown file in a pull request, the action verifies:
authorsandvalidatorsversionmust be strictly greater than on the base branch (semver)rssiinput is setauthorsvalidatorsmust exactly match the set of actual GitHub PR approvers (bidirectional)github-token+pr-numberare setContent change classification (Check 7)
#…) was added or removedA higher bump than required is always accepted; only bumping lower than the content warrants is an error.
PR comment
When
github-tokenandpr-numberare provided the action posts (or updates in place) a markdown comment on the PR with a ✅/❌ summary per check per file and an overall verdict.Files added
Files updated
.github/workflows/ci.yml— CI job running the Ruby test suite.sclng/metadata.toml— addedRubyto languages list.gitignore— ignore Python__pycache__directoriesTests
69 unit tests covering all checks and edge cases.
Example usage
Fix ICY-848