Skip to content

feat(flex-linux-setup): location of policy store file #2442

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.

Sign up for GitHub

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

Merged
duttarnab merged 5 commits into from
Nov 12, 2025
Merged

feat(flex-linux-setup): location of policy store file #2442

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9c3b8ee
feat(flex-linux-setup): location of policy store file
devrimyatar Nov 11, 2025
b97c38a
fix(flex-linux-setup): check existence of policy file as suggested by…
devrimyatar Nov 11, 2025
b3e68a2
fix(flex-linux-setup): validate json format of policy store file as s…
devrimyatar Nov 11, 2025
03230a3
fix(flex-linux-setup): replace hostname
devrimyatar Nov 12, 2025
50ccce5
fix(flex-linux-setup): typo
devrimyatar Nov 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion flex-linux-setup/flex_linux_setup/flex_setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,9 @@ def download_files(self, force=False):
'https://raw.githubusercontent.com/JanssenProject/jans/{}/jans-config-api/plugins/admin-ui-plugin/config/log4j2-adminui.xml'.format(
app_versions['JANS_BRANCH']), self.log4j2_adminui_path),
(self.adimin_ui_bin_url, os.path.join(Config.dist_jans_dir, os.path.basename(self.adimin_ui_bin_url))),
('https://raw.githubusercontent.com/GluuFederation/GluuFlexAdminUIPolicyStore/refs/heads/main/2fb50e468d9dfefa142d1fce4fa9747efbd3a0f08de5.json',
self.policy_store_path
),
Comment on lines +372 to +374
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Nov 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add integrity verification for the security-critical policy store file.

The policy store file controls authorization policies for Admin UI and is downloaded from an external source without any checksum or signature verification. This poses a security risk if the source is compromised or subject to man-in-the-middle attacks.

Consider implementing one of the following:

  1. Verify a checksum after download (preferred for immutable releases)
  2. Verify a signature if the repository provides signed artifacts
  3. Pin to a specific commit hash in the URL to ensure the exact content is fetched

Example implementation for checksum verification:

# After line 382, add verification for policy store
expected_sha256 = "your_expected_checksum_here"  # Define this
actual_sha256 = base.get_file_hash(self.policy_store_path, 'sha256')
if actual_sha256 != expected_sha256:
    raise Exception(f"Policy store integrity check failed. Expected {expected_sha256}, got {actual_sha256}")

Alternatively, modify the URL to pin to a specific commit:

-('https://raw.githubusercontent.com/GluuFederation/GluuFlexAdminUIPolicyStore/refs/heads/main/2fb50e468d9dfefa142d1fce4fa9747efbd3a0f08de5.json',
+('https://raw.githubusercontent.com/GluuFederation/GluuFlexAdminUIPolicyStore/<COMMIT_HASH>/2fb50e468d9dfefa142d1fce4fa9747efbd3a0f08de5.json',

Based on learnings (audit context should include policy store URL).

🤖 Prompt for AI Agents 
In flex-linux-setup/flex_linux_setup/flex_setup.py around lines 372-374, the
policy store JSON is downloaded without integrity checks; after the download
(around line ~382) add a SHA256 verification step that computes the file hash
(using base.get_file_hash or equivalent), compare it to a trusted expected
checksum stored as a constant or config value, and raise an exception/abort the
install if the hashes differ; alternatively, if signed artifacts are available
verify the signature instead or ensure the URL is pinned to an immutable commit
and document the expected checksum source.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify the policy store URL is accessible and consider using an immutable reference.

The policy store is downloaded from the main branch, which is mutable and can lead to non-reproducible builds. For stability and security, consider using a specific git tag or commit hash in the URL path instead of refs/heads/main.

Additionally, verify that the URL is accessible and that the filename hash 2fb50e468d9dfefa142d1fce4fa9747efbd3a0f08de5 is correct (it appears to be 44 characters, which is an unusual hash length).

Run the following script to verify URL accessibility:

🏁 Script executed:

#!/bin/bash
# Verify the policy store URL is accessible

policy_store_url="https://raw.githubusercontent.com/GluuFederation/GluuFlexAdminUIPolicyStore/refs/heads/main/2fb50e468d9dfefa142d1fce4fa9747efbd3a0f08de5.json"

echo "Testing URL accessibility..."
http_code=$(curl -s -o /dev/null -w "%{http_code}" "$policy_store_url")

if [ "$http_code" = "200" ]; then
    echo "✓ URL is accessible (HTTP $http_code)"
    echo ""
    echo "File size and content type:"
    curl -sI "$policy_store_url" | grep -E "(Content-Length|Content-Type)"
    echo ""
    echo "First 200 characters of content:"
    curl -s "$policy_store_url" | head -c 200
else
    echo "✗ URL returned HTTP $http_code"
fi

echo -e "\n\nChecking repository structure..."
curl -s "https://api.github.com/repos/GluuFederation/GluuFlexAdminUIPolicyStore/contents/" | jq -r '.[].name' 2>/dev/null || echo "Could not fetch repository contents"

Length of output: 438

🏁 Script executed:

#!/bin/bash

echo "=== 1. Finding base.download() implementation ==="
rg -n "def download|\.download\(" -A 10 -B 2 flex-linux-setup/flex_linux_setup/ | head -100

echo ""
echo "=== 2. Finding all URLs in source_files ==="
rg -n "https://" flex-linux-setup/flex_linux_setup/flex_setup.py | grep -E "(source_files|self\.source_files|http)" -A 1 -B 1

echo ""
echo "=== 3. Checking for checksum/signature validation ==="
rg -n "checksum|hash|verify|signature|integrity" flex-linux-setup/flex_linux_setup/flex_setup.py

echo ""
echo "=== 4. Examining download loop and error handling around policy_store ==="
rg -n "for.*source_files|policy_store" flex-linux-setup/flex_linux_setup/flex_setup.py -B 2 -A 5

Length of output: 8404

🏁 Script executed:

#!/bin/bash

echo "=== 1. Finding base module import ==="
rg -n "^import|^from" flex-linux-setup/flex_linux_setup/flex_setup.py | head -30

echo ""
echo "=== 2. Searching for base.download definition ==="
fd . flex-linux-setup -type f -name "*.py" -exec grep -l "def download" {} \;

echo ""
echo "=== 3. Checking base module for error handling ==="
fd . flex-linux-setup -type f -name "base.py" -exec cat {} \;

Length of output: 1464

🏁 Script executed:

#!/bin/bash

echo "=== 1. Finding base.py module ==="
find flex-linux-setup -name "base.py" -type f

echo ""
echo "=== 2. Extracting base.download() function ==="
find flex-linux-setup -name "base.py" -type f -exec grep -A 20 "def download" {} +

echo ""
echo "=== 3. Checking for exception handling in download flow ==="
find flex-linux-setup -name "base.py" -type f -exec cat {} \;

Length of output: 142

🏁 Script executed:

#!/bin/bash

echo "=== 1. Check if policy_store_path file is validated before use ==="
rg -n "policy_store_path|renderTemplateInOut" flex-linux-setup/flex_linux_setup/flex_setup.py -B 3 -A 3

echo ""
echo "=== 2. Look for any configuration or version control strategy doc ==="
find flex-linux-setup -name "README*" -o -name "CHANGELOG*" -o -name "*.md" | head -10

echo ""
echo "=== 3. Check error handling patterns in download_files method ==="
sed -n '344,386p' flex-linux-setup/flex_linux_setup/flex_setup.py

echo ""
echo "=== 4. See if there's version or branch configuration ==="
rg -n "FLEX_BRANCH|app_versions|refs/heads" flex-linux-setup/flex_linux_setup/flex_setup.py | head -20

Length of output: 6205

Address missing error handling and inconsistent branch reference for policy store.

The policy store URL is hardcoded to the mutable main branch, unlike other downloads which use configurable branches (JANS_BRANCH, FLEX_BRANCH). More critically, if the download fails, line 576 will execute renderTemplateInOut() on a non-existent file without validation, causing the setup to crash.

Recommendations:

  • Add error handling around the download at lines 380–382 or validate file existence before line 576
  • Either make the policy store URL configurable via CLI (like other branches) or pin it to a specific release tag/commit hash for reproducibility and stability
  • Consider adding checksum validation for this security-critical file before use
🤖 Prompt for AI Agents 
In flex-linux-setup/flex_linux_setup/flex_setup.py around lines 372-374 (and
referencing the download area ~380-382 and later renderTemplateInOut usage at
line 576), the policy store URL is hardcoded to the mutable "main" branch and
there is no error handling or validation before renderTemplateInOut is called.
Change the URL to use an existing branch/variable (e.g., FLEX_BRANCH or a new
CLI option) or pin it to a release tag/commit for reproducibility; wrap the
download call with try/except (or check the HTTP status) and raise or log a
clear error if it fails; before calling renderTemplateInOut at line 576,
validate that self.policy_store_path exists and is a readable file (and
optionally verify a checksum) and abort with a helpful error if validation
fails.

Copy link
Copy Markdown
Contributor Author

@devrimyatar devrimyatar Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai This file https://raw.githubusercontent.com/GluuFederation/GluuFlexAdminUIPolicyStore/refs/heads/main/2fb50e468d9dfefa142d1fce4fa9747efbd3a0f08de5.json is updated occasionally and not static, therefor we don't (and can't) have a checksum hash for that file. Additionally, we should download from main branch since all commits are meged into main.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@devrimyatar, thank you for the clarification! I understand that the policy store needs to be fetched from main since it's regularly updated, and static checksums aren't feasible for a living configuration file.

I notice the code already has a good safeguard at line 576 with if os.path.exists(self.policy_store_path): to handle cases where the file doesn't exist, which addresses the concern about rendering a non-existent file.

Given the dynamic nature of this file, here are a couple of optional suggestions to consider for robustness:

  1. Add explicit error handling for the download to provide clear feedback if it fails:
try:
    base.download(policy_store_url, self.policy_store_path, verbose=True)
except Exception as e:
    print(f"Warning: Failed to download policy store: {e}")
    print("Admin UI will continue without cedarling policy store integration")
  1. Basic JSON validation after download to catch corrupted downloads early:
if os.path.exists(self.policy_store_path):
    try:
        with open(self.policy_store_path) as f:
            json.load(f)  # Validates JSON format
        config_api_installer.renderTemplateInOut(self.policy_store_path, self.templates_dir, admin_ui_config_dir)
    except json.JSONDecodeError as e:
        print(f"Warning: Downloaded policy store is not valid JSON: {e}")

These are optional enhancements for better visibility when issues occur, but the current implementation with the existence check is already reasonably safe.

🧠 Learnings used 
Learnt from: devrimyatar
Repo: GluuFederation/flex PR: 2440
File: flex-linux-setup/flex_linux_setup/flex_setup.py:519-519
Timestamp: 2025-11-11T15:17:34.639Z
Learning: In flex-linux-setup/flex_linux_setup/flex_setup.py, the webhook LDIF (aui_webhook.ldif) should only be imported during fresh Admin UI installations, not during updates. The import is correctly guarded by the check `client_check_result['2001.'] == -1` which ensures it only runs when the Admin UI Web Client doesn't already exist. Docker update workflows differ from Linux update workflows in this regard.

Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).

Copy link
Copy Markdown
Contributor Author

@devrimyatar devrimyatar Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai

  1. You suggest error handling for fle download. But is is done in function base.download() (see https://github.com/JanssenProject/jans/blob/main/jans-linux-setup/jans_setup/setup_app/utils/base.py#L342-L374)
  2. I checked validty of JSON in commit
    b3e68a2

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch the commit diff to see the JSON validation implementation
gh api repos/GluuFederation/flex/commits/b3e68a27663df8491cc615dfaed2aa17d55c985a --jq '.files[] | select(.filename | contains("flex_setup.py")) | .patch'

Length of output: 955

@devrimyatar, thank you for the clarification and for implementing the JSON validation!

You're absolutely right on both points:

  1. Error handling: I see that base.download() already includes comprehensive error handling with retries and proper exception management. I wasn't aware of that implementation detail - thank you for the reference.

  2. JSON validation: Perfect! I can see from the commit that you've added exactly the validation I suggested:

    • JSON format validation with json.load()
    • Try/except block to catch JSONDecodeError
    • Warning message if validation fails
    • Graceful continuation without crashing the setup

The implementation now has multiple layers of protection:

  • ✅ Download error handling in base.download()
  • ✅ File existence check with os.path.exists()
  • ✅ JSON integrity validation before use

This looks solid and handles the dynamic nature of the policy store file appropriately. The setup will gracefully continue even if the policy store is temporarily unavailable or corrupted, which is the right behavior.

✏️ Learnings added 
Learnt from: devrimyatar
Repo: GluuFederation/flex PR: 2442
File: flex-linux-setup/flex_linux_setup/flex_setup.py:372-374
Timestamp: 2025-11-11T15:45:04.127Z
Learning: In flex-linux-setup/flex_linux_setup/flex_setup.py, the Admin UI policy store JSON file is downloaded from the main branch of the GluuFlexAdminUIPolicyStore repository (https://raw.githubusercontent.com/GluuFederation/GluuFlexAdminUIPolicyStore/refs/heads/main/2fb50e468d9dfefa142d1fce4fa9747efbd3a0f08de5.json). This file is regularly updated, so static checksums are not feasible. The download is handled by base.download() which includes built-in error handling, and JSON validation is performed after download to ensure file integrity before rendering the template.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used 
Learnt from: devrimyatar
Repo: GluuFederation/flex PR: 2440
File: flex-linux-setup/flex_linux_setup/flex_setup.py:519-519
Timestamp: 2025-11-11T15:17:34.639Z
Learning: In flex-linux-setup/flex_linux_setup/flex_setup.py, the webhook LDIF (aui_webhook.ldif) should only be imported during fresh Admin UI installations, not during updates. The import is correctly guarded by the check `client_check_result['2001.'] == -1` which ensures it only runs when the Admin UI Web Client doesn't already exist. Docker update workflows differ from Linux update workflows in this regard.

Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).

]

if argsp.update_admin_ui:
Expand Down Expand Up @@ -570,7 +573,14 @@ def install_gluu_admin_ui(self):

#cedarling integration
admin_ui_config_dir = os.path.join(config_api_installer.custom_config_dir, 'adminUI')
config_api_installer.renderTemplateInOut(self.policy_store_path, self.templates_dir, admin_ui_config_dir)
if os.path.exists(self.policy_store_path):
try:
with open(self.policy_store_path) as f:
json.load(f) # Validates JSON format
config_api_installer.renderTemplateInOut(self.policy_store_path, self.templates_dir, admin_ui_config_dir)
except json.JSONDecodeError as e:
print(f"Warning: Downloaded policy store is not valid JSON: {e}")

config_api_installer.chown(admin_ui_config_dir, Config.jetty_user, Config.jetty_group)
resource_scopes_mapping_lidf_fn = os.path.join(self.templates_dir, 'adminUIResourceScopesMapping.ldif')

Expand Down
Loading
Loading