test.py #27
Open
test.py #27
+201
−1
Conversation
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
WalkthroughA badge for CodeRabbit Pull Request Reviews was added to the README. Additionally, a new Python script was introduced to audit HashiCorp Vault AppRole authentication across namespaces, gather TTL data, and generate a formatted Excel report. Several functions were implemented for Vault interaction, data collection, and reporting. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Script
participant Vault
participant ExcelWorkbook
User->>Script: Run script
Script->>Vault: Authenticate using token
Script->>Vault: List namespaces under "admin"
loop For each namespace
Script->>Vault: List AppRole roles
loop For each role
Script->>Vault: Get role metadata
Script->>Vault: List secret ID accessors
loop For each accessor
Script->>Vault: Get accessor TTL
Script->>ExcelWorkbook: Record details if TTL criteria met
end
end
Script->>Vault: List auth methods
Script->>ExcelWorkbook: Record auth methods
end
Script->>ExcelWorkbook: Save workbook
Script->>User: Print progress and errors
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
test.py (1)
151-201: Improve main function structure and error handling.Issues identified:
- Hardcoded "admin" namespace should be configurable
- Multiple saves of the same workbook (inefficient)
- No error handling or cleanup
def main(): + root_namespace = os.getenv("VAULT_ROOT_NAMESPACE", "admin") + output_file = "./auth_secret_id_TTL.xlsx" + if not client.is_authenticated(): print("Vault authentication failed.") return - workbook = openpyxl.Workbook() + try: + workbook = openpyxl.Workbook() # ... header setup code ... - workbook.save(filename="./auth_secret_id_TTL.xlsx") # ... more sheet creation ... - workbook.save(filename="./auth_secret_id_TTL.xlsx") - namespaces = list_namespaces("admin") + namespaces = list_namespaces(root_namespace) # ... processing code ... + # Save once at the end + workbook.save(filename=output_file) + print(f"Report saved to {output_file}") + except Exception as e: + print(f"Error generating report: {e}") + # Clean up partial file if it exists + if os.path.exists(output_file): + os.remove(output_file) + raiseAlso consider refactoring to pass the workbook object to each function instead of repeatedly loading/saving it.
🧹 Nitpick comments (3)
test.py (3)
6-6: Remove duplicate import.
hvac.exceptionsis already available through thehvacimport on line 1.-import hvac.exceptions
117-117: Fix typo in comment.- approle_fill = PatternFill(start_color="FEDCAA", end_color="FEDCAA", fill_type="solid") #Peach color for highling AppRole auth methods + approle_fill = PatternFill(start_color="FEDCAA", end_color="FEDCAA", fill_type="solid") #Peach color for highlighting AppRole auth methods
1-201: Add documentation and consider security implications.
- Add module and function docstrings to explain the purpose and usage
- Consider the security implications of writing sensitive Vault data to an unencrypted Excel file
""" Vault AppRole Audit Script This script audits HashiCorp Vault AppRole configurations across namespaces, identifying roles and secret IDs with TTLs that are zero or exceed one year. Environment Variables: VAULT_ADDR: Vault server address VAULT_TOKEN: Authentication token VAULT_ROOT_NAMESPACE: Starting namespace (default: admin) """For security, consider:
- Encrypting the output file
- Restricting file permissions
- Using a more secure output format
- Implementing audit logging
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(1 hunks)test.py(1 hunks)
🔇 Additional comments (2)
README.md (1)
19-19: Badge addition looks good!The CodeRabbit PR review status badge is properly formatted and well-placed in the README.
test.py (1)
15-31: Fix variable scope issue in exception handlers.The
all_namespacesvariable is not defined in the exception handler scopes, which will cause aNameError.def list_namespaces(namespace=""): client.adapter.namespace = namespace + all_namespaces = [] try: - all_namespaces = [] response = client.sys.list_namespaces() namespaces = response.get("data", {}).get("keys", []) print(namespaces) full_namespaces = [namespace + "/" + ns.strip("/") for ns in namespaces] # Ensure no leading/trailing slashes for ns in full_namespaces: all_namespaces.append(ns) all_namespaces.extend(list_namespaces(ns)) return all_namespaces except hvac.exceptions.InvalidPath: #If invalid, it means we are at the root or no namespaces exist return all_namespaces except hvac.exceptions.Forbidden: return all_namespacesLikely an incorrect or invalid review comment.
Comment on lines
+74
to
+111
| def check_approle_roles(namespace, rownum): | ||
| workbook = openpyxl.load_workbook("./auth_secret_id_TTL.xlsx") | ||
|
|
||
| sheet = workbook.active = workbook["Approle Roles"] | ||
| client.adapter.namespace = namespace #Change to namespace from parameter | ||
| current_row = rownum | ||
| rows = [] | ||
| try: | ||
| roles = client.auth.approle.list_roles()["data"]["keys"] | ||
|
|
||
| for role in roles: #Go through each role in approle and check the secret_id_ttl settings | ||
| try: | ||
| role_id_data = client.auth.approle.read_role(role_name=role)["data"] #Grab role metadata | ||
| ttl = role_id_data["secret_id_ttl"] | ||
| if ttl == 0 or ttl > TTL_THRESHOLD: #If ttl is 0 or greater than threshold, add to spreadsheet | ||
| rows.append([namespace, role, ttl]) | ||
| print(f"[!] Namespace: '{namespace or 'root'}', Role: '{role}',TTL: {ttl} seconds") | ||
| except hvac.exceptions.InvalidPath: #Skip invalid path as it means role can potentially not exist or have metadata | ||
| pass | ||
| except hvac.exceptions.InvalidPath: | ||
| pass | ||
| except Exception as e: | ||
| print(f"Error in namespace '{namespace}': {e}") | ||
|
|
||
| for row_index, row_data in enumerate(rows, current_row): #Input values into sheet | ||
| current_row+=1 | ||
| for col_index, value in enumerate(row_data, start=1): | ||
| sheet.cell(row=row_index, column=col_index, value=value) | ||
|
|
||
| for column_cells in sheet.columns: #Adjust column widths based on content | ||
| new_column_length = max(len(str(cell.value)) for cell in column_cells) | ||
| new_column_letter = (chr(64+(column_cells[0].column))) | ||
| if new_column_length > 0: | ||
| sheet.column_dimensions[new_column_letter].width = new_column_length # *1.10 | ||
|
|
||
| workbook.save(filename="./auth_secret_id_TTL.xlsx") | ||
| return current_row #Return the next row number to write to for the next function call | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract common functionality to reduce duplication.
This function has identical workbook handling and column width adjustment code as check_approle_secret_id.
Consider extracting common functionality:
def adjust_column_widths(sheet):
"""Adjust column widths based on content."""
from openpyxl.utils import get_column_letter
for column_cells in sheet.columns:
new_column_length = max(len(str(cell.value)) for cell in column_cells)
column_letter = get_column_letter(column_cells[0].column)
if new_column_length > 0:
sheet.column_dimensions[column_letter].width = new_column_length * 1.1
def write_rows_to_sheet(sheet, rows, start_row):
"""Write rows to sheet starting at start_row."""
for i, row_data in enumerate(rows):
row_index = start_row + i
for col_index, value in enumerate(row_data, start=1):
sheet.cell(row=row_index, column=col_index, value=value)
return start_row + len(rows)🤖 Prompt for AI Agents
In test.py from lines 74 to 111, the function check_approle_roles duplicates
workbook handling and column width adjustment code found in
check_approle_secret_id. To fix this, extract the common logic into separate
helper functions such as adjust_column_widths(sheet) to handle column resizing
and write_rows_to_sheet(sheet, rows, start_row) to write data rows starting at a
given row. Replace the duplicated code in check_approle_roles with calls to
these helper functions to reduce redundancy and improve maintainability.
Comment on lines
+8
to
+13
| VAULT_ADDR = "https://hcp-vault-private-vault-fc507e0d.5d5b1f21.z1.hashicorp.cloud:8200" | ||
| VAULT_TOKEN = os.getenv("VAULT_TOKEN") | ||
| TTL_THRESHOLD = 31536000 # 1 year in seconds | ||
|
|
||
| client = hvac.Client(url=VAULT_ADDR, token=VAULT_TOKEN) | ||
| print(client.is_authenticated()) |
There was a problem hiding this comment.
Critical: Hardcoded Vault URL and missing error handling.
- The Vault URL should not be hardcoded. Use environment variables or configuration files.
- Add validation for the VAULT_TOKEN environment variable.
- Add error handling for client initialization.
-VAULT_ADDR = "https://hcp-vault-private-vault-fc507e0d.5d5b1f21.z1.hashicorp.cloud:8200"
-VAULT_TOKEN = os.getenv("VAULT_TOKEN")
+VAULT_ADDR = os.getenv("VAULT_ADDR")
+VAULT_TOKEN = os.getenv("VAULT_TOKEN")
+
+if not VAULT_ADDR:
+ raise ValueError("VAULT_ADDR environment variable is required")
+if not VAULT_TOKEN:
+ raise ValueError("VAULT_TOKEN environment variable is required")
+
TTL_THRESHOLD = 31536000 # 1 year in seconds
-client = hvac.Client(url=VAULT_ADDR, token=VAULT_TOKEN)
-print(client.is_authenticated())
+try:
+ client = hvac.Client(url=VAULT_ADDR, token=VAULT_TOKEN)
+ if not client.is_authenticated():
+ raise ValueError("Failed to authenticate with Vault")
+except Exception as e:
+ print(f"Failed to initialize Vault client: {e}")
+ raise📝 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
| VAULT_ADDR = "https://hcp-vault-private-vault-fc507e0d.5d5b1f21.z1.hashicorp.cloud:8200" | |
| VAULT_TOKEN = os.getenv("VAULT_TOKEN") | |
| TTL_THRESHOLD = 31536000 # 1 year in seconds | |
| client = hvac.Client(url=VAULT_ADDR, token=VAULT_TOKEN) | |
| print(client.is_authenticated()) | |
| VAULT_ADDR = os.getenv("VAULT_ADDR") | |
| VAULT_TOKEN = os.getenv("VAULT_TOKEN") | |
| if not VAULT_ADDR: | |
| raise ValueError("VAULT_ADDR environment variable is required") | |
| if not VAULT_TOKEN: | |
| raise ValueError("VAULT_TOKEN environment variable is required") | |
| TTL_THRESHOLD = 31536000 # 1 year in seconds | |
| try: | |
| client = hvac.Client(url=VAULT_ADDR, token=VAULT_TOKEN) | |
| if not client.is_authenticated(): | |
| raise ValueError("Failed to authenticate with Vault") | |
| except Exception as e: | |
| print(f"Failed to initialize Vault client: {e}") | |
| raise |
🤖 Prompt for AI Agents
In test.py around lines 8 to 13, the Vault URL is hardcoded, the VAULT_TOKEN
environment variable is not validated, and there is no error handling for client
initialization. Replace the hardcoded Vault URL with a value read from an
environment variable or configuration file. Add a check to ensure VAULT_TOKEN is
set and raise an appropriate error or exit if it is missing. Wrap the
hvac.Client initialization in a try-except block to catch and handle any
exceptions, logging or printing meaningful error messages.
Comment on lines
+32
to
+73
| def check_approle_secret_id(namespace, rownum): | ||
| workbook = openpyxl.load_workbook("./auth_secret_id_TTL.xlsx") | ||
|
|
||
| sheet = workbook.active = workbook["Approle Secret ID"] #Change to Approle Secret ID sheet | ||
| client.adapter.namespace = namespace #Change to namespace from parameter | ||
| current_row = rownum | ||
| rows = [] | ||
| try: | ||
| roles = client.auth.approle.list_roles()["data"]["keys"] | ||
| print(roles) | ||
|
|
||
| for role in roles: #Iterate through each role and if secret_id_ttl is greater than threshold | ||
| print(role) | ||
| try: | ||
| secret_id_accessors = client.auth.approle.list_secret_id_accessors(role_name=role)["data"]["keys"] | ||
| for accessor in secret_id_accessors: | ||
| secret_id_info = client.auth.approle.read_secret_id_accessor(role_name=role, secret_id_accessor=accessor) | ||
| ttl = secret_id_info["data"]["secret_id_ttl"] | ||
| if ttl == 0 or ttl > TTL_THRESHOLD: | ||
| rows.append([namespace, role, accessor, ttl]) | ||
| print(f"[!] Namespace: '{namespace or 'root'}', Role: '{role}', Secret ID Accessor: {accessor}, TTL: {ttl} seconds") | ||
| except hvac.exceptions.InvalidPath: | ||
| pass | ||
| except hvac.exceptions.InvalidPath: | ||
| pass | ||
| except Exception as e: | ||
| print(f"Error in namespace '{namespace}': {e}") | ||
|
|
||
| for row_index, row_data in enumerate(rows, current_row): #Input values into sheet | ||
| current_row+=1 | ||
| for col_index, value in enumerate(row_data, start=1): | ||
| sheet.cell(row=row_index, column=col_index, value=value) | ||
|
|
||
| for column_cells in sheet.columns: | ||
| new_column_length = max(len(str(cell.value)) for cell in column_cells) | ||
| new_column_letter = (chr(64+(column_cells[0].column))) | ||
| if new_column_length > 0: | ||
| sheet.column_dimensions[new_column_letter].width = new_column_length # *1.10 | ||
|
|
||
| workbook.save(filename="./auth_secret_id_TTL.xlsx") | ||
| return current_row | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor to improve efficiency and fix column handling.
Multiple issues identified:
- Workbook is loaded and saved for each namespace (inefficient)
- Column width calculation only works for columns A-Z
- The
current_rowincrement affects enumerate behavior
Consider passing the workbook as a parameter instead of loading/saving repeatedly. For the column width issue:
- for column_cells in sheet.columns:
- new_column_length = max(len(str(cell.value)) for cell in column_cells)
- new_column_letter = (chr(64+(column_cells[0].column)))
- if new_column_length > 0:
- sheet.column_dimensions[new_column_letter].width = new_column_length # *1.10
+ from openpyxl.utils import get_column_letter
+ for column_cells in sheet.columns:
+ new_column_length = max(len(str(cell.value)) for cell in column_cells)
+ column_letter = get_column_letter(column_cells[0].column)
+ if new_column_length > 0:
+ sheet.column_dimensions[column_letter].width = new_column_length * 1.1For the row tracking:
- for row_index, row_data in enumerate(rows, current_row): #Input values into sheet
- current_row+=1
+ for i, row_data in enumerate(rows):
+ row_index = current_row + i
for col_index, value in enumerate(row_data, start=1):
sheet.cell(row=row_index, column=col_index, value=value)
+ current_row += len(rows)📝 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
| def check_approle_secret_id(namespace, rownum): | |
| workbook = openpyxl.load_workbook("./auth_secret_id_TTL.xlsx") | |
| sheet = workbook.active = workbook["Approle Secret ID"] #Change to Approle Secret ID sheet | |
| client.adapter.namespace = namespace #Change to namespace from parameter | |
| current_row = rownum | |
| rows = [] | |
| try: | |
| roles = client.auth.approle.list_roles()["data"]["keys"] | |
| print(roles) | |
| for role in roles: #Iterate through each role and if secret_id_ttl is greater than threshold | |
| print(role) | |
| try: | |
| secret_id_accessors = client.auth.approle.list_secret_id_accessors(role_name=role)["data"]["keys"] | |
| for accessor in secret_id_accessors: | |
| secret_id_info = client.auth.approle.read_secret_id_accessor(role_name=role, secret_id_accessor=accessor) | |
| ttl = secret_id_info["data"]["secret_id_ttl"] | |
| if ttl == 0 or ttl > TTL_THRESHOLD: | |
| rows.append([namespace, role, accessor, ttl]) | |
| print(f"[!] Namespace: '{namespace or 'root'}', Role: '{role}', Secret ID Accessor: {accessor}, TTL: {ttl} seconds") | |
| except hvac.exceptions.InvalidPath: | |
| pass | |
| except hvac.exceptions.InvalidPath: | |
| pass | |
| except Exception as e: | |
| print(f"Error in namespace '{namespace}': {e}") | |
| for row_index, row_data in enumerate(rows, current_row): #Input values into sheet | |
| current_row+=1 | |
| for col_index, value in enumerate(row_data, start=1): | |
| sheet.cell(row=row_index, column=col_index, value=value) | |
| for column_cells in sheet.columns: | |
| new_column_length = max(len(str(cell.value)) for cell in column_cells) | |
| new_column_letter = (chr(64+(column_cells[0].column))) | |
| if new_column_length > 0: | |
| sheet.column_dimensions[new_column_letter].width = new_column_length # *1.10 | |
| workbook.save(filename="./auth_secret_id_TTL.xlsx") | |
| return current_row | |
| def check_approle_secret_id(namespace, rownum): | |
| workbook = openpyxl.load_workbook("./auth_secret_id_TTL.xlsx") | |
| sheet = workbook.active = workbook["Approle Secret ID"] #Change to Approle Secret ID sheet | |
| client.adapter.namespace = namespace #Change to namespace from parameter | |
| current_row = rownum | |
| rows = [] | |
| try: | |
| roles = client.auth.approle.list_roles()["data"]["keys"] | |
| print(roles) | |
| for role in roles: #Iterate through each role and if secret_id_ttl is greater than threshold | |
| print(role) | |
| try: | |
| secret_id_accessors = client.auth.approle.list_secret_id_accessors(role_name=role)["data"]["keys"] | |
| for accessor in secret_id_accessors: | |
| secret_id_info = client.auth.approle.read_secret_id_accessor(role_name=role, secret_id_accessor=accessor) | |
| ttl = secret_id_info["data"]["secret_id_ttl"] | |
| if ttl == 0 or ttl > TTL_THRESHOLD: | |
| rows.append([namespace, role, accessor, ttl]) | |
| print(f"[!] Namespace: '{namespace or 'root'}', Role: '{role}', Secret ID Accessor: {accessor}, TTL: {ttl} seconds") | |
| except hvac.exceptions.InvalidPath: | |
| pass | |
| except hvac.exceptions.InvalidPath: | |
| pass | |
| except Exception as e: | |
| print(f"Error in namespace '{namespace}': {e}") | |
| # Correct row tracking | |
| for i, row_data in enumerate(rows): | |
| row_index = current_row + i | |
| for col_index, value in enumerate(row_data, start=1): | |
| sheet.cell(row=row_index, column=col_index, value=value) | |
| current_row += len(rows) | |
| # Fix column width for A–ZZZ… using openpyxl utility | |
| from openpyxl.utils import get_column_letter | |
| for column_cells in sheet.columns: | |
| new_column_length = max(len(str(cell.value)) for cell in column_cells) | |
| column_letter = get_column_letter(column_cells[0].column) | |
| if new_column_length > 0: | |
| sheet.column_dimensions[column_letter].width = new_column_length * 1.1 | |
| workbook.save(filename="./auth_secret_id_TTL.xlsx") | |
| return current_row |
🤖 Prompt for AI Agents
In test.py from lines 32 to 73, improve efficiency by loading the workbook once
outside the function and passing it as a parameter to avoid repeated loading and
saving for each namespace. Fix the column width calculation to handle columns
beyond 'Z' by converting the column index to the correct Excel column letter(s)
instead of assuming single letters. Also, adjust the row tracking by removing
the manual increment of current_row inside the loop since enumerate already
handles row indexing; use the row_index directly for writing cells and update
current_row after processing all rows.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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 by CodeRabbit