-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[Core] raw githubusercontent urls are updated to refer azcli blob to restrict external system access #33240
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
[Core] raw githubusercontent urls are updated to refer azcli blob to restrict external system access #33240
Changes from 1 commit
ea5cde1
0c4b3c9
e5df171
1805d77
460df8f
12e5896
c226885
be9568e
6ff79c5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| #!/usr/bin/env python | ||
|
|
||
| # -------------------------------------------------------------------------------------------- | ||
| # Copyright (c) Microsoft Corporation. All rights reserved. | ||
| # Licensed under the MIT License. See License.txt in the project root for license information. | ||
| # -------------------------------------------------------------------------------------------- | ||
|
|
||
| """Fail CI if forbidden raw GitHub aliases URL is introduced in new diff lines.""" | ||
|
|
||
| import argparse | ||
| import re | ||
| import subprocess | ||
| import sys | ||
|
|
||
|
|
||
| FORBIDDEN_URL_PATTERN = re.compile( | ||
| r"https://raw\.githubusercontent\.com/Azure/azure-rest-api-specs/[A-Za-z0-9._/-]+/arm-compute/quickstart-templates/aliases\.json" | ||
|
msarfraz marked this conversation as resolved.
Outdated
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The regex only matches the exact azure-rest-api-specs/.../aliases.json path. If someone introduces a different raw.githubusercontent.com URL for other files, this check won't catch it. The PR's stated goal is to block GitHub URLs in network-isolated environments, so the pattern should be broader —
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The regex is updated to match base URL "https://raw.githubusercontent.com" in the code. The documentation, tests, scripts and recordings directories are excluded from validation to avoid false positives |
||
| ) | ||
| RECOMMENDED_ALIAS_URL = "https://azcliprod.blob.core.windows.net/cli/vm/aliases.json" | ||
|
|
||
|
msarfraz marked this conversation as resolved.
Outdated
|
||
|
|
||
| def _run_diff(src: str, tgt: str, cached: bool = False) -> str: | ||
| cmd = ["git", "diff", "--unified=0", "--no-color"] | ||
| if cached: | ||
| cmd.append("--cached") | ||
| else: | ||
| cmd.append(f"{tgt}...{src}") | ||
|
|
||
| proc = subprocess.run( | ||
| cmd, | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.PIPE, | ||
| text=True, | ||
| check=False, | ||
| ) | ||
| if proc.returncode != 0: | ||
| raise RuntimeError(proc.stderr.strip() or "git diff failed") | ||
| return proc.stdout | ||
|
|
||
|
|
||
| def _find_violations(diff_text: str): | ||
| violations = [] | ||
| current_file = "" | ||
|
|
||
| for line in diff_text.splitlines(): | ||
| if line.startswith("+++ b/"): | ||
| current_file = line[6:] | ||
| continue | ||
|
|
||
| if not line.startswith("+") or line.startswith("+++"): | ||
| continue | ||
|
|
||
| added_line = line[1:] | ||
| if FORBIDDEN_URL_PATTERN.search(added_line): | ||
| violations.append((current_file or "<unknown>", added_line.strip())) | ||
|
|
||
| return violations | ||
|
|
||
|
|
||
| def main() -> int: | ||
| parser = argparse.ArgumentParser(description="Check diff for forbidden raw aliases URL usage.") | ||
| parser.add_argument("--src", default="HEAD", help="Source ref/commit for git diff.") | ||
| parser.add_argument("--tgt", default="HEAD~1", help="Target ref/commit for git diff.") | ||
| parser.add_argument("--cached", action="store_true", help="Check staged changes in git index.") | ||
| args = parser.parse_args() | ||
|
|
||
| try: | ||
| diff_text = _run_diff(src=args.src, tgt=args.tgt, cached=args.cached) | ||
| except Exception as ex: # pylint: disable=broad-except | ||
| if args.cached: | ||
| print(f"Unable to evaluate staged diff: {ex}", file=sys.stderr) | ||
| else: | ||
| print(f"Unable to evaluate diff between '{args.tgt}' and '{args.src}': {ex}", file=sys.stderr) | ||
| return 1 | ||
|
|
||
| violations = _find_violations(diff_text) | ||
| if not violations: | ||
| print("No forbidden aliases source URL found in added lines.") | ||
| return 0 | ||
|
|
||
| print("Found forbidden aliases source URL in this change:", file=sys.stderr) | ||
| for file_path, content in violations: | ||
| print(f" - {file_path}: {content}", file=sys.stderr) | ||
|
|
||
| print( | ||
| f"Use '{RECOMMENDED_ALIAS_URL}' instead of raw GitHub URLs for aliases.json.", | ||
| file=sys.stderr, | ||
| ) | ||
| return 1 | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| sys.exit(main()) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the non-PR path, --tgt=HEAD~1 only diffs against the last commit, which means multi-commit pushes could bypass the check. Consider using the default branch (e.g., origin/dev) as the target instead, so all new changes are covered.