7.2 ansieng 4330 #2159
7.2 ansieng 4330 #2159
Conversation
| url_password: "{{kafka_broker_rest_health_check_password}}" | ||
| force_basic_auth: true | ||
| register: erp_result | ||
| no_log: "{{mask_secrets|bool}}" |
There was a problem hiding this comment.
probably not needed here and maybe needed in the above mds health check
| } | ||
| ] | ||
| status_code: 204 | ||
| no_log: "{{mask_secrets|bool}}" |
There was a problem hiding this comment.
This post request respond with mds toker, hence masking it.
There was a problem hiding this comment.
lets also add it for all other occurances of register cluster
|
lets add ci checks for 3 things in all prs. We can check the git diff and do the following -
This will help us warn incase we might leak any secret accidentally. Although not full proof but it should cover us in significant percentage of cases. |
|
|
|
|
||
| if not all_files: | ||
| print("No YAML files found to check.") | ||
| return 0 |
There was a problem hiding this comment.
we will always find yaml files. so if we dont then i think we should be returning error
| # Parse YAML content | ||
| try: | ||
| parsed_content = yaml.safe_load(content) | ||
| except yaml.YAMLError: |
There was a problem hiding this comment.
should this give different output then the case where we have no errors ?
| return False | ||
|
|
||
| # Check for direct authentication parameters | ||
| auth_params = ['Authorization'] |
There was a problem hiding this comment.
i dont think uri module has any argument called authorization. so this will never return true
| return False | ||
|
|
||
|
|
||
| def has_no_log_protection(task): |
There was a problem hiding this comment.
the name is a bit confusing. has_no_log_protection may mean it has no protection in logs or may mean it has no_log protecting logs 😅
|
|
||
| def get_task_line_number(content, task_name, task_index): | ||
| """Get approximate line number for a task in the file content.""" | ||
| if not task_name: |
There was a problem hiding this comment.
i didnt understand the logic here
| return task_index + 1 | ||
|
|
||
| lines = content.split('\n') | ||
| for i, line in enumerate(lines): |
There was a problem hiding this comment.
this may fail in case we have 2 tasks of same name in same file. But shouldn't be a big deal. Name should be sufficient to find in such cases.
| print(f"Comparing {current_branch} against base branch: {base_branch}") | ||
|
|
||
| # Build command list based on available base branch | ||
| if base_branch: |
There was a problem hiding this comment.
what is the reason for doing this if base_branch and the list of commands in both if and else blocks ?
Description
mask secrets
here, any leaked fact which might be a security concern is seen as a secret.
to find secrets which were leaked, went through all the logs manually with verbosity 4.
hiding all fields of a task for curl command as:
-> The uri module does not mark headers or Authorization as no_log fields internally.
-> Even if you mark your token secret as no_log: true in set_fact, it still leaks in -vvvv logs.
-> The VALUE_SPECIFIED_IN_NO_LOG_PARAMETER won’t be triggered unless the module itself marks that parameter as sensitive.
tomorrow if someone writes a new task, to ensure that we don't leak secrets, check below type of task:
-> if the task is a uri POST/PUT method
-> if the task is to set_fact, which sets any field which can be asecurity concern.
Fixes # (issue)
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist: