Skip to content

Make Spymasters necessary to guard secrets and receive reports - #111

Merged
XxFran10xX merged 1 commit into
mainfrom
fix/expose-vacant-spymaster
Oct 3, 2026
Merged

XxFran10xX merged 1 commit into
mainfrom
fix/expose-vacant-spymaster

Conversation

@XxFran10xX

@XxFran10xX XxFran10xX commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Factions without an eligible, living Spymaster now expose exact faction and guild information through the existing viewing bypass path. A faction without its own eligible Spymaster receives no daily reports about protected foreign factions, including previously cached findings. Its report header reads Absent and explains that no findings reach the vacant office's court.

This covers menus, full rosters, ledgers and wealth rankings. An eligible zero-aptitude Spymaster still protects information. Public viewing does not grant faction membership, office management, or private sabotage access; Minecraft account names remain exclusive to the staff bypass. Removal controls warn that faction and guild information will become public.

Unprotected targets and observers are excluded from daily report generation and staff regeneration. Existing appointment costs, stability settings and faction data formats remain unchanged; the deployment will preserve the user's reduced Main appointment penalties.

Validation: clean Java 21 mvn clean verify passed all 2,692 tests, and runtime JAR validation passed. Regression coverage includes vacancy, permanent character death, departed members, ineligible solo leaders, zero aptitude, cached report suppression, exact rankings and management restrictions. TFMCDev01 menu checks passed for exact public faction/guild views, rosters, government, training, ledgers, read-only offices and the shared Absent report header. Probe factions were deleted, and aptitude/config checksums are unchanged. Dev used a combined build preserving its existing pending leader-character export changes (2,703 tests); the isolated PR build passed all 2,692 tests. CodeRabbit approved the latest commit e5e43f0.

@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7a739576-1f06-499f-85e5-e3d7b035a80e
📥 Commits

Reviewing files that changed from the base of the PR and between 896c8b1 and e5e43f0.

📒 Files selected for processing (10)
  • ESPIONAGE.md
  • src/main/java/net/tfminecraft/simplefactions/espionage/EspionageService.java
  • src/main/java/net/tfminecraft/simplefactions/managers/inventory/EspionageView.java
  • src/test/java/net/tfminecraft/simplefactions/espionage/CharacterNameResolutionTest.java
  • src/test/java/net/tfminecraft/simplefactions/espionage/EspionageBypassTest.java
  • src/test/java/net/tfminecraft/simplefactions/espionage/EspionagePermissionsTest.java
  • src/test/java/net/tfminecraft/simplefactions/espionage/EspionageTestFixtures.java
  • src/test/java/net/tfminecraft/simplefactions/espionage/IntelligenceBrowsingTest.java
  • src/test/java/net/tfminecraft/simplefactions/espionage/ReportPresentationTest.java
  • src/test/java/net/tfminecraft/simplefactions/espionage/UnguardedFactionTest.java

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Summary

Summary by CodeRabbit

  • New Features
    • Factions without an eligible, living Spymaster now display exact information publicly. Eligible Spymasters protect faction information, even with zero aptitude.
    • Intelligence reports are available only when the observing faction has an eligible Spymaster; reports about protected factions are gathered only when they also have one.
    • Vacating the Spymaster’s office immediately stops reports and makes the faction’s information public. Reinstating an eligible Spymaster restores access to still-current cached reports.
    • The Spymaster’s Office now explains these visibility rules and indicates when the office is vacant.

Walkthrough

The espionage service now treats factions without an eligible, living Spymaster as unguarded. Unguarded factions expose exact information and do not receive reports about protected factions. Report access and generation require active Spymasters, and the office and report displays reflect these rules.

Changes

Spymaster protection and intelligence reports

Layer / File(s) Summary
Spymaster eligibility and exact visibility
src/main/java/net/tfminecraft/simplefactions/espionage/EspionageService.java, src/main/java/net/tfminecraft/simplefactions/managers/inventory/EspionageView.java, ESPIONAGE.md, src/test/.../espionage/*
The service checks whether a faction has an eligible, living Spymaster and allows exact information when it does not. Tests cover holder eligibility, faction visibility, and office access. The office description and documentation describe the visibility rules.
Report access and refresh
src/main/java/net/tfminecraft/simplefactions/espionage/EspionageService.java, src/main/java/net/tfminecraft/simplefactions/managers/inventory/EspionageView.java, ESPIONAGE.md, src/test/.../espionage/*
Report access and generation require active Spymasters in the observer and target factions. Refresh and regeneration count only reports that are produced. The report display, documentation, and tests cover report eligibility and cached-report access.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant ObserverFaction
  participant EspionageService
  participant TargetFaction
  participant CachedReports
  ObserverFaction->>EspionageService: Request report refresh
  EspionageService->>ObserverFaction: Check for active Spymaster
  EspionageService->>TargetFaction: Check for active Spymaster
  EspionageService->>CachedReports: Generate report when both factions qualify
Loading

Merge Risk: ⚪ Minimal · up to e5e43

No actionable issue remains from this review. The change is mergeable after normal deployment checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to e5e43

The intended information-sharing change preserves the inspected management restrictions, but it also widens a character-name fallback that can reveal Minecraft account names to ordinary foreign viewers. Cached-report behavior is documented, while some live-menu and runtime behavior remains unconfirmed.

Retained concerns

  • Medium · security · inferred: The expanded exact-information permission also selects the character-name formatter's account-name fallback. With RPCharacters enabled and no resolvable active character name, ordinary foreign viewers of unguarded factions can receive Minecraft account names instead of Unknown. Information visibility and account-identity disclosure therefore no longer remain separate privileges, contrary to the documented staff-only identity rule.
Security review details

Security Blast Radius

  • inferred — An ordinary server player can view exact information for every unguarded faction and its guilds, without belonging to those factions or having a Spymaster. The identified privacy path affects newly public name presentation when character-name resolution fails; no administrative privilege gain was established.

Security Findings and Attack Paths

  • inferred — The privacy path is foreign viewing of an unguarded target, followed by the expanded exact-view check selecting display rather than forForeign. If active-name lookup returns null, of returns the raw account name. Valid character names avoid this fallback, and the explicit account-name suffix remains staff-gated. Client-visible player-head profile metadata was not established and is not a separate finding.

Trust Boundaries and Controls

  • observed — Inspected appointment and removal APIs require current faction ownership and leadership; sabotage requires the appointed holder's identity. Company asset mutation APIs independently require company or guild leadership. Exact viewing alone does not satisfy these controls.

Resilience and Maintainability Implications

  • observed — Explicit appointment rolls back office state and treasury changes when checked persistence fails. Background cleanup and regeneration retain pre-existing unchecked save calls. Current eligibility is still checked before report access; restart recovery after failed writes was not fully established, and no introduced recovery vulnerability is claimed.

Hardening Proposals

  • proposed — Keep account-identity disclosure governed by an explicit identity permission, independent of exact faction-information visibility. Define a non-account fallback for ordinary foreign viewers when roleplay names are unavailable, and validate identity-bearing item metadata against the same policy.
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@XxFran10xX
XxFran10xX merged commit 17b8a1a into main Oct 3, 2026
2 checks passed
@XxFran10xX
XxFran10xX deleted the fix/expose-vacant-spymaster branch October 3, 2026 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant