Skip to content

Commit c850745

Browse files
dc-larsenclaudelelia
authored
Fix: Jira dashboard config params not reaching notifier (#22)
* Fix: Jira dashboard config params not reaching notifier Two bugs were preventing Jira settings configured in the Socket dashboard from being used by the Jira notifier: 1. Parameter name mismatch in notifications.yaml - Dashboard API returns `jiraUrl` -> normalized to `jira_url` - But notifications.yaml defined param as `name: server` - Manager looks up params by name, so `jira_url` was never found - Fixed by renaming params to match dashboard keys: `jira_url`, `jira_project` 2. Priority bug in manager param resolution - Comment said "app_config -> env var -> default" (app_config highest) - But code would set val from app_config, then overwrite with default if env var wasn't set - Fixed by restructuring: start with default, apply env var, then app_config Also updated jira_notifier.py to look for the correct param names (`jira_url`, `jira_project`, `jira_email`, `jira_api_token`) and fall back to the `auth` dict for backwards compatibility. Tested locally with simulated dashboard config - all four Jira params now resolve correctly. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Add script to verify JIRA config Signed-off-by: lelia <lelia@socket.dev> * Add unittests for notifier functionality, plus README for tests Signed-off-by: lelia <lelia@socket.dev> * Update ignore file to include necessary scripts Signed-off-by: lelia <lelia@socket.dev> * Update parameter docs to provide local verification option Signed-off-by: lelia <lelia@socket.dev> * Guard against empty env vars overriding valid config Reject empty/whitespace-only env var values in notifier param resolution so they don't silently override app_config or defaults. --------- Signed-off-by: lelia <lelia@socket.dev> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: lelia <le1ia@me.com> Co-authored-by: lelia <lelia@socket.dev>
1 parent 6f427b8 commit c850745

9 files changed

Lines changed: 269 additions & 18 deletions

File tree

.gitignore

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,12 @@ test
1212
run_container.sh
1313
*.zip
1414
bin
15+
# Note: Ignore any local dev scripts, but include ones used by the project
1516
scripts/*.py
17+
!scripts/enrich_rules.py
18+
!scripts/rewrite_messages.py
19+
!scripts/update_cwe_catalog.py
20+
!scripts/verify_jira_dashboard_config.py
1621
*.json
1722
markdown_overview_temp.md
1823
markdown_security_temp.md
@@ -26,6 +31,9 @@ test.py
2631
file_generator.py
2732
.env
2833
*.md
34+
!README.md
35+
!docs/*.md
36+
!tests/README.md
2937
test_results
3038
local_tests/
3139
custom_rules/
@@ -103,4 +111,4 @@ logs/
103111
.python-version
104112
.socket.fact.json
105113

106-
custom_rules/
114+
custom_rules/

docs/parameters.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,16 @@ socket-basics \
432432
--jira-api-token "your-token"
433433
```
434434

435+
**Local Verification (No Jira API Calls)**
436+
Use the helper script to confirm dashboard/env Jira settings are wired into the notifier:
437+
```bash
438+
./venv/bin/python scripts/verify_jira_dashboard_config.py
439+
```
440+
Notes:
441+
- The script only loads config and inspects notifier parameters; it does not contact Jira.
442+
- It requires `SOCKET_SECURITY_API_KEY` (and usually `SOCKET_ORG`) to fetch dashboard config.
443+
- You can use `INPUT_JIRA_*` env vars to simulate dashboard values.
444+
435445
### GitHub Pull Request Comments
436446

437447
**CLI Options:**
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
#!/usr/bin/env python3
2+
"""
3+
Verify that dashboard-provided Jira config (via Socket API) flows into JiraNotifier.
4+
5+
This script does NOT call Jira. It only loads Socket Basics config, builds the
6+
NotificationManager, and inspects JiraNotifier params.
7+
8+
Expected env vars:
9+
- SOCKET_SECURITY_API_KEY or SOCKET_SECURITY_API_TOKEN (required)
10+
- SOCKET_ORG or SOCKET_ORG_SLUG (recommended; auto-discovery is attempted)
11+
"""
12+
13+
from __future__ import annotations
14+
15+
import json
16+
import logging
17+
import os
18+
import sys
19+
from pathlib import Path
20+
21+
from socket_basics.core.config import merge_json_and_env_config
22+
from socket_basics.core.notification.manager import NotificationManager
23+
24+
25+
def _summarize_jira(notifier) -> dict:
26+
return {
27+
"server": getattr(notifier, "server", None),
28+
"project": getattr(notifier, "project", None),
29+
"email": getattr(notifier, "email", None),
30+
"api_token_present": bool(getattr(notifier, "api_token", None)),
31+
}
32+
33+
34+
def _load_dotenv(dotenv_path: Path) -> None:
35+
if not dotenv_path.exists():
36+
return
37+
try:
38+
for raw_line in dotenv_path.read_text().splitlines():
39+
line = raw_line.strip()
40+
if not line or line.startswith("#"):
41+
continue
42+
if line.startswith("export "):
43+
line = line[len("export "):].strip()
44+
if "=" not in line:
45+
continue
46+
key, val = line.split("=", 1)
47+
key = key.strip()
48+
val = val.strip().strip("'").strip('"')
49+
if key and key not in os.environ:
50+
os.environ[key] = val
51+
except Exception:
52+
# Best-effort; do not fail if .env parsing is imperfect
53+
pass
54+
55+
56+
def main() -> int:
57+
logging.basicConfig(level=logging.INFO, format="%(levelname)s %(message)s")
58+
59+
repo_root = Path(__file__).resolve().parents[1]
60+
_load_dotenv(repo_root / ".env")
61+
62+
try:
63+
config_dict = merge_json_and_env_config()
64+
except Exception as exc:
65+
print(f"ERROR: failed to load config: {exc}")
66+
return 2
67+
68+
# Load notifications.yaml and build manager
69+
try:
70+
import yaml
71+
72+
cfg_path = repo_root / "socket_basics" / "notifications.yaml"
73+
notif_cfg = None
74+
if cfg_path.exists():
75+
with open(cfg_path, "r") as f:
76+
notif_cfg = yaml.safe_load(f)
77+
except Exception as exc:
78+
print(f"ERROR: failed to load notifications.yaml: {exc}")
79+
return 2
80+
81+
nm = NotificationManager(notif_cfg, app_config=config_dict)
82+
nm.load_from_config()
83+
84+
jira_notifiers = [n for n in nm.notifiers if getattr(n, "name", "").lower() == "jira"]
85+
if not jira_notifiers:
86+
print("Jira notifier not enabled. Check that dashboard config includes jira_url or env provides JIRA_URL.")
87+
return 1
88+
89+
# Print details for first Jira notifier
90+
summary = _summarize_jira(jira_notifiers[0])
91+
print("Jira notifier config summary:")
92+
print(json.dumps(summary, indent=2))
93+
94+
missing = [k for k, v in summary.items() if k in ("server", "project", "email") and not v]
95+
if missing:
96+
print(f"WARNING: missing expected fields: {', '.join(missing)}")
97+
return 1
98+
99+
print("OK: Jira dashboard config appears to be wired into JiraNotifier.")
100+
return 0
101+
102+
103+
if __name__ == "__main__":
104+
sys.exit(main())

socket_basics/core/notification/jira_notifier.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,23 @@ class JiraNotifier(BaseNotifier):
2121
def __init__(self, params: Dict[str, Any] | None = None):
2222
super().__init__(params or {})
2323
# JIRA configuration from params, env variables, or app config
24+
# Parameter names match dashboard config keys (jira_url, jira_project)
2425
self.server = (
25-
self.config.get('server') or
26+
self.config.get('jira_url') or
2627
get_jira_url()
2728
)
2829
self.project = (
29-
self.config.get('project') or
30+
self.config.get('jira_project') or
3031
get_jira_project()
3132
)
3233
self.email = (
33-
self.config.get('email') or
34+
self.config.get('jira_email') or
35+
self.config.get('auth', {}).get('email') or
3436
get_jira_email()
3537
)
3638
self.api_token = (
37-
self.config.get('api_token') or
39+
self.config.get('jira_api_token') or
40+
self.config.get('auth', {}).get('api_token') or
3841
get_jira_api_token()
3942
)
4043

socket_basics/core/notification/manager.py

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -209,27 +209,28 @@ def load_from_config(self) -> None:
209209
env_var = p.get('env_variable')
210210
p_type = p.get('type', 'str')
211211

212-
# Resolve value: app_config -> env var -> default
213-
val = None
214-
if self.app_config and pname in self.app_config:
215-
val = self.app_config.get(pname)
216-
if env_var and os.getenv(env_var) is not None:
212+
# Resolve value priority: app_config (highest) -> env var -> default (lowest)
213+
val = p_default
214+
215+
# Check env var (overrides default)
216+
if env_var:
217217
ev = os.getenv(env_var)
218-
if ev is not None:
218+
if ev is not None and str(ev).strip() != "":
219219
if p_type == 'bool':
220220
val = ev.lower() == 'true'
221221
elif p_type == 'int':
222222
try:
223223
val = int(ev)
224224
except Exception:
225225
logger.warning("Failed to convert notifier param %s=%s to int for notifier %s; using default %s", pname, ev, name, p_default)
226-
val = p_default
227226
else:
228227
val = ev
229-
else:
230-
val = p_default
231-
else:
232-
val = p_default
228+
229+
# Check app_config (highest priority, overrides env var)
230+
if self.app_config and pname in self.app_config:
231+
app_val = self.app_config.get(pname)
232+
if app_val is not None:
233+
val = app_val
233234

234235
params[pname] = val
235236
else:

socket_basics/notifications.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,11 @@ notifiers:
6161
module_path: "socket_basics.core.notification.jira_notifier"
6262
class: "JiraNotifier"
6363
parameters:
64-
- name: server
64+
- name: jira_url
6565
option: --jira-url
6666
env_variable: INPUT_JIRA_URL
6767
type: str
68-
- name: project
68+
- name: jira_project
6969
option: --jira-project
7070
env_variable: INPUT_JIRA_PROJECT
7171
type: str

tests/README.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
# Tests
2+
3+
Quick-start for running tests locally:
4+
5+
```bash
6+
./venv/bin/python -m pytest
7+
```
8+
9+
Notes:
10+
- Tests are lightweight and should not require external services.
11+
- Use the local config-wiring script for Jira setup validation:
12+
`./venv/bin/python scripts/verify_jira_dashboard_config.py`

tests/test_jira_notifier_params.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
from socket_basics.core.notification.jira_notifier import JiraNotifier
2+
from socket_basics.core.config import normalize_api_config
3+
4+
5+
def test_jira_notifier_reads_new_param_names():
6+
n = JiraNotifier(
7+
{
8+
"jira_url": "https://acme.atlassian.net",
9+
"jira_project": "SEC",
10+
"jira_email": "bot@acme.example",
11+
"jira_api_token": "token123",
12+
}
13+
)
14+
assert n.server == "https://acme.atlassian.net"
15+
assert n.project == "SEC"
16+
assert n.email == "bot@acme.example"
17+
assert n.api_token == "token123"
18+
19+
20+
def test_jira_notifier_falls_back_to_auth_dict():
21+
n = JiraNotifier(
22+
{
23+
"jira_url": "https://acme.atlassian.net",
24+
"jira_project": "SEC",
25+
"auth": {"email": "auth@acme.example", "api_token": "auth-token"},
26+
}
27+
)
28+
assert n.email == "auth@acme.example"
29+
assert n.api_token == "auth-token"
30+
31+
32+
def test_normalize_api_config_maps_jira_keys():
33+
normalized = normalize_api_config(
34+
{
35+
"jiraUrl": "https://acme.atlassian.net",
36+
"jiraProject": "SEC",
37+
"jiraEmail": "bot@acme.example",
38+
"jiraApiToken": "token123",
39+
}
40+
)
41+
assert normalized["jira_url"] == "https://acme.atlassian.net"
42+
assert normalized["jira_project"] == "SEC"
43+
assert normalized["jira_email"] == "bot@acme.example"
44+
assert normalized["jira_api_token"] == "token123"
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import os
2+
3+
from socket_basics.core.notification.manager import NotificationManager
4+
5+
6+
def _base_cfg():
7+
return {
8+
"notifiers": {
9+
"jira": {
10+
"module_path": "socket_basics.core.notification.jira_notifier",
11+
"class": "JiraNotifier",
12+
"parameters": [
13+
{"name": "jira_url", "env_variable": "INPUT_JIRA_URL", "type": "str"},
14+
{"name": "jira_project", "env_variable": "INPUT_JIRA_PROJECT", "type": "str"},
15+
{"name": "jira_email", "env_variable": "INPUT_JIRA_EMAIL", "type": "str"},
16+
{"name": "jira_api_token", "env_variable": "INPUT_JIRA_API_TOKEN", "type": "str"},
17+
],
18+
}
19+
}
20+
}
21+
22+
23+
def test_param_precedence_app_config_over_env_and_default(monkeypatch):
24+
cfg = _base_cfg()
25+
# default should be overridden by env, then app_config should win
26+
cfg["notifiers"]["jira"]["parameters"][0]["default"] = "https://default.example"
27+
monkeypatch.setenv("INPUT_JIRA_URL", "https://env.example")
28+
29+
nm = NotificationManager(cfg, app_config={"jira_url": "https://app.example"})
30+
nm.load_from_config()
31+
32+
jira = next(n for n in nm.notifiers if getattr(n, "name", "") == "jira")
33+
assert jira.server == "https://app.example"
34+
35+
36+
def test_param_precedence_env_over_default(monkeypatch):
37+
cfg = _base_cfg()
38+
cfg["notifiers"]["jira"]["parameters"][0]["default"] = "https://default.example"
39+
monkeypatch.setenv("INPUT_JIRA_URL", "https://env.example")
40+
41+
nm = NotificationManager(cfg, app_config={})
42+
nm.load_from_config()
43+
44+
jira = next(n for n in nm.notifiers if getattr(n, "name", "") == "jira")
45+
assert jira.server == "https://env.example"
46+
47+
48+
def test_param_precedence_default_used_when_no_env_or_app_config(monkeypatch):
49+
cfg = _base_cfg()
50+
# Use default for jira_project while enabling via app_config jira_url
51+
cfg["notifiers"]["jira"]["parameters"][1]["default"] = "DEFAULTPROJ"
52+
monkeypatch.delenv("INPUT_JIRA_PROJECT", raising=False)
53+
54+
nm = NotificationManager(cfg, app_config={"jira_url": "https://app.example"})
55+
nm.load_from_config()
56+
57+
jira = next(n for n in nm.notifiers if getattr(n, "name", "") == "jira")
58+
assert jira.project == "DEFAULTPROJ"
59+
60+
61+
def test_jira_enabled_via_app_config(monkeypatch):
62+
cfg = _base_cfg()
63+
monkeypatch.delenv("INPUT_JIRA_URL", raising=False)
64+
65+
nm = NotificationManager(cfg, app_config={"jira_url": "https://app.example"})
66+
nm.load_from_config()
67+
68+
jira = next(n for n in nm.notifiers if getattr(n, "name", "") == "jira")
69+
assert jira.server == "https://app.example"

0 commit comments

Comments
 (0)