Skip to content

Commit e00ced2

Browse files
dvir001CopilotCopilot
authored
Store settings in config/ and add import/export (#49)
* Store settings in config/ and add import/export Move application settings into a dedicated config/ directory and unify email settings into the single app_settings.json file. Update simple_org_chart.config to expose CONFIG_DIR/REPO_DIR and ensure directories are created. Modify email_config and settings save/load logic to read/write the unified SETTINGS_FILE while preserving unrelated keys (so email/settings don't overwrite each other). Add API endpoints (/api/settings/export and /api/settings/import) and corresponding UI (configure page, JS, and locale entries) to export/import configuration. Change user-scanner behavior to use GitHub releases (with PyPI fallback) and add UI-driven manual install flow instead of auto-install. Update Dockerfiles and compose files to mount ./config, add gosu and an entrypoint script to fix bind-mount permissions before dropping privileges. Update tests to expect the new config location and adjust other frontend and backend code to handle missing DOM elements and defaults. Overall this enables persistent, portable settings and safer container bind-mount handling. * Fix settings file safety, export filtering, container permissions, and i18n in install flow (#50) * Initial plan * Address all PR review comments: locking, i18n, security, naming, and permissions Co-authored-by: dvir001 <39403717+dvir001@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: dvir001 <39403717+dvir001@users.noreply.github.com> * Harden settings file concurrency, email key isolation, and import guard (#51) * Initial plan * Fix inter-process locking, email key restriction, dict check, and conditional settings save Co-authored-by: dvir001 <39403717+dvir001@users.noreply.github.com> * Remove committed lock file and add config/*.lock to .gitignore Co-authored-by: dvir001 <39403717+dvir001@users.noreply.github.com> * Fix fd leak in lock acquire, remove locked() race, and extract _filter_email_keys helper Co-authored-by: dvir001 <39403717+dvir001@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: dvir001 <39403717+dvir001@users.noreply.github.com> * Fix lock path resolution, i18n gaps, and entrypoint symlink safety (#52) * Initial plan * Fix lock path at acquire-time, i18n hardcoded strings, entrypoint symlink safety Co-authored-by: dvir001 <39403717+dvir001@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: dvir001 <39403717+dvir001@users.noreply.github.com> * Update simple_org_chart/app_main.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update simple_org_chart/settings.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 91c1a60 commit e00ced2

16 files changed

Lines changed: 1328 additions & 834 deletions

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,6 @@ repositories/
77
__pycache__/
88
*.pyc
99
flask_session/
10+
11+
# Runtime lock files
12+
config/*.lock

Dockerfile

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ RUN apt-get update && apt-get install -y \
2020
python3-pip \
2121
gcc \
2222
curl \
23+
gosu \
2324
&& rm -rf /var/lib/apt/lists/* \
2425
&& groupadd --system app \
2526
&& useradd --system --gid app --create-home app
@@ -43,18 +44,23 @@ RUN playwright install --with-deps chromium && \
4344
COPY . .
4445

4546
# Create necessary directories for data persistence and adjust ownership
46-
RUN mkdir -p static data data/photos repositories && \
47+
RUN mkdir -p static data data/photos config repositories && \
48+
chmod 775 data config repositories && \
4749
chown -R app:app /app
4850

51+
# Make entrypoint executable
52+
COPY deploy/entrypoint.sh /entrypoint.sh
53+
RUN chmod +x /entrypoint.sh
54+
4955
# Expose port
5056
EXPOSE ${APP_PORT}
5157

5258
# Health check
5359
HEALTHCHECK --interval=30s --timeout=30s --start-period=5s --retries=3 \
5460
CMD curl -f http://localhost:${APP_PORT:-5000}/ || exit 1
5561

56-
# Switch to non-root user
57-
USER app
62+
# Entrypoint fixes bind-mount permissions then drops to 'app' user
63+
ENTRYPOINT ["/entrypoint.sh"]
5864

5965
# Run the application using gunicorn
6066
CMD ["gunicorn", "--config", "deploy/gunicorn.conf.py", "simple_org_chart:app"]

deploy/entrypoint.sh

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
#!/bin/bash
2+
set -e
3+
4+
APP_UID=$(id -u app)
5+
APP_GID=$(id -g app)
6+
7+
# Fix ownership of bind-mounted directories so the 'app' user can write to them.
8+
# Only chown when the top-level directory owner doesn't already match to avoid
9+
# slow recursive traversal of large bind mounts on every container start.
10+
for dir in /app/data /app/config /app/repositories; do
11+
if [ -d "$dir" ]; then
12+
current_owner=$(stat -c '%u:%g' "$dir" 2>/dev/null || echo "")
13+
if [ -z "$current_owner" ] || [ "$current_owner" != "$APP_UID:$APP_GID" ]; then
14+
chown -R --no-dereference app:app "$dir" 2>/dev/null || true
15+
fi
16+
fi
17+
done
18+
19+
# Drop privileges and exec the main process
20+
exec gosu app "$@"

docker-compose-dev.yml

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,6 @@ services:
1010
env_file:
1111
- .env
1212
volumes:
13-
- orgchart_data_dev:/app/data
14-
- orgchart_repos_dev:/app/repositories
15-
labels:
16-
com.docker.compose.project: "docker"
17-
networks:
18-
- orgchart_dev_default
19-
20-
networks:
21-
orgchart_dev_default:
22-
23-
volumes:
24-
orgchart_data_dev:
25-
orgchart_repos_dev:
13+
- ./data:/app/data
14+
- ./config:/app/config
15+
- ./repositories:/app/repositories

docker-compose.yml

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,6 @@ services:
1010
env_file:
1111
- .env
1212
volumes:
13-
- orgchart_data:/app/data
14-
- orgchart_repos:/app/repositories
15-
labels:
16-
com.docker.compose.project: "docker"
17-
networks:
18-
- default
19-
20-
networks:
21-
default:
22-
23-
volumes:
24-
orgchart_data:
25-
orgchart_repos:
13+
- ./data:/app/data
14+
- ./config:/app/config
15+
- ./repositories:/app/repositories

simple_org_chart/app_main.py

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ def flock(fd, op):
6060
from simple_org_chart.config import EMPLOYEE_LIST_FILE
6161
from simple_org_chart.auth import login_required, require_auth, sanitize_next_path
6262
from simple_org_chart.email_config import (
63+
DEFAULT_EMAIL_CONFIG,
6364
get_smtp_config,
6465
is_smtp_configured,
6566
load_email_config,
@@ -1128,6 +1129,7 @@ def reset_all_settings():
11281129
os.remove(logo_path)
11291130

11301131
save_settings(DEFAULT_SETTINGS)
1132+
save_email_config(DEFAULT_EMAIL_CONFIG)
11311133

11321134
threading.Thread(target=restart_scheduler).start()
11331135

@@ -1137,6 +1139,89 @@ def reset_all_settings():
11371139
return jsonify({'error': 'Reset failed'}), 500
11381140

11391141

1142+
# ---------------------------------------------------------------------------
1143+
# Config export / import
1144+
# ---------------------------------------------------------------------------
1145+
1146+
@app.route('/api/settings/export', methods=['GET'])
1147+
@require_auth
1148+
def export_settings():
1149+
"""Download all configuration as a single flat JSON file."""
1150+
settings = load_settings()
1151+
email_config = load_email_config()
1152+
# Merge everything flat; strip transient runtime fields and non-default keys
1153+
settings_public = {k: v for k, v in settings.items() if k in DEFAULT_SETTINGS}
1154+
merged = {}
1155+
merged.update(settings_public)
1156+
merged.update({k: v for k, v in email_config.items() if k != 'lastSent'})
1157+
payload = json.dumps(merged, indent=2)
1158+
return app.response_class(
1159+
payload,
1160+
mimetype='application/json',
1161+
headers={
1162+
'Content-Disposition': 'attachment; filename="simple-org-chart-config.json"',
1163+
},
1164+
)
1165+
1166+
1167+
@app.route('/api/settings/import', methods=['POST'])
1168+
@require_auth
1169+
def import_settings():
1170+
"""Import configuration from an uploaded JSON file."""
1171+
uploaded = request.files.get('file')
1172+
if not uploaded:
1173+
return jsonify({'error': 'No file uploaded'}), 400
1174+
1175+
# Enforce file size limit consistent with other upload endpoints
1176+
uploaded.seek(0, 2)
1177+
file_size = uploaded.tell()
1178+
uploaded.seek(0)
1179+
if file_size > MAX_FILE_SIZE:
1180+
return jsonify({'error': f'File too large. Maximum size: {MAX_FILE_SIZE // (1024 * 1024)}MB'}), 400
1181+
1182+
try:
1183+
raw = uploaded.read()
1184+
incoming = json.loads(raw)
1185+
except (json.JSONDecodeError, UnicodeDecodeError) as exc:
1186+
logger.warning("Config import: invalid JSON – %s", exc)
1187+
return jsonify({'error': 'Invalid JSON file'}), 400
1188+
1189+
if not isinstance(incoming, dict):
1190+
return jsonify({'error': 'Expected a JSON object'}), 400
1191+
1192+
# Split incoming keys into settings vs email config
1193+
email_keys = set(DEFAULT_EMAIL_CONFIG.keys()) - {'lastSent'}
1194+
settings_data = {}
1195+
email_data = {}
1196+
1197+
for key, value in incoming.items():
1198+
if key in DEFAULT_SETTINGS:
1199+
settings_data[key] = value
1200+
elif key in email_keys:
1201+
email_data[key] = value
1202+
1203+
# Only save settings if at least one valid settings key was provided
1204+
settings_ok = True
1205+
if settings_data:
1206+
settings_ok = save_settings(settings_data)
1207+
1208+
email_ok = True
1209+
if email_data:
1210+
email_ok = save_email_config(email_data)
1211+
1212+
if settings_ok and email_ok:
1213+
if 'updateTime' in settings_data or 'autoUpdateEnabled' in settings_data:
1214+
threading.Thread(target=restart_scheduler, daemon=True).start()
1215+
return jsonify({'success': True, 'settings': load_settings()})
1216+
1217+
errors = []
1218+
if not settings_ok:
1219+
errors.append('settings')
1220+
if not email_ok:
1221+
errors.append('email config')
1222+
return jsonify({'error': f'Failed to save: {", ".join(errors)}'}), 500
1223+
1224+
11401225
@app.route('/api/email-config', methods=['GET', 'POST'])
11411226
@require_auth
11421227
@limiter.limit(RATE_LIMIT_SETTINGS)

simple_org_chart/config.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,12 @@
1111

1212
BASE_DIR = Path(__file__).resolve().parent.parent
1313
DATA_DIR = BASE_DIR / "data"
14+
CONFIG_DIR = BASE_DIR / "config"
1415
STATIC_DIR = BASE_DIR / "static"
1516
TEMPLATE_DIR = BASE_DIR / "templates"
1617

17-
SETTINGS_FILE = DATA_DIR / "app_settings.json"
18+
REPO_DIR = BASE_DIR / "repositories"
19+
SETTINGS_FILE = CONFIG_DIR / "app_settings.json"
1820
DATA_FILE = DATA_DIR / "employee_data.json"
1921
MISSING_MANAGER_FILE = DATA_DIR / "missing_manager_records.json"
2022
EMPLOYEE_LIST_FILE = DATA_DIR / "employee_list.json"
@@ -28,8 +30,8 @@
2830

2931

3032
def ensure_directories() -> None:
31-
"""Ensure that the application's data and static directories exist."""
32-
for target in (DATA_DIR, STATIC_DIR):
33+
"""Ensure that the application's data, config, static, and repo directories exist."""
34+
for target in (DATA_DIR, CONFIG_DIR, STATIC_DIR, REPO_DIR):
3335
try:
3436
target.mkdir(parents=True, exist_ok=True)
3537
except OSError as error:
@@ -44,8 +46,10 @@ def as_posix_env(mapping: Dict[str, Path]) -> Dict[str, str]:
4446
__all__ = [
4547
"BASE_DIR",
4648
"DATA_DIR",
49+
"CONFIG_DIR",
4750
"STATIC_DIR",
4851
"TEMPLATE_DIR",
52+
"REPO_DIR",
4953
"SETTINGS_FILE",
5054
"DATA_FILE",
5155
"MISSING_MANAGER_FILE",

0 commit comments

Comments
 (0)